Skip to content

Commit 087bf3c

Browse files
feat(no-unbound-methods): allow unbound signals (#211)
Adds a new option `allowTypes` to the rule `no-unbound-methods` so users may specify specific types that are safe to pass unbound. By default, we set it to `Signal` to support Angular's signals out of the box. Resolves #209 and upstream cartant#137
1 parent 2d1bcd1 commit 087bf3c

File tree

4 files changed

+119
-8
lines changed

4 files changed

+119
-8
lines changed

docs/rules/no-unbound-methods.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
This rule effects failures if unbound methods are passed as callbacks.
1010

11+
This rule is aware of Angular's `Signal` type by default
12+
which can be safely passed unbound.
13+
1114
## Rule details
1215

1316
Examples of **incorrect** code for this rule:
@@ -41,6 +44,16 @@ return this.http
4144
);
4245
```
4346

47+
## Options
48+
49+
<!-- begin auto-generated rule options list -->
50+
51+
| Name | Description | Type | Default |
52+
| :----------- | :---------------------------------------------------------------- | :------- | :--------- |
53+
| `allowTypes` | An array of function types that are allowed to be passed unbound. | String[] | [`Signal`] |
54+
55+
<!-- end auto-generated rule options list -->
56+
4457
## When Not To Use It
4558

4659
If every handler in your project does not depend on the `this` context in their implementations,

src/constants.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ export const SOURCES_OBJECT_ACCEPTING_STATIC_OBSERVABLE_CREATORS = [
99
'forkJoin',
1010
];
1111

12+
/**
13+
* The names of types that are allowed to be passed unbound.
14+
*/
15+
export const DEFAULT_UNBOUND_ALLOWED_TYPES = [
16+
'Signal',
17+
];
18+
1219
/**
1320
* The names of operators that are safe to be used after
1421
* operators like `takeUntil` that complete the observable.

src/rules/no-unbound-methods.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,19 @@
11
import { TSESTree as es, ESLintUtils } from '@typescript-eslint/utils';
2+
import { DEFAULT_UNBOUND_ALLOWED_TYPES } from '../constants';
23
import {
4+
couldBeType,
35
getTypeServices,
46
isCallExpression,
5-
isMemberExpression } from '../etc';
7+
isMemberExpression,
8+
} from '../etc';
69
import { ruleCreator } from '../utils';
710

11+
const defaultOptions: readonly {
12+
allowTypes?: string[];
13+
}[] = [];
14+
815
export const noUnboundMethodsRule = ruleCreator({
9-
defaultOptions: [],
16+
defaultOptions,
1017
meta: {
1118
docs: {
1219
description: 'Disallow passing unbound methods.',
@@ -16,18 +23,33 @@ export const noUnboundMethodsRule = ruleCreator({
1623
messages: {
1724
forbidden: 'Unbound methods are forbidden.',
1825
},
19-
schema: [],
26+
schema: [
27+
{
28+
properties: {
29+
allowTypes: { type: 'array', items: { type: 'string' }, description: 'An array of function types that are allowed to be passed unbound.', default: DEFAULT_UNBOUND_ALLOWED_TYPES },
30+
},
31+
type: 'object',
32+
},
33+
],
2034
type: 'problem',
2135
},
2236
name: 'no-unbound-methods',
2337
create: (context) => {
2438
const { getTypeAtLocation } = ESLintUtils.getParserServices(context);
2539
const { couldBeObservable, couldBeSubscription } = getTypeServices(context);
2640
const nodeMap = new WeakMap<es.Node, void>();
41+
const [config = {}] = context.options;
42+
const { allowTypes = [] } = config;
2743

2844
function mapArguments(node: es.CallExpression | es.NewExpression) {
2945
node.arguments.filter(isMemberExpression).forEach((arg) => {
3046
const argType = getTypeAtLocation(arg);
47+
48+
// Skip if the type matches any of the allowed types.
49+
if (allowTypes.some(t => couldBeType(argType, t))) {
50+
return;
51+
}
52+
3153
if (argType.getCallSignatures().length > 0) {
3254
nodeMap.set(arg);
3355
}

tests/rules/no-unbound-methods.test.ts

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
1-
import { InvalidTestCase, ValidTestCase } from '@typescript-eslint/rule-tester';
1+
import { RunTests } from '@typescript-eslint/rule-tester';
22
import { stripIndent } from 'common-tags';
33
import { noUnboundMethodsRule } from '../../src/rules/no-unbound-methods';
44
import { fromFixture } from '../etc';
55
import { ruleTester } from '../rule-tester';
66

7-
interface Tests {
8-
valid: (string | ValidTestCase<never>)[];
9-
invalid: InvalidTestCase<keyof typeof noUnboundMethodsRule['meta']['messages'], never>[];
10-
}
7+
type Tests = RunTests<
8+
keyof typeof noUnboundMethodsRule['meta']['messages'],
9+
typeof noUnboundMethodsRule['defaultOptions']
10+
>;
1111

1212
const arrowTests: Tests = {
1313
valid: [
@@ -266,19 +266,88 @@ const unboundTests: Tests = {
266266
],
267267
};
268268

269+
const allowTypesTests: Tests = {
270+
valid: [
271+
{
272+
code: stripIndent`
273+
// allowed types
274+
import { of, tap } from "rxjs";
275+
276+
interface Signal<T> extends Function {
277+
(): T;
278+
}
279+
interface WritableSignal<T> extends Signal<T> {
280+
set(value: T): void;
281+
}
282+
283+
function customLog<T>(signal: Signal<T>) {
284+
return tap((value: T) => console.log(value, signal()));
285+
}
286+
function customSet<T>(signal: WritableSignal<T>) {
287+
return tap((value: T) => signal.set(value));
288+
}
289+
290+
class Something {
291+
private readonly x: Signal<string>;
292+
private readonly y: WritableSignal<number>;
293+
294+
constructor() {
295+
of(1).pipe(
296+
customLog(this.x),
297+
customSet(this.y),
298+
).subscribe();
299+
}
300+
}
301+
`,
302+
options: [{
303+
allowTypes: ['Signal'],
304+
}],
305+
},
306+
],
307+
invalid: [
308+
fromFixture(
309+
stripIndent`
310+
// unbound signal without allowed types
311+
import { of, tap } from "rxjs";
312+
313+
interface Signal<T> extends Function {
314+
(): T;
315+
}
316+
317+
function customOperator<T>(signal: Signal<T>) {
318+
return tap((value: T) => console.log(value, signal()));
319+
}
320+
321+
class Something {
322+
private readonly x: Signal<string>;
323+
324+
constructor() {
325+
of(1).pipe(
326+
customOperator(this.x),
327+
~~~~~~ [forbidden]
328+
).subscribe();
329+
}
330+
}
331+
`,
332+
),
333+
],
334+
};
335+
269336
ruleTester({ types: true }).run('no-unbound-methods', noUnboundMethodsRule, {
270337
valid: [
271338
...arrowTests.valid,
272339
...boundTests.valid,
273340
...deepTests.valid,
274341
...staticTests.valid,
275342
...unboundTests.valid,
343+
...allowTypesTests.valid,
276344
],
277345
invalid: [
278346
...arrowTests.invalid,
279347
...boundTests.invalid,
280348
...deepTests.invalid,
281349
...staticTests.invalid,
282350
...unboundTests.invalid,
351+
...allowTypesTests.invalid,
283352
],
284353
});

0 commit comments

Comments
 (0)