From 85e783c1fabe96d390729a5796d6d346e401692b Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Tue, 31 Jan 2023 09:21:23 +0200 Subject: [PATCH] fix(eslint-plugin): [prefer-optional-chain] fix `ThisExpression` and `PrivateIdentifier` errors (#6028) --- .../src/rules/prefer-optional-chain.ts | 46 +++++++++++++------ .../prefer-optional-chain.test.ts | 15 +++++- 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index f5ed3ccb589..efccc2ccfd2 100644 --- 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,15 +395,12 @@ 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); @@ -397,7 +408,7 @@ export default util.createRule({ /* 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 = new Set([ ]); const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet = 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 index ca783df13b3..a18de12bf7b 100644 --- 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,