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

fix(eslint-plugin): [unbound-method] report on destructuring in function parameters #8952

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
69 changes: 55 additions & 14 deletions packages/eslint-plugin/src/rules/unbound-method.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ export default createRule<Options, MessageIds>({
function checkIfMethodAndReport(
node: TSESTree.Node,
symbol: ts.Symbol | undefined,
): void {
): boolean {
if (!symbol) {
return;
return false;
}

const { dangerous, firstParamIsThis } = checkIfMethod(
Expand All @@ -152,7 +152,9 @@ export default createRule<Options, MessageIds>({
: 'unbound',
node,
});
return true;
}
return false;
}

return {
Expand All @@ -173,22 +175,25 @@ export default createRule<Options, MessageIds>({

checkIfMethodAndReport(node, services.getSymbolAtLocation(node));
},
'VariableDeclarator, AssignmentExpression'(
node: TSESTree.AssignmentExpression | TSESTree.VariableDeclarator,
): void {
const [idNode, initNode] =
node.type === AST_NODE_TYPES.VariableDeclarator
? [node.id, node.init]
: [node.left, node.right];

if (initNode && idNode.type === AST_NODE_TYPES.ObjectPattern) {
const rightSymbol = services.getSymbolAtLocation(initNode);
ObjectPattern(node): void {
let initNode: TSESTree.Node | null = null;
if (node.parent.type === AST_NODE_TYPES.VariableDeclarator) {
initNode = node.parent.init;
} else if (
node.parent.type === AST_NODE_TYPES.AssignmentPattern ||
node.parent.type === AST_NODE_TYPES.AssignmentExpression
) {
initNode = node.parent.right;
}
kirkwaiblinger marked this conversation as resolved.
Show resolved Hide resolved

if (initNode) {
const initSymbol = services.getSymbolAtLocation(initNode);
const initTypes = services.getTypeAtLocation(initNode);

const notImported =
rightSymbol && isNotImported(rightSymbol, currentSourceFile);
initSymbol && isNotImported(initSymbol, currentSourceFile);

idNode.properties.forEach(property => {
node.properties.forEach(property => {
if (
property.type === AST_NODE_TYPES.Property &&
property.key.type === AST_NODE_TYPES.Identifier
Expand All @@ -209,6 +214,42 @@ export default createRule<Options, MessageIds>({
);
}
});
return;
auvred marked this conversation as resolved.
Show resolved Hide resolved
}

if (node.typeAnnotation) {
auvred marked this conversation as resolved.
Show resolved Hide resolved
const subtypes = tsutils.unionTypeParts(
services.getTypeAtLocation(node.typeAnnotation.typeAnnotation),
);
for (const property of node.properties) {
for (const subtype of subtypes) {
if (
property.type !== AST_NODE_TYPES.Property ||
property.key.type !== AST_NODE_TYPES.Identifier
) {
continue;
}

const propertyKey = property.key;
if (
checkIfMethodAndReport(
auvred marked this conversation as resolved.
Show resolved Hide resolved
propertyKey,
subtype.getProperty(propertyKey.name),
) ||
(tsutils.isIntersectionType(subtype) &&
tsutils
.intersectionTypeParts(subtype)
.some(intersectionSubtype =>
checkIfMethodAndReport(
auvred marked this conversation as resolved.
Show resolved Hide resolved
propertyKey,
intersectionSubtype.getProperty(propertyKey.name),
),
))
) {
break;
}
}
}
}
},
};
Expand Down
224 changes: 224 additions & 0 deletions packages/eslint-plugin/tests/rules/unbound-method.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,43 @@ class Foo {
bound = () => 'foo';
}
const { bound } = new Foo();
`,
`
class Foo {
bound = () => 'foo';
}
function foo({ bound } = new Foo()) {}
`,
`
class Foo {
bound = () => 'foo';
}
declare const bar: Foo;
function foo({ bound }: Foo) {}
`,
`
class Foo {
bound = () => 'foo';
}
class Bar {
bound = () => 'bar';
}
function foo({ bound }: Foo | Bar) {}
`,
`
class Foo {
bound = () => 'foo';
}
type foo = ({ bound }: Foo) => void;
`,
`
class Foo {
bound = () => 'foo';
}
class Bar {
bound = () => 'bar';
}
function foo({ bound }: Foo & Bar) {}
`,
// https://github.com/typescript-eslint/typescript-eslint/issues/1866
`
Expand Down Expand Up @@ -516,6 +553,193 @@ let unbound;
},
{
code: `
class Foo {
unbound = function () {};
}
function foo({ unbound }: Foo = new Foo()) {}
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
declare const bar: Foo;
function foo({ unbound }: Foo = bar) {}
`,
errors: [
{
line: 6,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
function foo({ unbound }: Foo) {}
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};

foo({ unbound }: Foo) {}
}
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
type foo = ({ unbound }: Foo) => void;
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
class Bar {
unbound = function () {};
}
function foo({ unbound }: Foo | Bar) {}
`,
errors: [
{
line: 8,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
function foo({ unbound }: { unbound: () => string } | Foo) {}
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
class Bar {
unbound = () => {};
}
function foo({ unbound }: Foo | Bar) {}
`,
errors: [
{
line: 8,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
type foo = ({ unbound }: Foo & { foo: () => 'bar' }) => void;
`,
errors: [
{
line: 5,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
class Bar {
unbound = () => {};
}
type foo = ({ unbound }: (Foo & { foo: () => 'bar' }) | Bar) => void;
`,
errors: [
{
line: 8,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};
}
class Bar {
unbound = () => {};
}
type foo = ({ unbound }: Foo & Bar) => void;
`,
errors: [
{
line: 8,
messageId: 'unbound',
},
],
},
{
code: `
class Foo {
unbound = function () {};

other = function () {};
}
class Bar {
unbound = () => {};
}
type foo = ({ unbound, ...rest }: Foo & Bar) => void;
`,
errors: [
{
line: 10,
messageId: 'unbound',
},
],
},
{
code: `
class CommunicationError {
foo() {}
}
Expand Down