fix(eslint-plugin): [unbound-method] exempt all non-Promise built-in …

…statics (#8096) * fix(eslint-plugin): [unbound-method] exempt all non-Promise built-in statics * yarn format --write --------- Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
typescript-eslint · Dec 25, 2023 · 3182959 · Emsiiie · Dec 26, 2023 · 3182959
1 parent 3031117
commit 3182959
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 64 deletions.
diff --git a/.cspell.json b/.cspell.json
@@ -122,6 +122,7 @@
     "stringification",
     "stringifying",
     "stringly",
+    "subclassing",
     "superset",
     "thenables",
     "transpiled",

diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts
@@ -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',
@@ -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,
@@ -201,7 +165,7 @@ export default createRule<Options, MessageIds>({
 
         if (
           objectSymbol &&
-          nativelyBoundMembers.includes(getMemberFullName(node)) &&
+          nativelyBoundMembers.has(getMemberFullName(node)) &&
           isNotImported(objectSymbol, currentSourceFile)
         ) {
           return;
@@ -232,7 +196,7 @@ export default createRule<Options, MessageIds>({
               if (
                 notImported &&
                 isIdentifier(initNode) &&
-                nativelyBoundMembers.includes(
+                nativelyBoundMembers.has(
                   `${initNode.name}.${property.key.name}`,
                 )
               ) {

diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts
@@ -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();',