Add new "no element handle" rule (#40)

* rule: add new noElementHandle rule

* chore: move end range to const

* fix: getRange method for non-awaiting statements

* test: add additional tests for the noElementHandle rule

* chore: improve noElementHandle rule

* docs: update the wording of noElementHandle rule

* rule: update message to match with other rule naming conventions

* rule: change noElementHandle to warn level

* rule: noElementHandle change fixable to suggestion

* test: add valid tests for noElementHandle rule

* rule: change type for noElementHandle to suggestion

* rule: update noElementHandle rule's message for suggestions

* rule: update noElementHandle with getRange method

Author: elaichenkov

README.md

@@ -98,3 +98,24 @@ Example of **correct** code for this rule:
 await page.click('button');
 ```
 
+### `no-element-handle`
+
+Disallow the creation of element handles with `page.$` or `page.$$`.
+
+Examples of **incorrect** code for this rule:
+
+```js
+// Element Handle
+const buttonHandle = await page.$('button');
+await buttonHandle.click();
+
+// Element Handles
+const linkHandles = await page.$$('a');
+```
+
+Example of **correct** code for this rule:
+
+```js
+const buttonLocator = page.locator('button');
+await buttonLocator.click();
+```

lib/index.js

@@ -1,5 +1,6 @@
 const missingPlaywrightAwait = require("./rules/missing-playwright-await");
 const noPagePause = require("./rules/no-page-pause");
+const noElementHandle = require("./rules/no-element-handle");
 
 module.exports = {
   configs: {
@@ -12,6 +13,7 @@ module.exports = {
         "no-empty-pattern": "off",
         "playwright/missing-playwright-await": "error",
         "playwright/no-page-pause": "warn",
+        "playwright/no-element-handle": "warn",
       },
     },
     "jest-playwright": {
@@ -50,5 +52,6 @@ module.exports = {
   rules: {
     "missing-playwright-await": missingPlaywrightAwait,
     "no-page-pause": noPagePause,
+    "no-element-handle": noElementHandle
   },
 };

lib/rules/no-element-handle.js

@@ -0,0 +1,72 @@
+function isPageIdentifier({ callee }) {
+  return (
+    callee &&
+    callee.type === 'MemberExpression' &&
+    callee.object.type === 'Identifier' &&
+    callee.object.name === 'page'
+  );
+}
+
+function isElementHandleIdentifier({ callee }) {
+  return (
+    callee &&
+    callee.property &&
+    callee.property.type === 'Identifier' &&
+    callee.property.name === '$'
+  );
+}
+
+function isElementHandlesIdentifier({ callee }) {
+  return (
+    callee &&
+    callee.property &&
+    callee.property.type === 'Identifier' &&
+    callee.property.name === '$$'
+  );
+}
+
+function getRange(node) {
+  const start = node.parent && node.parent.type === 'AwaitExpression' 
+    ? node.parent.range[0]
+    : node.callee.object.range[0];
+
+  return [start, node.callee.property.range[1]];
+}
+
+module.exports = {
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (isPageIdentifier(node) && (isElementHandleIdentifier(node) || isElementHandlesIdentifier(node))) {
+          context.report({
+            messageId: 'noElementHandle',
+            suggest: [
+              {
+                messageId: isElementHandleIdentifier(node)
+                  ? 'replaceElementHandleWithLocator'
+                  : 'replaceElementHandlesWithLocator',
+                fix: (fixer) => fixer.replaceTextRange(getRange(node), 'page.locator'),
+              },
+            ],
+            node,
+          });
+        }
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Possible Errors',
+      description: 'The use of ElementHandle is discouraged, use Locator instead',
+      recommended: true,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright#no-element-handle',
+    },
+    hasSuggestions: true,
+    messages: {
+      noElementHandle: 'Unexpected use of element handles.',
+      replaceElementHandleWithLocator: 'Replace `page.$` with `page.locator`',
+      replaceElementHandlesWithLocator: 'Replace `page.$$` with `page.locator`',
+    },
+    type: 'suggestion',
+  },
+};

test/no-element-handle.spec.js

@@ -0,0 +1,106 @@
+const { RuleTester } = require('eslint');
+const rule = require('../lib/rules/no-element-handle');
+
+RuleTester.setDefaultConfig({
+  parserOptions: {
+    ecmaVersion: 2018,
+  },
+});
+
+const wrapInTest = (input) => `test('verify noElementHandle rule', async () => { ${input} })`;
+
+const invalid = (code, output) => ({
+  code: wrapInTest(code),
+  errors: [
+    {
+      messageId: 'noElementHandle',
+      suggestions: [
+        {
+          messageId: code.includes('page.$$') ? 'replaceElementHandlesWithLocator' : 'replaceElementHandleWithLocator',
+          output: wrapInTest(output),
+        },
+      ],
+    },
+  ],
+});
+
+const valid = (code) => ({
+  code: wrapInTest(code),
+});
+
+new RuleTester().run('no-element-handle', rule, {
+  invalid: [
+    // element handle as const
+    invalid('const handle = await page.$("text=Submit");', 'const handle = page.locator("text=Submit");'),
+
+    // element handle as let
+    invalid('let handle = await page.$("text=Submit");', 'let handle = page.locator("text=Submit");'),
+
+    // element handle as expression statement without await
+    invalid('page.$("div")', 'page.locator("div")'),
+
+    // element handles as expression statement without await
+    invalid('page.$$("div")', 'page.locator("div")'),
+
+    // element handle as expression statement
+    invalid('await page.$("div")', 'page.locator("div")'),
+
+    // element handle click
+    invalid('await (await page.$$("div")).click();', 'await (page.locator("div")).click();'),
+
+    // element handles as const
+    invalid('const handles = await page.$$("a")', 'const handles = page.locator("a")'),
+
+    // element handles as let
+    invalid('let handles = await page.$$("a")', 'let handles = page.locator("a")'),
+
+    // element handles as expression statement
+    invalid('await page.$$("a")', 'page.locator("a")'),
+
+    // return element handle without awaiting it
+    invalid(
+      'function getHandle() { return page.$("button"); }',
+      'function getHandle() { return page.locator("button"); }'
+    ),
+
+    // return element handles without awaiting it
+    invalid(
+      'function getHandle() { return page.$$("button"); }',
+      'function getHandle() { return page.locator("button"); }'
+    ),
+
+    // missed return for the element handle
+    invalid('function getHandle() { page.$("button"); }', 'function getHandle() { page.locator("button"); }'),
+
+    // arrow function return element handle without awaiting it
+    invalid('const getHandles = () => page.$("links");', 'const getHandles = () => page.locator("links");'),
+
+    // arrow function return element handles without awaiting it
+    invalid('const getHandles = () => page.$$("links");', 'const getHandles = () => page.locator("links");'),
+  ],
+  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('internalPage.$("a");'),
+
+    // this.page.$$$ method
+    valid('this.page.$$$("div");'),
+
+    // page.$$$ method
+    valid('page.$$$("div");'),
+  ],
+});