Skip to content

Commit de18660

Browse files
authored
fix(eslint-plugin): [prefer-optional-chain] handling first member expression (typescript-eslint#2156)
1 parent 2c8402a commit de18660

File tree

12 files changed

+82
-25
lines changed

12 files changed

+82
-25
lines changed

packages/eslint-plugin-tslint/src/rules/config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export default createRule<Options, MessageIds>({
135135
/**
136136
* Format the TSLint results for ESLint
137137
*/
138-
if (result.failures && result.failures.length) {
138+
if (result.failures?.length) {
139139
result.failures.forEach(failure => {
140140
const start = failure.getStartPosition().getLineAndCharacter();
141141
const end = failure.getEndPosition().getLineAndCharacter();

packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export default util.createRule({
5353
}
5454
case AST_NODE_TYPES.TSDeclareFunction:
5555
case AST_NODE_TYPES.FunctionDeclaration:
56-
return member.id && member.id.name;
56+
return member.id?.name ?? null;
5757
case AST_NODE_TYPES.TSMethodSignature:
5858
return util.getNameFromMember(member, sourceCode);
5959
case AST_NODE_TYPES.TSCallSignatureDeclaration:

packages/eslint-plugin/src/rules/array-type.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,7 @@ export default util.createRule<Options, MessageIds>({
281281
}
282282

283283
const readonlyPrefix = isReadonlyArrayType ? 'readonly ' : '';
284-
const typeParams = node.typeParameters && node.typeParameters.params;
284+
const typeParams = node.typeParameters?.params;
285285
const messageId =
286286
defaultOption === 'array'
287287
? 'errorStringArray'

packages/eslint-plugin/src/rules/indent-new-do-not-use/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -537,7 +537,7 @@ export default createRule<Options, MessageIds>({
537537
* A "legal ancestor" is an expression or statement that causes the function to get executed immediately.
538538
* For example, `!(function(){})()` is an outer IIFE even though it is preceded by a ! operator.
539539
*/
540-
let statement = node.parent && node.parent.parent;
540+
let statement = node.parent?.parent;
541541

542542
while (
543543
statement &&

packages/eslint-plugin/src/rules/no-empty-function.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,8 @@ export default util.createRule<Options, MessageIds>({
8383
function hasParameterProperties(
8484
node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression,
8585
): boolean {
86-
return (
87-
node.params &&
88-
node.params.some(
89-
param => param.type === AST_NODE_TYPES.TSParameterProperty,
90-
)
86+
return node.params?.some(
87+
param => param.type === AST_NODE_TYPES.TSParameterProperty,
9188
);
9289
}
9390

packages/eslint-plugin/src/rules/no-use-before-define.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,8 +137,7 @@ function isInInitializer(
137137
return true;
138138
}
139139
if (
140-
node.parent &&
141-
node.parent.parent &&
140+
node.parent?.parent &&
142141
(node.parent.parent.type === AST_NODE_TYPES.ForInStatement ||
143142
node.parent.parent.type === AST_NODE_TYPES.ForOfStatement) &&
144143
isInRange(node.parent.parent.right, location)

packages/eslint-plugin/src/rules/prefer-optional-chain.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,18 +55,20 @@ export default util.createRule({
5555
return {
5656
[[
5757
'LogicalExpression[operator="&&"] > Identifier',
58+
'LogicalExpression[operator="&&"] > MemberExpression',
5859
'LogicalExpression[operator="&&"] > BinaryExpression[operator="!=="]',
5960
'LogicalExpression[operator="&&"] > BinaryExpression[operator="!="]',
6061
].join(',')](
6162
initialIdentifierOrNotEqualsExpr:
6263
| TSESTree.BinaryExpression
63-
| TSESTree.Identifier,
64+
| TSESTree.Identifier
65+
| TSESTree.MemberExpression,
6466
): void {
6567
// selector guarantees this cast
6668
const initialExpression = initialIdentifierOrNotEqualsExpr.parent as TSESTree.LogicalExpression;
6769

6870
if (initialExpression.left !== initialIdentifierOrNotEqualsExpr) {
69-
// the identifier is not the deepest left node
71+
// the node(identifier or member expression) is not the deepest left node
7072
return;
7173
}
7274
if (!isValidChainTarget(initialIdentifierOrNotEqualsExpr, true)) {

packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,24 +16,44 @@ const baseCases = [
1616
code: 'foo && foo.bar',
1717
output: 'foo?.bar',
1818
},
19+
{
20+
code: 'foo.bar && foo.bar.baz',
21+
output: 'foo.bar?.baz',
22+
},
1923
{
2024
code: 'foo && foo()',
2125
output: 'foo?.()',
2226
},
27+
{
28+
code: 'foo.bar && foo.bar()',
29+
output: 'foo.bar?.()',
30+
},
2331
{
2432
code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz',
2533
output: 'foo?.bar?.baz?.buzz',
2634
},
2735
{
28-
// case with a jump (i.e. a non-nullish prop)
36+
code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz',
37+
output: 'foo.bar?.baz?.buzz',
38+
},
39+
// case with a jump (i.e. a non-nullish prop)
40+
{
2941
code: 'foo && foo.bar && foo.bar.baz.buzz',
3042
output: 'foo?.bar?.baz.buzz',
3143
},
3244
{
33-
// case where for some reason there is a doubled up expression
45+
code: 'foo.bar && foo.bar.baz.buzz',
46+
output: 'foo.bar?.baz.buzz',
47+
},
48+
// case where for some reason there is a doubled up expression
49+
{
3450
code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz',
3551
output: 'foo?.bar?.baz?.buzz',
3652
},
53+
{
54+
code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz',
55+
output: 'foo.bar?.baz?.buzz',
56+
},
3757
// chained members with element access
3858
{
3959
code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz',
@@ -55,10 +75,18 @@ const baseCases = [
5575
output: 'foo?.bar?.baz?.buzz?.()',
5676
},
5777
{
58-
// case with a jump (i.e. a non-nullish prop)
78+
code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()',
79+
output: 'foo.bar?.baz?.buzz?.()',
80+
},
81+
// case with a jump (i.e. a non-nullish prop)
82+
{
5983
code: 'foo && foo.bar && foo.bar.baz.buzz()',
6084
output: 'foo?.bar?.baz.buzz()',
6185
},
86+
{
87+
code: 'foo.bar && foo.bar.baz.buzz()',
88+
output: 'foo.bar?.baz.buzz()',
89+
},
6290
{
6391
// case with a jump (i.e. a non-nullish prop)
6492
code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()',
@@ -94,6 +122,10 @@ const baseCases = [
94122
code: 'foo && foo?.() && foo?.().bar',
95123
output: 'foo?.()?.bar',
96124
},
125+
{
126+
code: 'foo.bar && foo.bar?.() && foo.bar?.().baz',
127+
output: 'foo.bar?.()?.baz',
128+
},
97129
].map(
98130
c =>
99131
({
@@ -220,8 +252,8 @@ ruleTester.run('prefer-optional-chain', rule, {
220252
},
221253
],
222254
},
255+
// case with inconsistent checks
223256
{
224-
// case with inconsistent checks
225257
code:
226258
'foo && foo.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;',
227259
output: null,
@@ -237,6 +269,21 @@ ruleTester.run('prefer-optional-chain', rule, {
237269
},
238270
],
239271
},
272+
{
273+
code: noFormat`foo.bar && foo.bar.baz != null && foo.bar.baz.qux !== undefined && foo.bar.baz.qux.buzz;`,
274+
output: null,
275+
errors: [
276+
{
277+
messageId: 'preferOptionalChain',
278+
suggestions: [
279+
{
280+
messageId: 'optionalChainSuggest',
281+
output: 'foo.bar?.baz?.qux?.buzz;',
282+
},
283+
],
284+
},
285+
],
286+
},
240287
// ensure essential whitespace isn't removed
241288
{
242289
code: 'foo && foo.bar(baz => <This Requires Spaces />);',
@@ -404,6 +451,21 @@ foo?.bar(/* comment */a,
404451
},
405452
],
406453
},
454+
{
455+
code: 'foo.bar && foo.bar?.();',
456+
output: null,
457+
errors: [
458+
{
459+
messageId: 'preferOptionalChain',
460+
suggestions: [
461+
{
462+
messageId: 'optionalChainSuggest',
463+
output: 'foo.bar?.();',
464+
},
465+
],
466+
},
467+
],
468+
},
407469
// using suggestion instead of autofix
408470
{
409471
code:

packages/experimental-utils/src/eslint-utils/RuleTester.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,7 @@ class RuleTester extends TSESLint.RuleTester {
4040
}
4141
private getFilename(options?: TSESLint.ParserOptions): string {
4242
if (options) {
43-
const filename = `file.ts${
44-
options.ecmaFeatures && options.ecmaFeatures.jsx ? 'x' : ''
45-
}`;
43+
const filename = `file.ts${options.ecmaFeatures?.jsx ? 'x' : ''}`;
4644
if (options.project) {
4745
return path.join(
4846
options.tsconfigRootDir != null

packages/parser/src/analyze-scope.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -875,8 +875,7 @@ export function analyzeScope(
875875
directive: false,
876876
nodejsScope:
877877
parserOptions.sourceType === 'script' &&
878-
(parserOptions.ecmaFeatures &&
879-
parserOptions.ecmaFeatures.globalReturn) === true,
878+
parserOptions.ecmaFeatures?.globalReturn === true,
880879
impliedStrict: false,
881880
sourceType: parserOptions.sourceType,
882881
ecmaVersion: parserOptions.ecmaVersion ?? 2018,

0 commit comments

Comments
 (0)