Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefer to have length #94

Merged
merged 2 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
23 changes: 23 additions & 0 deletions docs/rules/prefer-to-have-length.md
Original file line number Diff line number Diff line change
@@ -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);
```
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -84,5 +85,6 @@ export = {
'valid-expect': validExpect,
'prefer-lowercase-title': preferLowercaseTitle,
'require-top-level-describe': requireTopLevelDescribe,
'prefer-to-have-length': preferToHaveLength,
},
};
63 changes: 63 additions & 0 deletions src/rules/prefer-to-have-length.ts
Original file line number Diff line number Diff line change
@@ -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;
20 changes: 20 additions & 0 deletions src/utils/ast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}
52 changes: 52 additions & 0 deletions test/spec/prefer-to-have-length.spec.ts
Original file line number Diff line number Diff line change
@@ -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 }],
},
],
});