Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): [prefer-optional-chain] fix ThisExpression and PrivateIdentifier errors #6028

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
71cb502
prevent thrown error from default case
Nov 17, 2022
5acc1af
try
Nov 17, 2022
a3b6eda
Revert "try"
Nov 17, 2022
a68b13a
More cases and format
Nov 18, 2022
7e2185e
Update packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts
omril1 Nov 18, 2022
1a30e94
Merge remote-tracking branch 'upstream/main' into fix/issue6024-fix-p…
Nov 24, 2022
21cfbd2
TSAsExpression,AwaitExpression,NewExpression
Nov 25, 2022
0ae6c45
Merge remote-tracking branch 'upstream/main' into fix/issue6024-fix-p…
Nov 25, 2022
ecfacff
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Dec 5, 2022
3ce9b0a
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Dec 17, 2022
c3c458e
CR: remove throws and simplify switch cases
Dec 21, 2022
9c15f28
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Dec 21, 2022
ab70949
support private identifiers
Dec 24, 2022
e58dab9
Merge remote-tracking branch 'upstream/main' into fix/issue6024-fix-p…
Dec 24, 2022
12effef
coverage
Dec 24, 2022
0afc516
over covered case
Dec 24, 2022
8efa51d
Clearly, we had some missing coverage 😄
Dec 24, 2022
57b86c1
this got uglier
Dec 24, 2022
99528f0
fix coverage
Dec 25, 2022
cbb5168
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Jan 1, 2023
b6d265c
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Jan 2, 2023
54d4664
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Jan 13, 2023
ac8b6a2
Merge remote-tracking branch 'upstream/main' into fix/issue6024-fix-p…
Jan 30, 2023
a2a6314
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Jan 30, 2023
37716b2
Merge branch 'main' into fix/issue6024-fix-prefer-optional-chain-priv…
Jan 31, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 32 additions & 14 deletions packages/eslint-plugin/src/rules/prefer-optional-chain.ts
Expand Up @@ -10,6 +10,7 @@ type ValidChainTarget =
| TSESTree.CallExpression
| TSESTree.ChainExpression
| TSESTree.Identifier
| TSESTree.PrivateIdentifier
| TSESTree.MemberExpression
| TSESTree.ThisExpression
| TSESTree.MetaProperty;
Expand Down Expand Up @@ -164,7 +165,9 @@ export default util.createRule({
break;
}

let invalidOptionallyChainedPrivateProperty;
({
invalidOptionallyChainedPrivateProperty,
expressionCount,
previousLeftText,
optionallyChainedCode,
Expand All @@ -178,6 +181,9 @@ export default util.createRule({
previous,
current,
));
if (invalidOptionallyChainedPrivateProperty) {
return;
}
}

reportIfMoreThanOne({
Expand Down Expand Up @@ -243,7 +249,9 @@ export default util.createRule({
break;
}

let invalidOptionallyChainedPrivateProperty;
({
invalidOptionallyChainedPrivateProperty,
expressionCount,
previousLeftText,
optionallyChainedCode,
Expand All @@ -257,6 +265,9 @@ export default util.createRule({
previous,
current,
));
if (invalidOptionallyChainedPrivateProperty) {
return;
}
}

reportIfMoreThanOne({
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand All @@ -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}]`;
Expand All @@ -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}`;
Expand All @@ -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 {
Expand Down Expand Up @@ -525,6 +535,7 @@ function reportIfMoreThanOne({
}

interface NormalizedPattern {
invalidOptionallyChainedPrivateProperty: boolean;
expressionCount: number;
previousLeftText: string;
optionallyChainedCode: string;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -591,6 +608,7 @@ function normalizeRepeatingPatterns(
util.NullThrowsReasons.MissingParent,
);
return {
invalidOptionallyChainedPrivateProperty,
expressionCount,
previousLeftText,
optionallyChainedCode,
Expand Down
Expand Up @@ -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;',
Expand All @@ -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(),
Expand Down Expand Up @@ -1240,7 +1254,6 @@ foo?.bar(/* comment */a,
},
],
},

{
code: noFormat`import.meta && import.meta?.() && import.meta?.().baz;`,
output: null,
Expand Down