Skip to content

Commit b1f4ed3

Browse files
G-RathG-Rath
authoredMar 13, 2021
feat(unbound-method): create rule (#765)
* feat(unbound-method): add original `unbound-method` rule * feat(unbound-method): modify rule for `jest` * refactor(unbound-method): extend base rule * docs(readme): add section about type-based rules * chore: add `@typescript-eslint/eslint-plugin` as an optional peer dep * fix(unbound-method): re-throw errors that are not `MODULE_NOT_FOUND` * chore(unbound-method): use early return instead of empty string * test(unbound-method): adjust test * build: ignore test fixtures * test: add end of file newline to fixture * docs(unbound-method): mention `@typescript-eslint/eslint-plugin` dep * refactor(unbound-method): improve method & variable name
·
v28.11.0v24.3.0
1 parent 712a56e commit b1f4ed3

22 files changed

+2067
-8
lines changed
 

‎.eslintignore

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
coverage/
22
lib/
33
!.eslintrc.js
4+
src/rules/__tests__/fixtures/

‎README.md

+27-2
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ installations requiring long-term consistency.
128128

129129
## Rules
130130

131-
<!-- begin rules list -->
131+
<!-- begin base rules list -->
132132

133133
| Rule | Description | Configurations | Fixable |
134134
| ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ |
@@ -173,7 +173,32 @@ installations requiring long-term consistency.
173173
| [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | |
174174
| [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] |
175175

176-
<!-- end rules list -->
176+
<!-- end base rules list -->
177+
178+
## TypeScript Rules
179+
180+
In addition to the above rules, this plugin also includes a few advanced rules
181+
that are powered by type-checking information provided by TypeScript.
182+
183+
In order to use these rules, you must be using `@typescript-eslint/parser` &
184+
adjust your eslint config as outlined
185+
[here](https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/TYPED_LINTING.md)
186+
187+
Note that unlike the type-checking rules in `@typescript-eslint/eslint-plugin`,
188+
the rules here will fallback to doing nothing if type information is not
189+
available, meaning its safe to include them in shared configs that could be used
190+
on JavaScript and TypeScript projects.
191+
192+
Also note that `unbound-method` depends on `@typescript-eslint/eslint-plugin`,
193+
as it extends the original `unbound-method` rule from that plugin.
194+
195+
<!-- begin type rules list -->
196+
197+
| Rule | Description | Configurations | Fixable |
198+
| ---------------------------------------------- | ------------------------------------------------------------- | -------------- | ------- |
199+
| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | |
200+
201+
<!-- end type rules list -->
177202

178203
## Credit
179204

‎babel.config.js

+1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ module.exports = {
77
'@babel/preset-typescript',
88
['@babel/preset-env', { targets: { node: 10 } }],
99
],
10+
ignore: ['src/**/__tests__/fixtures/**'],
1011
};

‎docs/rules/unbound-method.md

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
# Enforces unbound methods are called with their expected scope (`unbound-method`)
2+
3+
## Rule Details
4+
5+
This rule extends the base [`@typescript-eslint/unbound-method`][original-rule]
6+
rule, meaning you must depend on `@typescript-eslint/eslint-plugin` for it to
7+
work. It adds support for understanding when it's ok to pass an unbound method
8+
to `expect` calls.
9+
10+
See the [`@typescript-eslint` documentation][original-rule] for more details on
11+
the `unbound-method` rule.
12+
13+
Note that while this rule requires type information to work, it will fail
14+
silently when not available allowing you to safely enable it on projects that
15+
are not using TypeScript.
16+
17+
## How to use
18+
19+
```json5
20+
{
21+
parser: '@typescript-eslint/parser',
22+
parserOptions: {
23+
project: 'tsconfig.json',
24+
ecmaVersion: 2020,
25+
sourceType: 'module',
26+
},
27+
overrides: [
28+
{
29+
files: ['test/**'],
30+
extends: ['jest'],
31+
rules: {
32+
// you should turn the original rule off *only* for test files
33+
'@typescript-eslint/unbound-method': 'off',
34+
'jest/unbound-method': 'error',
35+
},
36+
},
37+
],
38+
rules: {
39+
'@typescript-eslint/unbound-method': 'error',
40+
},
41+
}
42+
```
43+
44+
This rule should be applied to your test files in place of the original rule,
45+
which should be applied to the rest of your codebase.
46+
47+
## Options
48+
49+
See [`@typescript-eslint/unbound-method`][original-rule] options.
50+
51+
<sup>Taken with ❤️ [from `@typescript-eslint` core][original-rule]</sup>
52+
53+
[original-rule]:
54+
https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md

‎package.json

+8-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,8 @@
6262
"displayName": "test",
6363
"testEnvironment": "node",
6464
"testPathIgnorePatterns": [
65-
"<rootDir>/lib/.*"
65+
"<rootDir>/lib/.*",
66+
"<rootDir>/src/rules/__tests__/fixtures/*"
6667
]
6768
},
6869
{
@@ -121,8 +122,14 @@
121122
"typescript": "^4.0.0"
122123
},
123124
"peerDependencies": {
125+
"@typescript-eslint/eslint-plugin": ">= 4",
124126
"eslint": ">=5"
125127
},
128+
"peerDependenciesMeta": {
129+
"@typescript-eslint/eslint-plugin": {
130+
"optional": true
131+
}
132+
},
126133
"engines": {
127134
"node": ">=10"
128135
},

‎src/__tests__/__snapshots__/rules.test.ts.snap

+1
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ Object {
4646
"jest/prefer-todo": "error",
4747
"jest/require-to-throw-message": "error",
4848
"jest/require-top-level-describe": "error",
49+
"jest/unbound-method": "error",
4950
"jest/valid-describe": "error",
5051
"jest/valid-expect": "error",
5152
"jest/valid-expect-in-promise": "error",

‎src/__tests__/rules.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
22
import { resolve } from 'path';
33
import plugin from '../';
44

5-
const numberOfRules = 44;
5+
const numberOfRules = 45;
66
const ruleNames = Object.keys(plugin.rules);
77
const deprecatedRules = Object.entries(plugin.rules)
88
.filter(([, rule]) => rule.meta.deprecated)

‎src/rules/__tests__/fixtures/class.ts

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// used by no-throw-literal test case to validate custom error
2+
export class Error {}
3+
4+
// used by unbound-method test case to test imports
5+
export const console = { log() {} };
6+
7+
// used by prefer-reduce-type-parameter to test native vs userland check
8+
export class Reducable {
9+
reduce() {}
10+
}
11+
12+
// used by no-implied-eval test function imports
13+
export class Function {}

‎src/rules/__tests__/fixtures/file.ts

Whitespace-only changes.

‎src/rules/__tests__/fixtures/foo.ts

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export type T = number;

‎src/rules/__tests__/fixtures/indent/indent-invalid-fixture-1.js

+530
Large diffs are not rendered by default.

‎src/rules/__tests__/fixtures/indent/indent-valid-fixture-1.js

+530
Large diffs are not rendered by default.

‎src/rules/__tests__/fixtures/react.tsx

Whitespace-only changes.

‎src/rules/__tests__/fixtures/tsconfig-withmeta.json

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"extends": "./tsconfig.json",
3+
"compilerOptions": {
4+
"emitDecoratorMetadata": true,
5+
}
6+
}

‎src/rules/__tests__/fixtures/tsconfig.json

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"compilerOptions": {
3+
"jsx": "preserve",
4+
"target": "es5",
5+
"module": "commonjs",
6+
"strict": true,
7+
"esModuleInterop": true,
8+
"lib": ["es2015", "es2017", "esnext"],
9+
"experimentalDecorators": true
10+
},
11+
"files": ["estree.ts"],
12+
"include": [
13+
"file.ts",
14+
"react.tsx"
15+
]
16+
}

‎src/rules/__tests__/fixtures/unstrict/file.ts

Whitespace-only changes.

‎src/rules/__tests__/fixtures/unstrict/react.tsx

Whitespace-only changes.

‎src/rules/__tests__/fixtures/unstrict/tsconfig.json

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
{
2+
"compilerOptions": {
3+
"jsx": "preserve",
4+
"target": "es5",
5+
"module": "commonjs",
6+
"strict": false,
7+
"esModuleInterop": true,
8+
"lib": ["es2015", "es2017", "esnext"],
9+
"experimentalDecorators": true
10+
},
11+
"include": [
12+
"file.ts",
13+
"react.tsx"
14+
]
15+
}

‎src/rules/__tests__/unbound-method.test.ts

+728
Large diffs are not rendered by default.

‎src/rules/unbound-method.ts

+111
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils';
2+
import { createRule, isExpectCall, parseExpectCall } from './utils';
3+
4+
const toThrowMatchers = [
5+
'toThrow',
6+
'toThrowError',
7+
'toThrowErrorMatchingSnapshot',
8+
'toThrowErrorMatchingInlineSnapshot',
9+
];
10+
11+
const isJestExpectToThrowCall = (node: TSESTree.CallExpression) => {
12+
if (!isExpectCall(node)) {
13+
return false;
14+
}
15+
16+
const { matcher } = parseExpectCall(node);
17+
18+
if (!matcher) {
19+
return false;
20+
}
21+
22+
return !toThrowMatchers.includes(matcher.name);
23+
};
24+
25+
const baseRule = (() => {
26+
try {
27+
// eslint-disable-next-line @typescript-eslint/no-require-imports
28+
const TSESLintPlugin = require('@typescript-eslint/eslint-plugin');
29+
30+
return TSESLintPlugin.rules['unbound-method'] as TSESLint.RuleModule<
31+
MessageIds,
32+
Options
33+
>;
34+
} catch (e: unknown) {
35+
const error = e as { code: string };
36+
37+
if (error.code === 'MODULE_NOT_FOUND') {
38+
return null;
39+
}
40+
41+
throw error;
42+
}
43+
})();
44+
45+
const tryCreateBaseRule = (
46+
context: Readonly<TSESLint.RuleContext<MessageIds, Options>>,
47+
) => {
48+
try {
49+
return baseRule?.create(context);
50+
} catch {
51+
return null;
52+
}
53+
};
54+
55+
interface Config {
56+
ignoreStatic: boolean;
57+
}
58+
59+
export type Options = [Config];
60+
61+
export type MessageIds = 'unbound';
62+
63+
export default createRule<Options, MessageIds>({
64+
defaultOptions: [{ ignoreStatic: false }],
65+
...baseRule,
66+
name: __filename,
67+
meta: {
68+
messages: {
69+
unbound: 'This rule requires `@typescript-eslint/eslint-plugin`',
70+
},
71+
schema: [],
72+
type: 'problem',
73+
...baseRule?.meta,
74+
docs: {
75+
category: 'Best Practices',
76+
description:
77+
'Enforces unbound methods are called with their expected scope',
78+
requiresTypeChecking: true,
79+
...baseRule?.meta.docs,
80+
recommended: false,
81+
},
82+
},
83+
create(context) {
84+
const baseSelectors = tryCreateBaseRule(context);
85+
86+
if (!baseSelectors) {
87+
return {};
88+
}
89+
90+
let inExpectToThrowCall = false;
91+
92+
return {
93+
...baseSelectors,
94+
CallExpression(node: TSESTree.CallExpression): void {
95+
inExpectToThrowCall = isJestExpectToThrowCall(node);
96+
},
97+
'CallExpression:exit'(node: TSESTree.CallExpression): void {
98+
if (inExpectToThrowCall && isJestExpectToThrowCall(node)) {
99+
inExpectToThrowCall = false;
100+
}
101+
},
102+
MemberExpression(node: TSESTree.MemberExpression): void {
103+
if (inExpectToThrowCall) {
104+
return;
105+
}
106+
107+
baseSelectors.MemberExpression?.(node);
108+
},
109+
};
110+
},
111+
});

‎tools/regenerate-docs.ts

+20-4
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ interface RuleDetails {
2222
name: string;
2323
description: string;
2424
fixable: FixType | false;
25+
requiresTypeChecking: boolean;
2526
}
2627

2728
type RuleModule = TSESLint.RuleModule<string, unknown[]> & {
@@ -63,9 +64,13 @@ const generateRulesListMarkdown = (details: RuleDetails[]): string =>
6364
.map(column => [...column, ' '].join('|'))
6465
.join('\n');
6566

66-
const updateRulesList = (details: RuleDetails[], markdown: string): string => {
67-
const listBeginMarker = `<!-- begin rules list -->`;
68-
const listEndMarker = `<!-- end rules list -->`;
67+
const updateRulesList = (
68+
listName: 'base' | 'type',
69+
details: RuleDetails[],
70+
markdown: string,
71+
): string => {
72+
const listBeginMarker = `<!-- begin ${listName} rules list -->`;
73+
const listEndMarker = `<!-- end ${listName} rules list -->`;
6974

7075
const listStartIndex = markdown.indexOf(listBeginMarker);
7176
const listEndIndex = markdown.indexOf(listEndMarker);
@@ -112,6 +117,7 @@ const details: RuleDetails[] = Object.keys(config.configs.all.rules)
112117
: rule.meta.docs.suggestion
113118
? 'suggest'
114119
: false,
120+
requiresTypeChecking: rule.meta.docs.requiresTypeChecking ?? false,
115121
}),
116122
);
117123

@@ -125,8 +131,18 @@ details.forEach(({ name, description }) => {
125131
fs.writeFileSync(pathToDoc, format(contents.join('\n')));
126132
});
127133

134+
const [baseRules, typeRules] = details.reduce<[RuleDetails[], RuleDetails[]]>(
135+
(groups, details) => {
136+
groups[details.requiresTypeChecking ? 1 : 0].push(details);
137+
138+
return groups;
139+
},
140+
[[], []],
141+
);
142+
128143
let readme = fs.readFileSync(pathTo.readme, 'utf8');
129144

130-
readme = updateRulesList(details, readme);
145+
readme = updateRulesList('base', baseRules, readme);
146+
readme = updateRulesList('type', typeRules, readme);
131147

132148
fs.writeFileSync(pathTo.readme, format(readme), 'utf8');

‎yarn.lock

+4
Original file line numberDiff line numberDiff line change
@@ -4602,7 +4602,11 @@ __metadata:
46024602
ts-node: ^9.0.0
46034603
typescript: ^4.0.0
46044604
peerDependencies:
4605+
"@typescript-eslint/eslint-plugin": ">= 4"
46054606
eslint: ">=5"
4607+
peerDependenciesMeta:
4608+
"@typescript-eslint/eslint-plugin":
4609+
optional: true
46064610
languageName: unknown
46074611
linkType: soft
46084612

0 commit comments

Comments
 (0)
Please sign in to comment.