fix(eslint-plugin): [prefer-optional-chain] fix ThisExpression and …

…`PrivateIdentifier` errors (#6028)
typescript-eslint · Jan 31, 2023 · 85e783c · 85e783c
1 parent 202d9fb
commit 85e783c
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 15 deletions.
diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts
@@ -10,6 +10,7 @@ type ValidChainTarget =
   | TSESTree.CallExpression
   | TSESTree.ChainExpression
   | TSESTree.Identifier
+  | TSESTree.PrivateIdentifier
   | TSESTree.MemberExpression
   | TSESTree.ThisExpression
   | TSESTree.MetaProperty;
@@ -164,7 +165,9 @@ export default util.createRule({
             break;
           }
 
+          let invalidOptionallyChainedPrivateProperty;
           ({
+            invalidOptionallyChainedPrivateProperty,
             expressionCount,
             previousLeftText,
             optionallyChainedCode,
@@ -178,6 +181,9 @@ export default util.createRule({
             previous,
             current,
           ));
+          if (invalidOptionallyChainedPrivateProperty) {
+            return;
+          }
         }
 
         reportIfMoreThanOne({
@@ -243,7 +249,9 @@ export default util.createRule({
             break;
           }
 
+          let invalidOptionallyChainedPrivateProperty;
           ({
+            invalidOptionallyChainedPrivateProperty,
             expressionCount,
             previousLeftText,
             optionallyChainedCode,
@@ -257,6 +265,9 @@ export default util.createRule({
             previous,
             current,
           ));
+          if (invalidOptionallyChainedPrivateProperty) {
+            return;
+          }
         }
 
         reportIfMoreThanOne({
@@ -343,7 +354,10 @@ export default util.createRule({
         return `${calleeText}${argumentsText}`;
       }
 
-      if (node.type === AST_NODE_TYPES.Identifier) {
+      if (
+        node.type === AST_NODE_TYPES.Identifier ||
+        node.type === AST_NODE_TYPES.PrivateIdentifier
+      ) {
         return node.name;
       }
 
@@ -381,23 +395,20 @@ export default util.createRule({
 
       // cases should match the list in ALLOWED_MEMBER_OBJECT_TYPES
       switch (node.object.type) {
-        case AST_NODE_TYPES.CallExpression:
-        case AST_NODE_TYPES.Identifier:
-          objectText = getText(node.object);
-          break;
-
         case AST_NODE_TYPES.MemberExpression:
           objectText = getMemberExpressionText(node.object);
           break;
 
+        case AST_NODE_TYPES.CallExpression:
+        case AST_NODE_TYPES.Identifier:
         case AST_NODE_TYPES.MetaProperty:
         case AST_NODE_TYPES.ThisExpression:
           objectText = getText(node.object);
           break;
 
         /* istanbul ignore next */
         default:
-          throw new Error(`Unexpected member object type: ${node.object.type}`);
+          return '';
       }
 
       let propertyText: string;
@@ -420,9 +431,7 @@ export default util.createRule({
 
           /* istanbul ignore next */
           default:
-            throw new Error(
-              `Unexpected member property type: ${node.object.type}`,
-            );
+            return '';
         }
 
         return `${objectText}${node.optional ? '?.' : ''}[${propertyText}]`;
@@ -432,12 +441,12 @@ export default util.createRule({
           case AST_NODE_TYPES.Identifier:
             propertyText = getText(node.property);
             break;
+          case AST_NODE_TYPES.PrivateIdentifier:
+            propertyText = '#' + getText(node.property);
+            break;
 
-          /* istanbul ignore next */
           default:
-            throw new Error(
-              `Unexpected member property type: ${node.object.type}`,
-            );
+            propertyText = sourceCode.getText(node.property);
         }
 
         return `${objectText}${node.optional ? '?.' : '.'}${propertyText}`;
@@ -461,6 +470,7 @@ const ALLOWED_COMPUTED_PROP_TYPES: ReadonlySet<AST_NODE_TYPES> = new Set([
 ]);
 const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet<AST_NODE_TYPES> = new Set([
   AST_NODE_TYPES.Identifier,
+  AST_NODE_TYPES.PrivateIdentifier,
 ]);
 
 interface ReportIfMoreThanOneOptions {
@@ -525,6 +535,7 @@ function reportIfMoreThanOne({
 }
 
 interface NormalizedPattern {
+  invalidOptionallyChainedPrivateProperty: boolean;
   expressionCount: number;
   previousLeftText: string;
   optionallyChainedCode: string;
@@ -541,6 +552,7 @@ function normalizeRepeatingPatterns(
   current: TSESTree.Node,
 ): NormalizedPattern {
   const leftText = previousLeftText;
+  let invalidOptionallyChainedPrivateProperty = false;
   // omit weird doubled up expression that make no sense like foo.bar && foo.bar
   if (rightText !== previousLeftText) {
     expressionCount += 1;
@@ -576,6 +588,11 @@ function normalizeRepeatingPatterns(
     diff === '?.buzz'
     */
     const diff = rightText.replace(leftText, '');
+    if (diff.startsWith('.#')) {
+      // Do not handle direct optional chaining on private properties because of a typescript bug (https://github.com/microsoft/TypeScript/issues/42734)
+      // We still allow in computed properties
+      invalidOptionallyChainedPrivateProperty = true;
+    }
     if (diff.startsWith('?')) {
       // item was "pre optional chained"
       optionallyChainedCode += diff;
@@ -591,6 +608,7 @@ function normalizeRepeatingPatterns(
     util.NullThrowsReasons.MissingParent,
   );
   return {
+    invalidOptionallyChainedPrivateProperty,
     expressionCount,
     previousLeftText,
     optionallyChainedCode,

diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain/prefer-optional-chain.test.ts
@@ -45,6 +45,16 @@ ruleTester.run('prefer-optional-chain', rule, {
     'match && match$1 !== undefined;',
     'foo !== null && foo !== undefined;',
     "x['y'] !== undefined && x['y'] !== null;",
+    // private properties
+    'this.#a && this.#b;',
+    '!this.#a || !this.#b;',
+    'a.#foo?.bar;',
+    '!a.#foo?.bar;',
+    '!foo().#a || a;',
+    '!a.b.#a || a;',
+    '!new A().#b || a;',
+    '!(await a).#b || a;',
+    "!(foo as any).bar || 'anything';",
     // currently do not handle complex computed properties
     'foo && foo[bar as string] && foo[bar as string].baz;',
     'foo && foo[1 + 2] && foo[1 + 2].baz;',
@@ -68,6 +78,10 @@ ruleTester.run('prefer-optional-chain', rule, {
     '!import.meta && !import.meta.foo;',
     'new.target || new.target.length;',
     '!new.target || true;',
+    // Do not handle direct optional chaining on private properties because of a typescript bug (https://github.com/microsoft/TypeScript/issues/42734)
+    // We still allow in computed properties
+    'foo && foo.#bar;',
+    '!foo || !foo.#bar;',
   ],
   invalid: [
     ...BaseCases.all(),
@@ -1240,7 +1254,6 @@ foo?.bar(/* comment */a,
         },
       ],
     },
-
     {
       code: noFormat`import.meta && import.meta?.() && import.meta?.().baz;`,
       output: null,