Prefer to have length (#94)

* Add prefer-to-have-length * AST helper
playwright-community · Aug 23, 2022 · d0876ca · d0876ca
1 parent 7fc8e36
commit d0876ca
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 0 deletions.
diff --git a/README.md b/README.md
@@ -63,5 +63,6 @@ command line option.\
 |  ✔  | 🔧  |     | [no-useless-not](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-useless-not.md)                         | Disallow usage of `not` matchers when a specific matcher exists   |
 |  ✔  |     | 💡  | [no-wait-for-timeout](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/no-wait-for-timeout.md)               | Disallow usage of `page.waitForTimeout`                           |
 |     | 🔧  |     | [prefer-lowercase-title](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-lowercase-title.md)         | Enforce lowercase test names                                      |
+|     | 🔧  |     | [prefer-to-have-length](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md)           | Suggest using `toHaveLength()`                                    |
 |     |     |     | [require-top-level-describe](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `test.describe` block |
 |  ✔  |     |     | [valid-expect](https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/valid-expect.md)                             | Enforce valid `expect()` usage                                    |
diff --git a/docs/rules/prefer-to-have-length.md b/docs/rules/prefer-to-have-length.md
@@ -0,0 +1,23 @@
+# Suggest using `toHaveLength()` (`prefer-to-have-length`)
+
+In order to have a better failure message, `toHaveLength()` should be used upon
+asserting expectations on objects length property.
+
+## Rule details
+
+This rule triggers a warning if `toBe()`, `toEqual()` or `toStrictEqual()` is
+used to assert objects length property.
+
+The following patterns are considered warnings:
+
+```javascript
+expect(files.length).toBe(1);
+expect(files.length).toEqual(1);
+expect(files.length).toStrictEqual(1);
+```
+
+The following pattern is **not** a warning:
+
+```javascript
+expect(files).toHaveLength(1);
+```
diff --git a/src/index.ts b/src/index.ts
@@ -12,6 +12,7 @@ import noUselessNot from './rules/no-useless-not';
 import validExpect from './rules/valid-expect';
 import preferLowercaseTitle from './rules/prefer-lowercase-title';
 import requireTopLevelDescribe from './rules/require-top-level-describe';
+import preferToHaveLength from './rules/prefer-to-have-length';
 
 export = {
   configs: {
@@ -84,5 +85,6 @@ export = {
     'valid-expect': validExpect,
     'prefer-lowercase-title': preferLowercaseTitle,
     'require-top-level-describe': requireTopLevelDescribe,
+    'prefer-to-have-length': preferToHaveLength,
   },
 };
diff --git a/src/rules/prefer-to-have-length.ts b/src/rules/prefer-to-have-length.ts
@@ -0,0 +1,63 @@
+import { Rule } from 'eslint';
+import {
+  isExpectCall,
+  getNodeName,
+  getMatchers,
+  isPropertyAccessor,
+} from '../utils/ast';
+
+const matchers = new Set(['toBe', 'toEqual', 'toStrictEqual']);
+
+export default {
+  create(context) {
+    return {
+      CallExpression(node) {
+        if (!isExpectCall(node)) {
+          return;
+        }
+
+        const [argument] = node.arguments;
+        const [matcher] = getMatchers(node).slice(-1);
+
+        if (
+          !matcher ||
+          !matchers.has(getNodeName(matcher) ?? '') ||
+          argument?.type !== 'MemberExpression' ||
+          !isPropertyAccessor(argument, 'length')
+        ) {
+          return;
+        }
+
+        context.report({
+          fix(fixer) {
+            return [
+              // remove the "length" property accessor
+              fixer.removeRange([
+                argument.property.range![0] - 1,
+                argument.range![1],
+              ]),
+              // replace the current matcher with "toHaveLength"
+              fixer.replaceText(matcher, 'toHaveLength'),
+            ];
+          },
+          messageId: 'useToHaveLength',
+          node: matcher,
+        });
+      },
+    };
+  },
+  meta: {
+    docs: {
+      category: 'Best Practices',
+      description: 'Suggest using `toHaveLength()`',
+      recommended: false,
+      url: 'https://github.com/playwright-community/eslint-plugin-playwright/tree/main/docs/rules/prefer-to-have-length.md',
+    },
+    messages: {
+      useToHaveLength: 'Use toHaveLength() instead',
+    },
+    fixable: 'code',
+    type: 'suggestion',
+    schema: [],
+  },
+} as Rule.RuleModule;
diff --git a/src/utils/ast.ts b/src/utils/ast.ts
@@ -141,3 +141,23 @@ export function isExpectCall(node: ESTree.CallExpression) {
       expectSubCommands.has(node.callee.property.name))
   );
 }
+
+export function getMatchers(
+  node: ESTree.Node & Rule.NodeParentExtension,
+  chain: ESTree.Node[] = []
+): ESTree.Node[] {
+  if (node.parent.type === 'MemberExpression' && node.parent.object === node) {
+    return getMatchers(node.parent, [...chain, node.parent.property]);
+  }
+
+  return chain;
+}
+
+export function isPropertyAccessor(
+  node: ESTree.MemberExpression,
+  name: string
+) {
+  return (
+    isIdentifier(node.property, name) || isStringLiteral(node.property, name)
+  );
+}
diff --git a/test/spec/prefer-to-have-length.spec.ts b/test/spec/prefer-to-have-length.spec.ts
@@ -0,0 +1,52 @@
+import rule from '../../src/rules/prefer-to-have-length';
+import { runRuleTester } from '../utils/rule-tester';
+
+runRuleTester('prefer-to-have-length', rule, {
+  valid: [
+    'expect(files).toHaveLength(1)',
+    "expect(files.name).toBe('file')",
+    "expect(files['name']).toBe('file')",
+    "expect(files[`name`]).toBe('file')",
+    'expect(result).toBe(true)',
+    `expect(user.getUserName(5)).not.toEqual('Paul')`,
+    `expect(user.getUserName(5)).not.toEqual('Paul')`,
+    'expect(a)',
+  ],
+  invalid: [
+    {
+      code: 'expect(files.length).toBe(1)',
+      output: 'expect(files).toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 22, line: 1 }],
+    },
+    {
+      code: 'expect(files.length).not.toBe(1)',
+      output: 'expect(files).not.toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 26, line: 1 }],
+    },
+    {
+      code: 'expect(files["length"]).not.toBe(1)',
+      output: 'expect(files).not.toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 29, line: 1 }],
+    },
+    {
+      code: 'expect(files["length"]).not.toBe(1)',
+      output: 'expect(files).not.toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 29, line: 1 }],
+    },
+    {
+      code: 'expect(files.length).toEqual(1)',
+      output: 'expect(files).toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 22, line: 1 }],
+    },
+    {
+      code: 'expect(files.length).toStrictEqual(1)',
+      output: 'expect(files).toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 22, line: 1 }],
+    },
+    {
+      code: 'expect(files.length).not.toStrictEqual(1)',
+      output: 'expect(files).not.toHaveLength(1)',
+      errors: [{ messageId: 'useToHaveLength', column: 26, line: 1 }],
+    },
+  ],
+});