Fix type assertions and remove nolint

This commit properly type asserts and removes nolint directives. There
are new helpers to reduce code bloat and possible error messages
refactoring work:
+ asGojaValue: returns a given value as a goja value or panics.
+ gojaValueToString: returns a given a value as string or panics.

This commit also refactors tests to remove nolint directives.

Fixes: #417

common/browser_test.go

@@ -52,13 +52,17 @@ func TestBrowserNewPageInContext(t *testing.T) {
 				ctx context.Context, method string, params easyjson.Marshaler, res easyjson.Unmarshaler,
 			) error {
 				require.Equal(t, target.CommandCreateTarget, method)
-				tp := params.(*target.CreateTargetParams) //nolint:forcetypeassert
+				require.IsType(t, params, &target.CreateTargetParams{})
+				tp, _ := params.(*target.CreateTargetParams)
 				require.Equal(t, "about:blank", tp.URL)
 				require.Equal(t, browserContextID, tp.BrowserContextID)
 
 				// newPageInContext event handler will catch this target ID, and compare it to
-				// the new page's target ID to detect whether the page is loaded.
-				res.(*target.CreateTargetReturns).TargetID = targetID //nolint:forcetypeassert
+				// the new page's target ID to detect whether the page
+				// is loaded.
+				require.IsType(t, res, &target.CreateTargetReturns{})
+				v, _ := res.(*target.CreateTargetReturns)
+				v.TargetID = targetID
 
 				// for the event handler to work, there needs to be an event called
 				// EventBrowserContextPage to be fired. this normally happens when the browser's

common/element_handle.go

@@ -63,7 +63,13 @@ func (h *ElementHandle) boundingBox() (*Rect, error) {
 func (h *ElementHandle) checkHitTargetAt(apiCtx context.Context, point Position) (bool, error) {
 	frame := h.ownerFrame(apiCtx)
 	if frame != nil && frame.parentFrame != nil {
-		element := h.frame.FrameElement().(*ElementHandle) //nolint:forcetypeassert
+		var (
+			el          = h.frame.FrameElement()
+			element, ok = el.(*ElementHandle)
+		)
+		if !ok {
+			return false, fmt.Errorf("unexpected type %T", el)
+		}
 		box, err := element.boundingBox()
 		if err != nil {
 			return false, err
@@ -91,9 +97,7 @@ func (h *ElementHandle) checkHitTargetAt(apiCtx context.Context, point Position)
 
 	// Either we're done or an error happened (returned as "error:..." from JS)
 	const done = "done"
-	//nolint:forcetypeassert
-	value := result.(goja.Value)
-	if value.ExportType().Kind() != reflect.String {
+	if v := asGojaValue(h.ctx, result); v.ExportType().Kind() != reflect.String {
 		// We got a { hitTargetDescription: ... } result
 		// Meaning: Another element is preventing pointer events.
 		//
@@ -102,14 +106,14 @@ func (h *ElementHandle) checkHitTargetAt(apiCtx context.Context, point Position)
 		// because we don't need any more functionality from this JS function
 		// right now.
 		return false, errorFromDOMError("error:intercept")
-	} else if value.String() != done {
-		return false, errorFromDOMError(value.String())
+	} else if v.String() != done {
+		return false, errorFromDOMError(v.String())
 	}
 
 	return true, nil
 }
 
-func (h *ElementHandle) checkElementState(apiCtx context.Context, state string) (*bool, error) {
+func (h *ElementHandle) checkElementState(_ context.Context, state string) (*bool, error) {
 	fn := `
 		(node, injected, state) => {
 			return injected.checkElementState(node, state);
@@ -124,15 +128,16 @@ func (h *ElementHandle) checkElementState(apiCtx context.Context, state string)
 		return nil, err
 	}
 
-	value := result.(goja.Value)
-	switch value.ExportType().Kind() {
+	//nolint:exhaustive
+	switch v := asGojaValue(h.ctx, result); v.ExportType().Kind() {
 	case reflect.String: // An error happened (returned as "error:..." from JS)
-		return nil, errorFromDOMError(value.String())
+		return nil, errorFromDOMError(v.String())
 	case reflect.Bool:
 		returnVal := new(bool)
-		*returnVal = value.ToBoolean()
+		*returnVal = v.ToBoolean()
 		return returnVal, nil
 	}
+
 	return nil, fmt.Errorf("cannot check state %q of element: %q", state, reflect.TypeOf(result))
 }
 
@@ -263,11 +268,7 @@ func (h *ElementHandle) fill(_ context.Context, value string) error {
 	if err != nil {
 		return err
 	}
-	r, ok := result.(goja.Value)
-	if !ok {
-		return fmt.Errorf("expected goja value; got %T", result)
-	}
-	if s := r.String(); s != resultDone {
+	if s := gojaValueToString(h.ctx, result); s != resultDone {
 		// Either we're done or an error happened (returned as "error:..." from JS)
 		return errorFromDOMError(s)
 	}
@@ -642,7 +643,9 @@ func (h *ElementHandle) waitAndScrollIntoViewIfNeeded(apiCtx context.Context, fo
 	return nil
 }
 
-func (h *ElementHandle) waitForElementState(apiCtx context.Context, states []string, timeout time.Duration) (bool, error) {
+func (h *ElementHandle) waitForElementState(
+	apiCtx context.Context, states []string, timeout time.Duration,
+) (bool, error) {
 	fn := `
 		(node, injected, states, timeout) => {
 			return injected.waitForElementStates(node, states, timeout);
@@ -657,16 +660,17 @@ func (h *ElementHandle) waitForElementState(apiCtx context.Context, states []str
 		return false, errorFromDOMError(err.Error())
 	}
 
-	value := result.(goja.Value)
-	switch value.ExportType().Kind() {
+	//nolint:exhaustive
+	switch v := asGojaValue(h.ctx, result); v.ExportType().Kind() {
 	case reflect.String: // Either we're done or an error happened (returned as "error:..." from JS)
-		if value.String() == "done" {
+		if v.String() == "done" {
 			return true, nil
 		}
-		return false, errorFromDOMError(value.String())
+		return false, errorFromDOMError(v.String())
 	case reflect.Bool:
-		return value.ToBoolean(), nil
+		return v.ToBoolean(), nil
 	}
+
 	return false, fmt.Errorf("cannot check states %v of element: %q", states, reflect.TypeOf(result))
 }
 
@@ -815,12 +819,13 @@ func (h *ElementHandle) GetAttribute(name string) goja.Value {
 	}
 	opts := NewElementHandleBaseOptions(h.defaultTimeout())
 	actFn := h.newAction([]string{}, fn, opts.Force, opts.NoWaitAfter, opts.Timeout)
-	value, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
+	v, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
 	if err != nil {
 		k6ext.Panic(h.ctx, "cannot get attribute of %q: %q", name, err)
 	}
 	applySlowMo(h.ctx)
-	return value.(goja.Value)
+
+	return asGojaValue(h.ctx, v)
 }
 
 // Hover scrolls element into view and hovers over its center point.
@@ -847,12 +852,13 @@ func (h *ElementHandle) InnerHTML() string {
 	}
 	opts := NewElementHandleBaseOptions(h.defaultTimeout())
 	actFn := h.newAction([]string{}, fn, opts.Force, opts.NoWaitAfter, opts.Timeout)
-	value, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
+	v, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
 	if err != nil {
 		k6ext.Panic(h.ctx, "cannot get element's inner HTML: %w", err)
 	}
 	applySlowMo(h.ctx)
-	return value.(goja.Value).String()
+
+	return gojaValueToString(h.ctx, v)
 }
 
 // InnerText returns the inner text of the element.
@@ -862,12 +868,13 @@ func (h *ElementHandle) InnerText() string {
 	}
 	opts := NewElementHandleBaseOptions(h.defaultTimeout())
 	actFn := h.newAction([]string{}, fn, opts.Force, opts.NoWaitAfter, opts.Timeout)
-	value, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
+	v, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
 	if err != nil {
 		k6ext.Panic(h.ctx, "cannot get element's inner text: %w", err)
 	}
 	applySlowMo(h.ctx)
-	return value.(goja.Value).String()
+
+	return gojaValueToString(h.ctx, v)
 }
 
 func (h *ElementHandle) InputValue(opts goja.Value) string {
@@ -879,12 +886,13 @@ func (h *ElementHandle) InputValue(opts goja.Value) string {
 		return handle.inputValue(apiCtx)
 	}
 	actFn := h.newAction([]string{}, fn, actionOpts.Force, actionOpts.NoWaitAfter, actionOpts.Timeout)
-	value, err := callApiWithTimeout(h.ctx, actFn, actionOpts.Timeout)
+	v, err := callApiWithTimeout(h.ctx, actFn, actionOpts.Timeout)
 	if err != nil {
 		k6ext.Panic(h.ctx, "cannot get element's input value: %w", err)
 	}
 	applySlowMo(h.ctx)
-	return value.(goja.Value).String()
+
+	return gojaValueToString(h.ctx, v)
 }
 
 // IsChecked checks if a checkbox or radio is checked.
@@ -1180,10 +1188,12 @@ func (h *ElementHandle) SelectOption(values goja.Value, opts goja.Value) []strin
 		k6ext.Panic(h.ctx, "cannot handle element select option: %w", err)
 	}
 	var returnVal []string
-	if err := rt.ExportTo(selectedOptions.(goja.Value), &returnVal); err != nil {
+	if err := rt.ExportTo(asGojaValue(h.ctx, selectedOptions), &returnVal); err != nil {
 		k6ext.Panic(h.ctx, "cannot unpack options in element select option: %w", err)
 	}
+
 	applySlowMo(h.ctx)
+
 	return returnVal
 }
 
@@ -1232,12 +1242,13 @@ func (h *ElementHandle) TextContent() string {
 	}
 	opts := NewElementHandleBaseOptions(h.defaultTimeout())
 	actFn := h.newAction([]string{}, fn, opts.Force, opts.NoWaitAfter, opts.Timeout)
-	value, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
+	v, err := callApiWithTimeout(h.ctx, actFn, opts.Timeout)
 	if err != nil {
 		k6ext.Panic(h.ctx, "cannot get text content of element: %w", err)
 	}
 	applySlowMo(h.ctx)
-	return value.(goja.Value).String()
+
+	return gojaValueToString(h.ctx, v)
 }
 
 // Type scrolls element into view, focuses element and types text.

common/frame.go

@@ -817,23 +817,17 @@ func (f *Frame) Content() string {
 
 	rt := f.vu.Runtime()
 	js := `() => {
-			let content = '';
-			if (document.doctype) {
-				content = new XMLSerializer().serializeToString(document.doctype);
-			}
-			if (document.documentElement) {
-				content += document.documentElement.outerHTML;
-			}
-			return content;
-		}`
-
-	c := f.Evaluate(rt.ToValue(js))
-	content, ok := c.(goja.Value)
-	if !ok {
-		k6ext.Panic(f.ctx, "unexpected content() value type: %T", c)
-	}
+		let content = '';
+		if (document.doctype) {
+			content = new XMLSerializer().serializeToString(document.doctype);
+		}
+		if (document.documentElement) {
+			content += document.documentElement.outerHTML;
+		}
+		return content;
+	}`
 
-	return content.ToString().String()
+	return gojaValueToString(f.ctx, f.Evaluate(rt.ToValue(js)))
 }
 
 // Dblclick double clicks an element matching provided selector.
@@ -1033,12 +1027,8 @@ func (f *Frame) getAttribute(selector, name string, opts *FrameBaseOptions) (goj
 	if err != nil {
 		return nil, err
 	}
-	gv, ok := v.(goja.Value)
-	if !ok {
-		return nil, fmt.Errorf("unexpected type %T", v)
-	}
 
-	return gv, nil
+	return asGojaValue(f.ctx, v), nil
 }
 
 // Goto will navigate the frame to the specified URL and return a HTTP response object.
@@ -1108,12 +1098,8 @@ func (f *Frame) innerHTML(selector string, opts *FrameInnerHTMLOptions) (string,
 	if v == nil {
 		return "", nil
 	}
-	gv, ok := v.(goja.Value)
-	if !ok {
-		return "", fmt.Errorf("unexpected type %T", v)
-	}
 
-	return gv.ToString().String(), nil
+	return gojaValueToString(f.ctx, v), nil
 }
 
 // InnerText returns the inner text of the first element found
@@ -1150,12 +1136,8 @@ func (f *Frame) innerText(selector string, opts *FrameInnerTextOptions) (string,
 	if v == nil {
 		return "", nil
 	}
-	gv, ok := v.(goja.Value)
-	if !ok {
-		return "", fmt.Errorf("unexpected type %T", v)
-	}
 
-	return gv.ToString().String(), nil
+	return gojaValueToString(f.ctx, v), nil
 }
 
 // InputValue returns the input value of the first element found
@@ -1187,12 +1169,8 @@ func (f *Frame) inputValue(selector string, opts *FrameInputValueOptions) (strin
 	if err != nil {
 		return "", err
 	}
-	gv, ok := v.(goja.Value)
-	if !ok {
-		return "", fmt.Errorf("unexpected type %T", v)
-	}
 
-	return gv.ToString().String(), nil
+	return gojaValueToString(f.ctx, v), nil
 }
 
 // IsDetached returns whether the frame is detached or not.
@@ -1664,12 +1642,8 @@ func (f *Frame) textContent(selector string, opts *FrameTextContentOptions) (str
 	if v == nil {
 		return "", nil
 	}
-	gv, ok := v.(goja.Value)
-	if !ok {
-		return "", fmt.Errorf("unexpected type %T", v)
-	}
 
-	return gv.ToString().String(), nil
+	return gojaValueToString(f.ctx, v), nil
 }
 
 func (f *Frame) Title() string {

common/frame_session.go

@@ -866,7 +866,11 @@ func (fs *FrameSession) onTargetCrashed(event *inspector.EventTargetCrashed) {
 	fs.logger.Debugf("FrameSession:onTargetCrashed", "sid:%v tid:%v", fs.session.ID(), fs.targetID)
 
 	// TODO:?
-	fs.session.(*Session).markAsCrashed() //nolint:forcetypeassert
+	s, ok := fs.session.(*Session)
+	if !ok {
+		k6ext.Panic(fs.ctx, "unexpected type %T", fs.session)
+	}
+	s.markAsCrashed()
 	fs.page.didCrash()
 }
 

common/helpers.go

@@ -223,3 +223,19 @@ func TrimQuotes(s string) string {
 func gojaValueExists(v goja.Value) bool {
 	return v != nil && !goja.IsUndefined(v) && !goja.IsNull(v)
 }
+
+// asGojaValue return v as a goja value.
+// panics if v is not a goja value.
+func asGojaValue(ctx context.Context, v interface{}) goja.Value {
+	gv, ok := v.(goja.Value)
+	if !ok {
+		k6ext.Panic(ctx, "unexpected type %T", v)
+	}
+	return gv
+}
+
+// gojaValueToString returns v as string.
+// panics if v is not a goja value.
+func gojaValueToString(ctx context.Context, v interface{}) string {
+	return asGojaValue(ctx, v).String()
+}

common/page.go

@@ -814,9 +814,8 @@ func (p *Page) TextContent(selector string, opts goja.Value) string {
 func (p *Page) Title() string {
 	p.logger.Debugf("Page:Title", "sid:%v", p.sessionID())
 
-	rt := p.vu.Runtime()
-	js := `() => document.title`
-	return p.Evaluate(rt.ToValue(js)).(goja.Value).String()
+	v := p.vu.Runtime().ToValue(`() => document.title`)
+	return gojaValueToString(p.ctx, p.Evaluate(v))
 }
 
 func (p *Page) Type(selector string, text string, opts goja.Value) {

common/page_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // TestPageLocator can be removed later on when we add integration
@@ -23,7 +24,9 @@ func TestPageLocator(t *testing.T) {
 			mainFrame: &Frame{id: wantMainFrameID, ctx: ctx},
 		},
 	}
-	l := p.Locator(wantSelector, nil).(*Locator) //nolint:forcetypeassert
+	v := p.Locator(wantSelector, nil)
+	require.IsType(t, v, &Locator{})
+	l, _ := v.(*Locator)
 	assert.Equal(t, wantSelector, l.selector)
 	assert.Equal(t, wantMainFrameID, string(l.frame.id))