Fix false positive for well-known symbols in es-x/no-nonstandard-*-properties rules (#231)

Author: ota-meshi

lib/util/define-nonstandard-prototype-properties-handler/index.js

@@ -1,6 +1,6 @@
 "use strict"
 
-const { getPropertyName } = require("@eslint-community/eslint-utils")
+const { getPropertyKeyValue } = require("../get-property-key-value")
 const {
     buildObjectTypeChecker,
 } = require("../type-checker/object-type-checker")
@@ -48,12 +48,13 @@ function defineNonstandardPrototypePropertiesHandler(
 
     return {
         MemberExpression(node) {
-            const propertyName = getPropertyName(
+            const propertyName = getPropertyKeyValue(
                 node,
                 sourceCode.getScope(node),
             )
             if (
-                propertyName == null ||
+                // If the key is a symbol, it is ignored.
+                typeof propertyName !== "string" ||
                 options?.allowsPropertyName?.(propertyName)
             ) {
                 return
@@ -75,12 +76,13 @@ function defineNonstandardPrototypePropertiesHandler(
             "AssignmentExpression > ObjectPattern.left > Property.properties",
             "AssignmentPattern > ObjectPattern.left > Property.properties",
         ].join(",")](node) {
-            const propertyName = getPropertyName(
+            const propertyName = getPropertyKeyValue(
                 node,
                 sourceCode.getScope(node),
             )
             if (
-                propertyName == null ||
+                // If the key is a symbol, it is ignored.
+                typeof propertyName !== "string" ||
                 options?.allowsPropertyName?.(propertyName)
             ) {
                 return

lib/util/define-nonstandard-static-properties-handler/index.js

@@ -1,10 +1,7 @@
 "use strict"
 
-const {
-    getPropertyName,
-    ReferenceTracker,
-    READ,
-} = require("@eslint-community/eslint-utils")
+const { getPropertyKeyValue } = require("../get-property-key-value")
+const { ReferenceTracker, READ } = require("@eslint-community/eslint-utils")
 const { getSourceCode } = require("eslint-compat-utils")
 
 /**
@@ -51,11 +48,15 @@ function defineNonstandardStaticPropertiesHandler(context, nameMap) {
                 if (prop.type !== "Property") {
                     continue
                 }
-                const propertyName = getPropertyName(
+                const propertyName = getPropertyKeyValue(
                     prop,
                     sourceCode.getScope(node),
                 )
-                if (propertyName == null || propertyNames.has(propertyName)) {
+                if (
+                    // If the key is a symbol, it is ignored.
+                    typeof propertyName !== "string" ||
+                    propertyNames.has(propertyName)
+                ) {
                     continue
                 }
                 yield { node: prop, propertyName }
@@ -74,11 +75,15 @@ function defineNonstandardStaticPropertiesHandler(context, nameMap) {
                 if (parent.object !== node) {
                     continue
                 }
-                const propertyName = getPropertyName(
+                const propertyName = getPropertyKeyValue(
                     parent,
                     sourceCode.getScope(node),
                 )
-                if (propertyName == null || propertyNames.has(propertyName)) {
+                if (
+                    // If the key is a symbol, it is ignored.
+                    typeof propertyName !== "string" ||
+                    propertyNames.has(propertyName)
+                ) {
                     continue
                 }
                 report(parent, path, propertyName)

lib/util/get-property-key-value.js

@@ -0,0 +1,63 @@
+"use strict"
+
+const { getStaticValue } = require("@eslint-community/eslint-utils")
+
+/**
+ * @typedef {import('estree').Property} Property
+ * @typedef {import('estree').MemberExpression} MemberExpression
+ * @typedef {import('estree').Expression} Expression
+ * @typedef {import('estree').PrivateIdentifier} PrivateIdentifier
+ * @typedef {import('eslint').Scope.Scope} Scope
+ */
+/**
+ * Get the property name/symbol from a MemberExpression node or a Property node.
+ *
+ * The difference from `@eslint-community/eslint-utils.getPropertyName()` is
+ * that if key is a Symbol, this function returns a Symbol.
+ * @param {Property|MemberExpression} node The node to get.
+ * @param {Scope} [initialScope] The scope to start finding variable. Optional. If the node is a computed property node and this scope was given, this checks the computed property name by the `getStringIfConstant` function with the scope, and returns the value of it.
+ * @returns {string|symbol|null}
+ */
+function getPropertyKeyValue(node, initialScope) {
+    switch (node.type) {
+        case "MemberExpression":
+            if (node.computed) {
+                return getStaticKeyValue(node.property, initialScope)
+            }
+            if (node.property.type === "PrivateIdentifier") {
+                return null
+            }
+            return node.property.name
+
+        case "Property":
+        case "MethodDefinition":
+        case "PropertyDefinition":
+            if (node.computed) {
+                return getStaticKeyValue(node.key, initialScope)
+            }
+            if (node.key.type === "Literal") {
+                return String(node.key.value)
+            }
+            if (node.key.type === "PrivateIdentifier") {
+                return null
+            }
+            return node.key.name
+
+        // no default
+    }
+
+    return null
+}
+
+/**
+ * @param {Expression|PrivateIdentifier} node
+ * @param {Scope} initialScope
+ */
+function getStaticKeyValue(node, initialScope) {
+    const value = getStaticValue(node, initialScope)
+    return value && typeof value.value === "symbol"
+        ? value.value
+        : String(value.value)
+}
+
+module.exports = { getPropertyKeyValue }

tests/lib/rules/no-nonstandard-array-properties.js

@@ -7,6 +7,8 @@ const { arrayProperties } = require("../../../lib/util/well-known-properties")
 new RuleTester().run("no-nonstandard-array-properties", rule, {
     valid: [
         ...[...arrayProperties].map((p) => `Array.${p}`),
+        "Array[Symbol.species]",
+        "const {[Symbol.species]:foo} = Array",
         { code: "Array.unknown()", options: [{ allow: ["unknown"] }] },
     ],
     invalid: [

tests/lib/rules/no-nonstandard-array-prototype-properties.js

@@ -16,6 +16,9 @@ new RuleTester().run(ruleId, rule, {
         ...[...arrayPrototypeProperties].map((p) => `['A'].${p}`),
         "['A'][0]",
         "['A']['0']",
+        "['A'][Symbol.iterator]",
+        "['A'][Symbol.unscopables]",
+        "const {[Symbol.iterator]:iterator} = []",
         { code: "['A'].unknown()", options: [{ allow: ["unknown"] }] },
         // Test for https://github.com/eslint-community/eslint-plugin-es-x/issues/223
         "for (const { x } of foo) {}",