More tests

src/rules/no-element-handle.ts

@@ -1,4 +1,4 @@
-import { isObject, isCalleeProperty } from '../utils/ast';
+import { isCalleeObject, isCalleeProperty } from '../utils/ast';
 import * as ESTree from 'estree';
 import { Rule, AST } from 'eslint';
 
@@ -11,15 +11,15 @@ function getRange(
       ? node.parent.range![0]
       : callee.object.range![0];
 
-  return [start, callee.property.range![1]];
+  return [start, callee.range![1]];
 }
 
 export default {
   create(context) {
     return {
       CallExpression(node) {
         if (
-          isObject(node, 'page') &&
+          isCalleeObject(node, 'page') &&
           (isCalleeProperty(node, '$') || isCalleeProperty(node, '$$'))
         ) {
           context.report({

src/rules/no-eval.ts

@@ -1,12 +1,12 @@
 import { Rule } from 'eslint';
-import { isObject, isCalleeProperty } from '../utils/ast';
+import { isCalleeObject, isCalleeProperty } from '../utils/ast';
 
 export default {
   create(context) {
     return {
       CallExpression(node) {
         if (
-          isObject(node, 'page') &&
+          isCalleeObject(node, 'page') &&
           (isCalleeProperty(node, '$eval') || isCalleeProperty(node, '$$eval'))
         ) {
           context.report({

src/rules/no-focused-test.ts

@@ -1,34 +1,28 @@
 import { Rule } from 'eslint';
-import * as ESTree from 'estree';
-import { isIdentifier, isTestIdentifier } from '../utils/ast';
-
-function isTestGroup(node: ESTree.MemberExpression) {
-  const testGroups = new Set(['describe', 'parallel', 'serial']);
-
-  return (
-    node.object.type === 'MemberExpression' &&
-    node.object.property.type === 'Identifier' &&
-    testGroups.has(node.object.property.name)
-  );
-}
+import { isDescribeCall, isPropertyAccessor, isTest } from '../utils/ast';
 
 export default {
   create(context) {
     return {
-      MemberExpression(node) {
+      CallExpression(node) {
         if (
-          (isTestIdentifier(node) || isTestGroup(node)) &&
-          isIdentifier(node.property, 'only')
+          (isTest(node) || isDescribeCall(node)) &&
+          node.callee.type === 'MemberExpression' &&
+          isPropertyAccessor(node.callee, 'only')
         ) {
-          const range = node.property.range!;
+          const { callee } = node;
 
           context.report({
             messageId: 'noFocusedTest',
             suggest: [
               {
-                messageId: 'removeFocusedTestAnnotation',
+                messageId: 'suggestRemoveOnly',
                 // - 1 to remove the `.only` annotation with dot notation
-                fix: (fixer) => fixer.removeRange([range[0] - 1, range[1]]),
+                fix: (fixer) =>
+                  fixer.removeRange([
+                    callee.property.range![0] - 1,
+                    callee.range![1],
+                  ]),
               },
             ],
             node,
@@ -46,8 +40,8 @@ export default {
     },
     hasSuggestions: true,
     messages: {
-      noFocusedTest: 'Unexpected use of .only() annotation.',
-      removeFocusedTestAnnotation: 'Remove .only() annotation',
+      noFocusedTest: 'Unexpected focused test.',
+      suggestRemoveOnly: 'Remove .only() annotation.',
     },
     type: 'problem',
   },

src/rules/no-page-pause.ts

@@ -1,11 +1,11 @@
 import { Rule } from 'eslint';
-import { isObject, isCalleeProperty } from '../utils/ast';
+import { isCalleeObject, isCalleeProperty } from '../utils/ast';
 
 export default {
   create(context) {
     return {
       CallExpression(node) {
-        if (isObject(node, 'page') && isCalleeProperty(node, 'pause')) {
+        if (isCalleeObject(node, 'page') && isCalleeProperty(node, 'pause')) {
           context.report({ messageId: 'noPagePause', node });
         }
       },

src/rules/no-wait-for-timeout.ts

@@ -1,12 +1,12 @@
 import { Rule } from 'eslint';
-import { isObject, isCalleeProperty } from '../utils/ast';
+import { isCalleeObject, isCalleeProperty } from '../utils/ast';
 
 export default {
   create(context) {
     return {
       CallExpression(node) {
         if (
-          isObject(node, 'page') &&
+          isCalleeObject(node, 'page') &&
           isCalleeProperty(node, 'waitForTimeout')
         ) {
           context.report({

src/utils/ast.ts

@@ -1,5 +1,6 @@
 import { Rule } from 'eslint';
 import * as ESTree from 'estree';
+import { NodeWithParent, TypedNodeWithParent } from './types';
 
 export function getStringValue(node: ESTree.Node) {
   return node.type === 'TemplateLiteral'
@@ -17,7 +18,37 @@ export function isIdentifier(node: ESTree.Node, name: string) {
   return getNodeName(node) === name;
 }
 
-export function isObject(node: ESTree.CallExpression, name: string) {
+function isLiteral<T>(node: ESTree.Node, type: string, value?: T) {
+  return (
+    node.type === 'Literal' &&
+    (value === undefined
+      ? typeof node.value === type
+      : (node.value as any) === value)
+  );
+}
+
+export function isStringLiteral(node: ESTree.Node, value?: string) {
+  return isLiteral(node, 'string', value);
+}
+
+export function isBooleanLiteral(node: ESTree.Node, value?: boolean) {
+  return isLiteral(node, 'boolean', value);
+}
+
+export function getPropertyName(node: ESTree.MemberExpression) {
+  return node.property.type === 'Identifier'
+    ? node.property.name
+    : getStringValue(node.property);
+}
+
+export function isPropertyAccessor(
+  node: ESTree.MemberExpression,
+  name: string
+) {
+  return getPropertyName(node) === name;
+}
+
+export function isCalleeObject(node: ESTree.CallExpression, name: string) {
   return (
     node.callee.type === 'MemberExpression' &&
     isIdentifier(node.callee.object, name)
@@ -27,7 +58,7 @@ export function isObject(node: ESTree.CallExpression, name: string) {
 export function isCalleeProperty(node: ESTree.CallExpression, name: string) {
   return (
     node.callee.type === 'MemberExpression' &&
-    isIdentifier(node.callee.property, name)
+    isPropertyAccessor(node.callee, name)
   );
 }
 
@@ -45,23 +76,6 @@ export function isObjectProperty(node: ESTree.MemberExpression, name: string) {
   );
 }
 
-function isLiteral<T>(node: ESTree.Node, type: string, value?: T) {
-  return (
-    node.type === 'Literal' &&
-    (value === undefined
-      ? typeof node.value === type
-      : (node.value as any) === value)
-  );
-}
-
-export function isStringLiteral(node: ESTree.Node, value?: string) {
-  return isLiteral(node, 'string', value);
-}
-
-export function isBooleanLiteral(node: ESTree.Node, value?: boolean) {
-  return isLiteral(node, 'boolean', value);
-}
-
 const describeProperties = new Set([
   'parallel',
   'serial',
@@ -70,10 +84,6 @@ const describeProperties = new Set([
   'fixme',
 ]);
 
-function isDescribeProperty(node: ESTree.Node) {
-  return describeProperties.has(getNodeName(node) ?? '');
-}
-
 export function isDescribeCall(node: ESTree.Node): boolean {
   const inner = node.type === 'CallExpression' ? node.callee : node;
 
@@ -88,25 +98,19 @@ export function isDescribeCall(node: ESTree.Node): boolean {
 
   return isIdentifier(inner.property, 'describe')
     ? true
-    : isDescribeProperty(inner.property)
+    : describeProperties.has(getPropertyName(inner))
     ? isDescribeCall(inner.object)
     : false;
 }
 
-type NodeWithParent<T extends ESTree.Node['type']> = Extract<
-  ESTree.Node,
-  { type: T }
-> &
-  Rule.NodeParentExtension;
-
 export function findParent<T extends ESTree.Node['type']>(
-  node: ESTree.Node & Rule.NodeParentExtension,
+  node: NodeWithParent,
   type: T
-): NodeWithParent<T> | undefined {
+): TypedNodeWithParent<T> | undefined {
   if (!node.parent) return;
 
   return node.parent.type === type
-    ? (node.parent as unknown as NodeWithParent<T>)
+    ? (node.parent as unknown as TypedNodeWithParent<T>)
     : findParent(node.parent, type);
 }
 
@@ -152,12 +156,3 @@ export function getMatchers(
 
   return chain;
 }
-
-export function isPropertyAccessor(
-  node: ESTree.MemberExpression,
-  name: string
-) {
-  return (
-    isIdentifier(node.property, name) || isStringLiteral(node.property, name)
-  );
-}

src/utils/types.ts

@@ -2,3 +2,9 @@ import { Rule } from 'eslint';
 import * as ESTree from 'estree';
 
 export type NodeWithParent = ESTree.Node & Rule.NodeParentExtension;
+
+export type TypedNodeWithParent<T extends ESTree.Node['type']> = Extract<
+  ESTree.Node,
+  { type: T }
+> &
+  Rule.NodeParentExtension;

test/spec/no-element-handle.spec.ts

@@ -8,7 +8,7 @@ const invalid = (code: string, output: string) => ({
       messageId: 'noElementHandle',
       suggestions: [
         {
-          messageId: code.includes('page.$$')
+          messageId: code.includes('$$')
             ? 'replaceElementHandlesWithLocator'
             : 'replaceElementHandleWithLocator',
           output: wrapInTest(output),
@@ -27,6 +27,14 @@ runRuleTester('no-element-handle', rule, {
       'const handle = await page.$("text=Submit");',
       'const handle = page.locator("text=Submit");'
     ),
+    invalid(
+      'const handle = await page["$$"]("text=Submit");',
+      'const handle = page.locator("text=Submit");'
+    ),
+    invalid(
+      'const handle = await page[`$$`]("text=Submit");',
+      'const handle = page.locator("text=Submit");'
+    ),
 
     // element handle as let
     invalid(
@@ -54,6 +62,14 @@ runRuleTester('no-element-handle', rule, {
       'const handles = await page.$$("a")',
       'const handles = page.locator("a")'
     ),
+    invalid(
+      'const handle = await page["$$"]("a");',
+      'const handle = page.locator("a");'
+    ),
+    invalid(
+      'const handle = await page[`$$`]("a");',
+      'const handle = page.locator("a");'
+    ),
 
     // element handles as let
     invalid(
@@ -95,28 +111,15 @@ runRuleTester('no-element-handle', rule, {
     ),
   ],
   valid: [
-    // page locator
     valid('page.locator("a")'),
-
-    // page locator with action
     valid('await page.locator("a").click();'),
-
-    // const $
     valid('const $ = "text";'),
-
-    // $ as a method
     valid('$("a");'),
-
-    // this.$ as a method
     valid('this.$("a");'),
-
-    // internalPage.$ method
+    valid('this["$"]("a");'),
+    valid('this[`$`]("a");'),
     valid('internalPage.$("a");'),
-
-    // this.page.$$$ method
     valid('this.page.$$$("div");'),
-
-    // page.$$$ method
     valid('page.$$$("div");'),
   ],
 });