From 71cb5020c64ee7023bb44afbcf3439c8e60d7c03 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Thu, 17 Nov 2022 21:35:35 +0200 Subject: [PATCH 01/13] prevent thrown error from default case --- .../eslint-plugin/src/rules/prefer-optional-chain.ts | 9 ++++++--- .../tests/rules/prefer-optional-chain.test.ts | 7 +++++++ 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 73fbcc27af3..33e07552444 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -425,9 +425,12 @@ export default util.createRule({ /* istanbul ignore next */ default: - throw new Error( - `Unexpected member property type: ${node.object.type}`, - ); + if (![AST_NODE_TYPES.Identifier, AST_NODE_TYPES.ThisExpression].includes(node.object.type)) { + throw new Error( + `Unexpected member property type: ${node.object.type}`, + ); + } + propertyText = sourceCode.getText(node.property); } return `${objectText}${node.optional ? '?.' : '.'}${propertyText}`; diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index e1a1467d80f..a545f70497c 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -194,6 +194,13 @@ 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 && a.#foo.bar;', + '!a.#foo || !a.#foo.bar;', + 'a.#foo?.bar;', + '!a.#foo?.bar;', // 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;', From 5acc1aff1dabb98498d3da761b5134e6d8634f08 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Thu, 17 Nov 2022 22:58:07 +0200 Subject: [PATCH 02/13] try --- .../src/rules/prefer-optional-chain.ts | 21 +++++++++--- .../tests/rules/prefer-optional-chain.test.ts | 32 +++++++++++++++++-- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 33e07552444..550d0fd25b7 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -113,11 +113,13 @@ export default util.createRule({ }, [[ 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > Identifier', + 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > PrivateIdentifier', 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > MemberExpression', 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > ChainExpression > MemberExpression', ].join(',')]( initialIdentifierOrNotEqualsExpr: | TSESTree.Identifier + | TSESTree.PrivateIdentifier | TSESTree.MemberExpression, ): void { // selector guarantees this cast @@ -425,7 +427,12 @@ export default util.createRule({ /* istanbul ignore next */ default: - if (![AST_NODE_TYPES.Identifier, AST_NODE_TYPES.ThisExpression].includes(node.object.type)) { + if ( + ![ + AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.ThisExpression, + ].includes(node.object.type) + ) { throw new Error( `Unexpected member property type: ${node.object.type}`, ); @@ -442,17 +449,20 @@ export default util.createRule({ const ALLOWED_MEMBER_OBJECT_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.ThisExpression, ]); const ALLOWED_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.Literal, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.TemplateLiteral, ]); const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, ]); interface ReportIfMoreThanOneOptions { @@ -487,6 +497,9 @@ function reportIfMoreThanOne({ previous.right.operator } ${sourceCode.getText(previous.right.right)}`; } + const newText = `${ + shouldHandleChainedAnds ? '' : '!' + }${optionallyChainedCode}`; context.report({ node: previous, @@ -494,11 +507,9 @@ function reportIfMoreThanOne({ suggest: [ { messageId: 'optionalChainSuggest', + // For some reason the fixer either doesn't accept it or accepts and reverts? :shrug: fix: (fixer): TSESLint.RuleFix[] => [ - fixer.replaceText( - previous, - `${shouldHandleChainedAnds ? '' : '!'}${optionallyChainedCode}`, - ), + fixer.replaceText(previous, newText), ], }, ], diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index a545f70497c..2bf351d3423 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -197,8 +197,6 @@ ruleTester.run('prefer-optional-chain', rule, { // private properties 'this.#a && this.#b;', '!this.#a || !this.#b;', - 'a.#foo && a.#foo.bar;', - '!a.#foo || !a.#foo.bar;', 'a.#foo?.bar;', '!a.#foo?.bar;', // currently do not handle complex computed properties @@ -1328,5 +1326,35 @@ foo?.bar(/* comment */a, }, ], }, + { + code: 'a.#foo && a.#foo.bar;', + output: 'a.#foo?.bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'a.#foo?.bar;', + }, + ], + }, + ], + }, + { + code: '!a.#foo || !a.#foo.bar;', + output: '!a.#foo?.bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '!a.#foo?.bar;', + }, + ], + }, + ], + }, ], }); From a3b6eda6542178644a00d533ebb157ebbf687eae Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Thu, 17 Nov 2022 22:58:28 +0200 Subject: [PATCH 03/13] Revert "try" This reverts commit 5acc1aff1dabb98498d3da761b5134e6d8634f08. --- .../src/rules/prefer-optional-chain.ts | 21 +++--------- .../tests/rules/prefer-optional-chain.test.ts | 32 ++----------------- 2 files changed, 7 insertions(+), 46 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 550d0fd25b7..33e07552444 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -113,13 +113,11 @@ export default util.createRule({ }, [[ 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > Identifier', - 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > PrivateIdentifier', 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > MemberExpression', 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > ChainExpression > MemberExpression', ].join(',')]( initialIdentifierOrNotEqualsExpr: | TSESTree.Identifier - | TSESTree.PrivateIdentifier | TSESTree.MemberExpression, ): void { // selector guarantees this cast @@ -427,12 +425,7 @@ export default util.createRule({ /* istanbul ignore next */ default: - if ( - ![ - AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.ThisExpression, - ].includes(node.object.type) - ) { + if (![AST_NODE_TYPES.Identifier, AST_NODE_TYPES.ThisExpression].includes(node.object.type)) { throw new Error( `Unexpected member property type: ${node.object.type}`, ); @@ -449,20 +442,17 @@ export default util.createRule({ const ALLOWED_MEMBER_OBJECT_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.ThisExpression, ]); const ALLOWED_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.Literal, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.TemplateLiteral, ]); const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.PrivateIdentifier, ]); interface ReportIfMoreThanOneOptions { @@ -497,9 +487,6 @@ function reportIfMoreThanOne({ previous.right.operator } ${sourceCode.getText(previous.right.right)}`; } - const newText = `${ - shouldHandleChainedAnds ? '' : '!' - }${optionallyChainedCode}`; context.report({ node: previous, @@ -507,9 +494,11 @@ function reportIfMoreThanOne({ suggest: [ { messageId: 'optionalChainSuggest', - // For some reason the fixer either doesn't accept it or accepts and reverts? :shrug: fix: (fixer): TSESLint.RuleFix[] => [ - fixer.replaceText(previous, newText), + fixer.replaceText( + previous, + `${shouldHandleChainedAnds ? '' : '!'}${optionallyChainedCode}`, + ), ], }, ], diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 2bf351d3423..a545f70497c 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -197,6 +197,8 @@ ruleTester.run('prefer-optional-chain', rule, { // private properties 'this.#a && this.#b;', '!this.#a || !this.#b;', + 'a.#foo && a.#foo.bar;', + '!a.#foo || !a.#foo.bar;', 'a.#foo?.bar;', '!a.#foo?.bar;', // currently do not handle complex computed properties @@ -1326,35 +1328,5 @@ foo?.bar(/* comment */a, }, ], }, - { - code: 'a.#foo && a.#foo.bar;', - output: 'a.#foo?.bar;', - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'a.#foo?.bar;', - }, - ], - }, - ], - }, - { - code: '!a.#foo || !a.#foo.bar;', - output: '!a.#foo?.bar;', - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!a.#foo?.bar;', - }, - ], - }, - ], - }, ], }); From a68b13aebfffe42dc7a2d6e73b79886690c7d95f Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Fri, 18 Nov 2022 06:29:30 +0200 Subject: [PATCH 04/13] More cases and format --- packages/eslint-plugin/src/rules/prefer-optional-chain.ts | 7 ++++++- .../tests/rules/prefer-optional-chain.test.ts | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 33e07552444..5e6f2138d21 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -425,7 +425,12 @@ export default util.createRule({ /* istanbul ignore next */ default: - if (![AST_NODE_TYPES.Identifier, AST_NODE_TYPES.ThisExpression].includes(node.object.type)) { + if ( + ![ + AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.ThisExpression, + ].includes(node.object.type) + ) { throw new Error( `Unexpected member property type: ${node.object.type}`, ); diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index a545f70497c..87fd41be115 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -201,6 +201,10 @@ ruleTester.run('prefer-optional-chain', rule, { '!a.#foo || !a.#foo.bar;', 'a.#foo?.bar;', '!a.#foo?.bar;', + 'foo().#a;', + 'a.b.#a;', + 'new A().#b;', + '(await a).#b;', // 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;', From 7e2185e2466d89c1f4af89d4cf741fd98086a14e Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Fri, 18 Nov 2022 06:39:48 +0200 Subject: [PATCH 05/13] Update packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts Co-authored-by: Joshua Chen --- .../tests/rules/prefer-optional-chain.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 87fd41be115..c375fcf5611 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -201,10 +201,10 @@ ruleTester.run('prefer-optional-chain', rule, { '!a.#foo || !a.#foo.bar;', 'a.#foo?.bar;', '!a.#foo?.bar;', - 'foo().#a;', - 'a.b.#a;', - 'new A().#b;', - '(await a).#b;', + '!foo().#a || a;', + '!a.b.#a || a;', + '!new A().#b || a;', + '!(await a).#b || a;', // 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;', From 21cfbd22784fb094c3d42e95242979022c1a813d Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Fri, 25 Nov 2022 18:43:34 +0200 Subject: [PATCH 06/13] TSAsExpression,AwaitExpression,NewExpression --- .../eslint-plugin/src/rules/prefer-optional-chain.ts | 11 +++++++++++ .../tests/rules/prefer-optional-chain.test.ts | 1 + 2 files changed, 12 insertions(+) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 5e6f2138d21..c6ef897456f 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -368,6 +368,15 @@ export default util.createRule({ * Gets a normalized representation of the given MemberExpression */ function getMemberExpressionText(node: TSESTree.MemberExpression): string { + if ( + [ + AST_NODE_TYPES.TSAsExpression, + AST_NODE_TYPES.AwaitExpression, + AST_NODE_TYPES.NewExpression, + ].includes(node.object.type) + ) { + return ''; + } let objectText: string; // cases should match the list in ALLOWED_MEMBER_OBJECT_TYPES @@ -429,6 +438,8 @@ export default util.createRule({ ![ AST_NODE_TYPES.Identifier, AST_NODE_TYPES.ThisExpression, + AST_NODE_TYPES.CallExpression, + AST_NODE_TYPES.MemberExpression, ].includes(node.object.type) ) { throw new Error( diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index c375fcf5611..7038898cd8b 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -205,6 +205,7 @@ ruleTester.run('prefer-optional-chain', rule, { '!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;', From c3c458e1f063dc88a63c180806c12da1f4efbb94 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Wed, 21 Dec 2022 22:59:48 +0200 Subject: [PATCH 07/13] CR: remove throws and simplify switch cases --- .../src/rules/prefer-optional-chain.ts | 35 +++---------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index a8319e721e9..60931ea2dc5 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -377,28 +377,16 @@ export default util.createRule({ * Gets a normalized representation of the given MemberExpression */ function getMemberExpressionText(node: TSESTree.MemberExpression): string { - if ( - [ - AST_NODE_TYPES.TSAsExpression, - AST_NODE_TYPES.AwaitExpression, - AST_NODE_TYPES.NewExpression, - ].includes(node.object.type) - ) { - return ''; - } let objectText: string; // 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); @@ -406,7 +394,7 @@ export default util.createRule({ /* istanbul ignore next */ default: - throw new Error(`Unexpected member object type: ${node.object.type}`); + return ''; } let propertyText: string; @@ -429,9 +417,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}]`; @@ -442,20 +428,7 @@ export default util.createRule({ propertyText = getText(node.property); break; - /* istanbul ignore next */ default: - if ( - ![ - AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.ThisExpression, - AST_NODE_TYPES.CallExpression, - AST_NODE_TYPES.MemberExpression, - ].includes(node.object.type) - ) { - throw new Error( - `Unexpected member property type: ${node.object.type}`, - ); - } propertyText = sourceCode.getText(node.property); } From ab7094910e93278ad3bff12b4c7e8f09ae97d37c Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sat, 24 Dec 2022 13:52:29 +0200 Subject: [PATCH 08/13] support private identifiers --- .../src/rules/prefer-optional-chain.ts | 13 ++++++- .../tests/rules/prefer-optional-chain.test.ts | 35 +++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 60931ea2dc5..7ce1fa7a73e 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; @@ -343,7 +344,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; } @@ -427,6 +431,9 @@ export default util.createRule({ case AST_NODE_TYPES.Identifier: propertyText = getText(node.property); break; + case AST_NODE_TYPES.PrivateIdentifier: + propertyText = '#' + getText(node.property); + break; default: propertyText = sourceCode.getText(node.property); @@ -441,18 +448,21 @@ export default util.createRule({ const ALLOWED_MEMBER_OBJECT_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.ThisExpression, AST_NODE_TYPES.MetaProperty, ]); const ALLOWED_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.Literal, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.TemplateLiteral, ]); const ALLOWED_NON_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, + AST_NODE_TYPES.PrivateIdentifier, ]); interface ReportIfMoreThanOneOptions { @@ -612,6 +622,7 @@ function isValidChainTarget( if ( allowIdentifier && (node.type === AST_NODE_TYPES.Identifier || + node.type === AST_NODE_TYPES.PrivateIdentifier || node.type === AST_NODE_TYPES.ThisExpression || node.type === AST_NODE_TYPES.MetaProperty) ) { diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index cca1e07dac1..6dfca9f1b5a 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -133,6 +133,23 @@ const baseCases = [ code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', output: 'foo.bar?.()?.baz', }, + // private properties + { + code: 'foo.#bar && foo.#bar.baz', + output: 'foo.#bar?.baz', + }, + { + code: 'foo && foo.#bar', + output: 'foo?.#bar', + }, + { + code: 'foo && foo[bar.#baz]', + output: 'foo?.[bar.#baz]', + }, + { + code: 'foo[bar.#baz] && foo[bar.#baz].buzz', + output: 'foo[bar.#baz]?.buzz', + }, ].map( c => ({ @@ -197,8 +214,6 @@ ruleTester.run('prefer-optional-chain', rule, { // private properties 'this.#a && this.#b;', '!this.#a || !this.#b;', - 'a.#foo && a.#foo.bar;', - '!a.#foo || !a.#foo.bar;', 'a.#foo?.bar;', '!a.#foo?.bar;', '!foo().#a || a;', @@ -1396,7 +1411,6 @@ foo?.bar(/* comment */a, }, ], }, - { code: noFormat`import.meta && import.meta?.() && import.meta?.().baz;`, output: null, @@ -1412,5 +1426,20 @@ foo?.bar(/* comment */a, }, ], }, + { + code: 'a[a.#b] && a[a.#b].c;', + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'a[a.#b]?.c;', + }, + ], + }, + ], + }, ], }); From 12effef5c2f5644a1f34e5bfe17558aed1114211 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sat, 24 Dec 2022 13:55:48 +0200 Subject: [PATCH 09/13] coverage --- packages/eslint-plugin/src/rules/prefer-optional-chain.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 7ce1fa7a73e..91a38d1fe27 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -435,6 +435,7 @@ export default util.createRule({ propertyText = '#' + getText(node.property); break; + /* istanbul ignore next */ default: propertyText = sourceCode.getText(node.property); } From 0afc516667c81d5b6f9c8ea09f7d6415bccc987d Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sat, 24 Dec 2022 13:56:06 +0200 Subject: [PATCH 10/13] over covered case --- .../tests/rules/prefer-optional-chain.test.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 6dfca9f1b5a..d2f73514b7a 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -1426,20 +1426,5 @@ foo?.bar(/* comment */a, }, ], }, - { - code: 'a[a.#b] && a[a.#b].c;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'a[a.#b]?.c;', - }, - ], - }, - ], - }, ], }); From 8efa51deb7b7d7b0d9369b10fe8af140ba4d7829 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sat, 24 Dec 2022 18:59:45 +0200 Subject: [PATCH 11/13] =?UTF-8?q?Clearly,=20we=20had=20some=20missing=20co?= =?UTF-8?q?verage=20=F0=9F=98=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/eslint-plugin/src/rules/prefer-optional-chain.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 91a38d1fe27..7ce1fa7a73e 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -435,7 +435,6 @@ export default util.createRule({ propertyText = '#' + getText(node.property); break; - /* istanbul ignore next */ default: propertyText = sourceCode.getText(node.property); } From 57b86c17ea631dc637c4620ecfb5dc95dc2657b8 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sat, 24 Dec 2022 20:33:49 +0200 Subject: [PATCH 12/13] this got uglier --- .../src/rules/prefer-optional-chain.ts | 26 ++++++++++++++----- .../tests/rules/prefer-optional-chain.test.ts | 7 +++-- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index 7ce1fa7a73e..e57d20fc066 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -165,7 +165,9 @@ export default util.createRule({ break; } + let invalidOptionallyChainedPrivateProperty; ({ + invalidOptionallyChainedPrivateProperty, expressionCount, previousLeftText, optionallyChainedCode, @@ -179,6 +181,9 @@ export default util.createRule({ previous, current, )); + if (invalidOptionallyChainedPrivateProperty) { + return; + } } reportIfMoreThanOne({ @@ -244,7 +249,9 @@ export default util.createRule({ break; } + let invalidOptionallyChainedPrivateProperty; ({ + invalidOptionallyChainedPrivateProperty, expressionCount, previousLeftText, optionallyChainedCode, @@ -258,6 +265,9 @@ export default util.createRule({ previous, current, )); + if (invalidOptionallyChainedPrivateProperty) { + return; + } } reportIfMoreThanOne({ @@ -448,14 +458,12 @@ export default util.createRule({ const ALLOWED_MEMBER_OBJECT_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.CallExpression, AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.ThisExpression, AST_NODE_TYPES.MetaProperty, ]); const ALLOWED_COMPUTED_PROP_TYPES: ReadonlySet = new Set([ AST_NODE_TYPES.Identifier, - AST_NODE_TYPES.PrivateIdentifier, AST_NODE_TYPES.Literal, AST_NODE_TYPES.MemberExpression, AST_NODE_TYPES.TemplateLiteral, @@ -492,10 +500,9 @@ function reportIfMoreThanOne({ shouldHandleChainedAnds && previous.right.type === AST_NODE_TYPES.BinaryExpression ) { + const rightText = sourceCode.getText(previous.right.right); // case like foo && foo.bar !== someValue - optionallyChainedCode += ` ${ - previous.right.operator - } ${sourceCode.getText(previous.right.right)}`; + optionallyChainedCode += ` ${previous.right.operator} ${rightText}`; } context.report({ @@ -517,6 +524,7 @@ function reportIfMoreThanOne({ } interface NormalizedPattern { + invalidOptionallyChainedPrivateProperty: boolean; expressionCount: number; previousLeftText: string; optionallyChainedCode: string; @@ -533,6 +541,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; @@ -568,6 +577,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; @@ -583,6 +597,7 @@ function normalizeRepeatingPatterns( util.NullThrowsReasons.MissingParent, ); return { + invalidOptionallyChainedPrivateProperty, expressionCount, previousLeftText, optionallyChainedCode, @@ -622,7 +637,6 @@ function isValidChainTarget( if ( allowIdentifier && (node.type === AST_NODE_TYPES.Identifier || - node.type === AST_NODE_TYPES.PrivateIdentifier || node.type === AST_NODE_TYPES.ThisExpression || node.type === AST_NODE_TYPES.MetaProperty) ) { diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index d2f73514b7a..742dc1b42e9 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -138,10 +138,6 @@ const baseCases = [ code: 'foo.#bar && foo.#bar.baz', output: 'foo.#bar?.baz', }, - { - code: 'foo && foo.#bar', - output: 'foo?.#bar', - }, { code: 'foo && foo[bar.#baz]', output: 'foo?.[bar.#baz]', @@ -244,6 +240,9 @@ 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;', ], invalid: [ ...baseCases, From 99528f0b3819c9809d95b76c7689f4d361e6ebf7 Mon Sep 17 00:00:00 2001 From: Omri Luzon Date: Sun, 25 Dec 2022 21:32:32 +0200 Subject: [PATCH 13/13] fix coverage --- packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts index 742dc1b42e9..29e09b1b988 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -243,6 +243,7 @@ ruleTester.run('prefer-optional-chain', rule, { // 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,