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

feat(eslint-plugin): [unbound-method] add support for methods with a this: void parameter #2796

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions packages/eslint-plugin/docs/rules/unbound-method.md
Expand Up @@ -23,6 +23,14 @@ myLog();

// This log might later be called with an incorrect scope
const { log } = instance;

// arith.double may refer to `this` internally
const arith = {
double(x: number): number {
return x * 2;
},
};
const { double } = arith;
```

Examples of **correct** code for this rule
Expand All @@ -45,6 +53,14 @@ logBound();
// .bind and lambdas will also add a correct scope
const dotBindLog = instance.logBound.bind(instance);
const innerLog = () => instance.logBound();

// arith.double explicitly declares that it does not refer to `this` internally
const arith = {
double(this: void, x: number): number {
return x * 2;
},
};
const { double } = arith;
```

## Options
Expand Down
25 changes: 19 additions & 6 deletions packages/eslint-plugin/src/rules/unbound-method.ts
Expand Up @@ -248,14 +248,27 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean {
ts.SyntaxKind.FunctionExpression
);
case ts.SyntaxKind.MethodDeclaration:
case ts.SyntaxKind.MethodSignature:
return !(
ignoreStatic &&
tsutils.hasModifier(
valueDeclaration.modifiers,
ts.SyntaxKind.StaticKeyword,
case ts.SyntaxKind.MethodSignature: {
const decl = valueDeclaration as
| ts.MethodDeclaration
| ts.MethodSignature;
const firstParam = decl.parameters[0];
const thisArgIsVoid =
firstParam?.name.kind === ts.SyntaxKind.Identifier &&
firstParam?.name.escapedText === 'this' &&
firstParam?.type?.kind === ts.SyntaxKind.VoidKeyword;

return (
!thisArgIsVoid &&
!(
ignoreStatic &&
tsutils.hasModifier(
valueDeclaration.modifiers,
ts.SyntaxKind.StaticKeyword,
)
)
);
}
}

return false;
Expand Down
16 changes: 12 additions & 4 deletions packages/eslint-plugin/tests/rules/unbound-method.test.ts
Expand Up @@ -25,6 +25,12 @@ class ContainsMethods {

let instance = new ContainsMethods();

const arith = {
double(this: void, x: number): number {
return x * 2;
}
};

${code}
`;
}
Expand All @@ -35,7 +41,7 @@ function addContainsMethodsClassInvalid(
code: addContainsMethodsClass(c),
errors: [
{
line: 12,
line: 18,
messageId: 'unbound',
},
],
Expand Down Expand Up @@ -153,6 +159,8 @@ ruleTester.run('unbound-method', rule, {
'if (!!instance.unbound) {}',
'void instance.unbound',
'delete instance.unbound',

'const { double } = arith;',
].map(addContainsMethodsClass),
`
interface RecordA {
Expand Down Expand Up @@ -300,15 +308,15 @@ function foo(arg: ContainsMethods | null) {
`),
errors: [
{
line: 14,
line: 20,
messageId: 'unbound',
},
{
line: 15,
line: 21,
messageId: 'unbound',
},
{
line: 16,
line: 22,
messageId: 'unbound',
},
],
Expand Down