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] exempt all non-Promise built-in statics #8096

Merged
merged 2 commits into from Dec 25, 2023
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
1 change: 1 addition & 0 deletions .cspell.json
Expand Up @@ -121,6 +121,7 @@
"stringification",
"stringifying",
"stringly",
"subclassing",
"superset",
"thenables",
"transpiled",
Expand Down
92 changes: 28 additions & 64 deletions packages/eslint-plugin/src/rules/unbound-method.ts
Expand Up @@ -24,51 +24,16 @@ export type Options = [Config];
export type MessageIds = 'unbound' | 'unboundWithoutThisAnnotation';

/**
* The following is a list of exceptions to the rule
* Generated via the following script.
* This is statically defined to save making purposely invalid calls every lint run
* ```
SUPPORTED_GLOBALS.flatMap(namespace => {
const object = window[namespace];
return Object.getOwnPropertyNames(object)
.filter(
name =>
!name.startsWith('_') &&
typeof object[name] === 'function',
)
.map(name => {
try {
const x = object[name];
x();
} catch (e) {
if (e.message.includes("called on non-object")) {
return `${namespace}.${name}`;
}
}
});
}).filter(Boolean);
* ```
* Static methods on these globals are either not `this`-aware or supported being
* called without `this`.
*
* - `Promise` is not in the list because it supports subclassing by using `this`
* - `Array` is in the list because although it supports subclassing, the `this`
* value defaults to `Array` when unbound
*
* This is now a language-design invariant: static methods are never `this`-aware
* because TC39 wants to make `array.map(Class.method)` work!
*/
const nativelyNotBoundMembers = new Set([
'Promise.all',
'Promise.race',
'Promise.resolve',
'Promise.reject',
'Promise.allSettled',
'Object.defineProperties',
'Object.defineProperty',
'Reflect.defineProperty',
'Reflect.deleteProperty',
'Reflect.get',
'Reflect.getOwnPropertyDescriptor',
'Reflect.getPrototypeOf',
'Reflect.has',
'Reflect.isExtensible',
'Reflect.ownKeys',
'Reflect.preventExtensions',
'Reflect.set',
'Reflect.setPrototypeOf',
]);
const SUPPORTED_GLOBALS = [
'Number',
'Object',
Expand All @@ -78,31 +43,30 @@ const SUPPORTED_GLOBALS = [
'Array',
'Proxy',
'Date',
'Infinity',
'Atomics',
'Reflect',
'console',
'Math',
'JSON',
'Intl',
] as const;
const nativelyBoundMembers = SUPPORTED_GLOBALS.map(namespace => {
if (!(namespace in global)) {
// node.js might not have namespaces like Intl depending on compilation options
// https://nodejs.org/api/intl.html#intl_options_for_building_node_js
return [];
}
const object = global[namespace];
return Object.getOwnPropertyNames(object)
.filter(
name =>
!name.startsWith('_') &&
typeof (object as Record<string, unknown>)[name] === 'function',
)
.map(name => `${namespace}.${name}`);
})
.reduce((arr, names) => arr.concat(names), [])
.filter(name => !nativelyNotBoundMembers.has(name));
const nativelyBoundMembers = new Set(
SUPPORTED_GLOBALS.flatMap(namespace => {
if (!(namespace in global)) {
// node.js might not have namespaces like Intl depending on compilation options
// https://nodejs.org/api/intl.html#intl_options_for_building_node_js
return [];
}
const object = global[namespace];
return Object.getOwnPropertyNames(object)
.filter(
name =>
!name.startsWith('_') &&
typeof (object as Record<string, unknown>)[name] === 'function',
)
.map(name => `${namespace}.${name}`);
}),
);

const isNotImported = (
symbol: ts.Symbol,
Expand Down Expand Up @@ -201,7 +165,7 @@ export default createRule<Options, MessageIds>({

if (
objectSymbol &&
nativelyBoundMembers.includes(getMemberFullName(node)) &&
nativelyBoundMembers.has(getMemberFullName(node)) &&
isNotImported(objectSymbol, currentSourceFile)
) {
return;
Expand Down Expand Up @@ -232,7 +196,7 @@ export default createRule<Options, MessageIds>({
if (
notImported &&
isIdentifier(initNode) &&
nativelyBoundMembers.includes(
nativelyBoundMembers.has(
`${initNode.name}.${property.key.name}`,
)
) {
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin/tests/rules/unbound-method.test.ts
Expand Up @@ -57,6 +57,7 @@ ruleTester.run('unbound-method', rule, {
"['1', '2', '3'].map(Number.parseInt);",
'[5.2, 7.1, 3.6].map(Math.floor);',
'const x = console.log;',
'const x = Object.defineProperty;',
...[
'instance.bound();',
'instance.unbound();',
Expand Down