Skip to content

Commit 7fce912

Browse files
JoshuaKGoldbergclaude
JoshuaKGoldberg
and
claude
committed
fix(eslint-plugin): [unbound-method] respect this: void on class properties
A class property initialized with a function expression was the only literal-function case that skipped `checkMethod`, so its own `this: void` annotation was ignored: ```ts const Foo = { floor: function (this: void) {} }; const { floor } = Foo; // correctly not reported class Foo { floor = function (this: void) {} } const { floor } = new Foo(); // reported, despite `this: void` ``` Routing it through `checkMethod` fixes that, and makes the two property cases identical enough to share a branch. `ts.isFunctionExpression` narrows the initializer for real, so the cast the old `.kind` check needed goes away. Skipping `checkMethod` also meant `firstParamIsThis` went uncomputed, so these reports never carried the message that suggests annotating with `this: void`. They do now, which moves 19 expectations - and leaves the plain `unbound` message covered only by a new case whose function expression annotates a non-`void` `this`. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
1 parent 7a8dc4d commit 7fce912

2 files changed

Lines changed: 48 additions & 31 deletions

File tree

‎packages/eslint-plugin/src/rules/unbound-method.ts‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -395,20 +395,14 @@ function checkIfMethod(
395395
}
396396

397397
switch (valueDeclaration.kind) {
398-
case ts.SyntaxKind.PropertyDeclaration:
399-
return {
400-
dangerous:
401-
(valueDeclaration as ts.PropertyDeclaration).initializer?.kind ===
402-
ts.SyntaxKind.FunctionExpression,
403-
};
404-
case ts.SyntaxKind.PropertyAssignment: {
405-
const assignee = (valueDeclaration as ts.PropertyAssignment).initializer;
406-
if (assignee.kind !== ts.SyntaxKind.FunctionExpression) {
407-
return {
408-
dangerous: false,
409-
};
398+
case ts.SyntaxKind.PropertyAssignment:
399+
case ts.SyntaxKind.PropertyDeclaration: {
400+
const { initializer } = valueDeclaration as
401+
ts.PropertyAssignment | ts.PropertyDeclaration;
402+
if (!initializer || !ts.isFunctionExpression(initializer)) {
403+
return { dangerous: false };
410404
}
411-
return checkMethod(assignee as ts.FunctionExpression, ignoreStatic);
405+
return checkMethod(initializer, ignoreStatic);
412406
}
413407
case ts.SyntaxKind.MethodDeclaration:
414408
case ts.SyntaxKind.MethodSignature: {

‎packages/eslint-plugin/tests/rules/unbound-method.test.ts‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,12 @@ const o = {
8282
const f = o.f;
8383
`,
8484
`
85+
class Foo {
86+
f = function (this: void) {};
87+
}
88+
const f = new Foo().f;
89+
`,
90+
`
8591
const { alert } = window;
8692
`,
8793
`
@@ -2327,7 +2333,7 @@ const foo = Foo;
23272333
endColumn: 33,
23282334
endLine: 6,
23292335
line: 6,
2330-
messageId: 'unbound',
2336+
messageId: 'unboundWithoutThisAnnotation',
23312337
},
23322338
],
23332339
},
@@ -2456,6 +2462,23 @@ const x = Foo.parse;
24562462
class Foo {
24572463
unbound = function () {};
24582464
}
2465+
const unbound = new Foo().unbound;
2466+
`,
2467+
errors: [
2468+
{
2469+
column: 17,
2470+
endColumn: 34,
2471+
endLine: 5,
2472+
line: 5,
2473+
messageId: 'unboundWithoutThisAnnotation',
2474+
},
2475+
],
2476+
},
2477+
{
2478+
code: `
2479+
class Foo {
2480+
unbound = function (this: Foo) {};
2481+
}
24592482
const unbound = new Foo().unbound;
24602483
`,
24612484
errors: [
@@ -2498,7 +2521,7 @@ const { unbound } = new Foo();
24982521
endColumn: 16,
24992522
endLine: 5,
25002523
line: 5,
2501-
messageId: 'unbound',
2524+
messageId: 'unboundWithoutThisAnnotation',
25022525
},
25032526
],
25042527
},
@@ -2534,7 +2557,7 @@ let unbound;
25342557
endColumn: 11,
25352558
endLine: 6,
25362559
line: 6,
2537-
messageId: 'unbound',
2560+
messageId: 'unboundWithoutThisAnnotation',
25382561
},
25392562
],
25402563
},
@@ -2551,7 +2574,7 @@ function foo({ unbound }: Foo = new Foo()) {}
25512574
endColumn: 23,
25522575
endLine: 5,
25532576
line: 5,
2554-
messageId: 'unbound',
2577+
messageId: 'unboundWithoutThisAnnotation',
25552578
},
25562579
],
25572580
},
@@ -2569,7 +2592,7 @@ function foo({ unbound }: Foo = bar) {}
25692592
endColumn: 23,
25702593
endLine: 6,
25712594
line: 6,
2572-
messageId: 'unbound',
2595+
messageId: 'unboundWithoutThisAnnotation',
25732596
},
25742597
],
25752598
},
@@ -2587,7 +2610,7 @@ function foo({ unbound }: Foo = { unbound: () => {} }) {}
25872610
endColumn: 23,
25882611
endLine: 6,
25892612
line: 6,
2590-
messageId: 'unbound',
2613+
messageId: 'unboundWithoutThisAnnotation',
25912614
},
25922615
],
25932616
},
@@ -2622,7 +2645,7 @@ function foo({ unbound }: Foo) {}
26222645
endColumn: 23,
26232646
endLine: 5,
26242647
line: 5,
2625-
messageId: 'unbound',
2648+
messageId: 'unboundWithoutThisAnnotation',
26262649
},
26272650
],
26282651
},
@@ -2640,7 +2663,7 @@ bar(({ unbound }) => {});
26402663
endColumn: 15,
26412664
endLine: 6,
26422665
line: 6,
2643-
messageId: 'unbound',
2666+
messageId: 'unboundWithoutThisAnnotation',
26442667
},
26452668
],
26462669
},
@@ -2658,7 +2681,7 @@ bar(({ unbound } = new Foo()) => {});
26582681
endColumn: 15,
26592682
endLine: 6,
26602683
line: 6,
2661-
messageId: 'unbound',
2684+
messageId: 'unboundWithoutThisAnnotation',
26622685
},
26632686
],
26642687
},
@@ -2676,7 +2699,7 @@ for (const { unbound } of [new Foo(), new Foo()]) {
26762699
endColumn: 21,
26772700
endLine: 5,
26782701
line: 5,
2679-
messageId: 'unbound',
2702+
messageId: 'unboundWithoutThisAnnotation',
26802703
},
26812704
],
26822705
},
@@ -2694,7 +2717,7 @@ class Foo {
26942717
endColumn: 16,
26952718
endLine: 5,
26962719
line: 5,
2697-
messageId: 'unbound',
2720+
messageId: 'unboundWithoutThisAnnotation',
26982721
},
26992722
],
27002723
},
@@ -2714,7 +2737,7 @@ function foo({ unbound }: Foo | Bar) {}
27142737
endColumn: 23,
27152738
endLine: 8,
27162739
line: 8,
2717-
messageId: 'unbound',
2740+
messageId: 'unboundWithoutThisAnnotation',
27182741
},
27192742
],
27202743
},
@@ -2731,7 +2754,7 @@ function foo({ unbound }: { unbound: () => string } | Foo) {}
27312754
endColumn: 23,
27322755
endLine: 5,
27332756
line: 5,
2734-
messageId: 'unbound',
2757+
messageId: 'unboundWithoutThisAnnotation',
27352758
},
27362759
],
27372760
},
@@ -2751,7 +2774,7 @@ function foo({ unbound }: Foo | Bar) {}
27512774
endColumn: 23,
27522775
endLine: 8,
27532776
line: 8,
2754-
messageId: 'unbound',
2777+
messageId: 'unboundWithoutThisAnnotation',
27552778
},
27562779
],
27572780
},
@@ -2768,7 +2791,7 @@ const foo = ({ unbound }: Foo & { foo: () => 'bar' }) => {};
27682791
endColumn: 23,
27692792
endLine: 5,
27702793
line: 5,
2771-
messageId: 'unbound',
2794+
messageId: 'unboundWithoutThisAnnotation',
27722795
},
27732796
],
27742797
},
@@ -2788,7 +2811,7 @@ const foo = ({ unbound }: (Foo & { foo: () => 'bar' }) | Bar) => {};
27882811
endColumn: 23,
27892812
endLine: 8,
27902813
line: 8,
2791-
messageId: 'unbound',
2814+
messageId: 'unboundWithoutThisAnnotation',
27922815
},
27932816
],
27942817
},
@@ -2808,7 +2831,7 @@ const foo = ({ unbound }: Foo & Bar) => {};
28082831
endColumn: 23,
28092832
endLine: 8,
28102833
line: 8,
2811-
messageId: 'unbound',
2834+
messageId: 'unboundWithoutThisAnnotation',
28122835
},
28132836
],
28142837
},
@@ -2830,7 +2853,7 @@ const foo = ({ unbound, ...rest }: Foo & Bar) => {};
28302853
endColumn: 23,
28312854
endLine: 10,
28322855
line: 10,
2833-
messageId: 'unbound',
2856+
messageId: 'unboundWithoutThisAnnotation',
28342857
},
28352858
],
28362859
},

0 commit comments

Comments
 (0)