From de18660a8cf8f7033798646d8c5b0938d1accb12 Mon Sep 17 00:00:00 2001 From: YeonJuan Date: Wed, 3 Jun 2020 08:01:17 +0900 Subject: [PATCH] fix(eslint-plugin): [prefer-optional-chain] handling first member expression (#2156) --- .../eslint-plugin-tslint/src/rules/config.ts | 2 +- .../src/rules/adjacent-overload-signatures.ts | 2 +- .../eslint-plugin/src/rules/array-type.ts | 2 +- .../src/rules/indent-new-do-not-use/index.ts | 2 +- .../src/rules/no-empty-function.ts | 7 +- .../src/rules/no-use-before-define.ts | 3 +- .../src/rules/prefer-optional-chain.ts | 6 +- .../tests/rules/prefer-optional-chain.test.ts | 70 +++++++++++++++++-- .../src/eslint-utils/RuleTester.ts | 4 +- packages/parser/src/analyze-scope.ts | 3 +- packages/typescript-estree/src/convert.ts | 4 +- .../src/create-program/createWatchProgram.ts | 2 +- 12 files changed, 82 insertions(+), 25 deletions(-) diff --git a/packages/eslint-plugin-tslint/src/rules/config.ts b/packages/eslint-plugin-tslint/src/rules/config.ts index 32198d0fc8c..b6da50bbf50 100644 --- a/packages/eslint-plugin-tslint/src/rules/config.ts +++ b/packages/eslint-plugin-tslint/src/rules/config.ts @@ -135,7 +135,7 @@ export default createRule({ /** * Format the TSLint results for ESLint */ - if (result.failures && result.failures.length) { + if (result.failures?.length) { result.failures.forEach(failure => { const start = failure.getStartPosition().getLineAndCharacter(); const end = failure.getEndPosition().getLineAndCharacter(); diff --git a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts index a72bd186e29..7b46240c581 100644 --- a/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts +++ b/packages/eslint-plugin/src/rules/adjacent-overload-signatures.ts @@ -53,7 +53,7 @@ export default util.createRule({ } case AST_NODE_TYPES.TSDeclareFunction: case AST_NODE_TYPES.FunctionDeclaration: - return member.id && member.id.name; + return member.id?.name ?? null; case AST_NODE_TYPES.TSMethodSignature: return util.getNameFromMember(member, sourceCode); case AST_NODE_TYPES.TSCallSignatureDeclaration: diff --git a/packages/eslint-plugin/src/rules/array-type.ts b/packages/eslint-plugin/src/rules/array-type.ts index de3cca75343..0a33d48957c 100644 --- a/packages/eslint-plugin/src/rules/array-type.ts +++ b/packages/eslint-plugin/src/rules/array-type.ts @@ -281,7 +281,7 @@ export default util.createRule({ } const readonlyPrefix = isReadonlyArrayType ? 'readonly ' : ''; - const typeParams = node.typeParameters && node.typeParameters.params; + const typeParams = node.typeParameters?.params; const messageId = defaultOption === 'array' ? 'errorStringArray' diff --git a/packages/eslint-plugin/src/rules/indent-new-do-not-use/index.ts b/packages/eslint-plugin/src/rules/indent-new-do-not-use/index.ts index 73123355a32..05c35c7a295 100644 --- a/packages/eslint-plugin/src/rules/indent-new-do-not-use/index.ts +++ b/packages/eslint-plugin/src/rules/indent-new-do-not-use/index.ts @@ -537,7 +537,7 @@ export default createRule({ * A "legal ancestor" is an expression or statement that causes the function to get executed immediately. * For example, `!(function(){})()` is an outer IIFE even though it is preceded by a ! operator. */ - let statement = node.parent && node.parent.parent; + let statement = node.parent?.parent; while ( statement && diff --git a/packages/eslint-plugin/src/rules/no-empty-function.ts b/packages/eslint-plugin/src/rules/no-empty-function.ts index e1bc1ed716b..6dc610941e6 100644 --- a/packages/eslint-plugin/src/rules/no-empty-function.ts +++ b/packages/eslint-plugin/src/rules/no-empty-function.ts @@ -83,11 +83,8 @@ export default util.createRule({ function hasParameterProperties( node: TSESTree.FunctionDeclaration | TSESTree.FunctionExpression, ): boolean { - return ( - node.params && - node.params.some( - param => param.type === AST_NODE_TYPES.TSParameterProperty, - ) + return node.params?.some( + param => param.type === AST_NODE_TYPES.TSParameterProperty, ); } diff --git a/packages/eslint-plugin/src/rules/no-use-before-define.ts b/packages/eslint-plugin/src/rules/no-use-before-define.ts index ed0240082d2..cb6955e8e3d 100644 --- a/packages/eslint-plugin/src/rules/no-use-before-define.ts +++ b/packages/eslint-plugin/src/rules/no-use-before-define.ts @@ -137,8 +137,7 @@ function isInInitializer( return true; } if ( - node.parent && - node.parent.parent && + node.parent?.parent && (node.parent.parent.type === AST_NODE_TYPES.ForInStatement || node.parent.parent.type === AST_NODE_TYPES.ForOfStatement) && isInRange(node.parent.parent.right, location) diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index c2d0db90ea7..52067d2f9b7 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -55,18 +55,20 @@ export default util.createRule({ return { [[ 'LogicalExpression[operator="&&"] > Identifier', + 'LogicalExpression[operator="&&"] > MemberExpression', 'LogicalExpression[operator="&&"] > BinaryExpression[operator="!=="]', 'LogicalExpression[operator="&&"] > BinaryExpression[operator="!="]', ].join(',')]( initialIdentifierOrNotEqualsExpr: | TSESTree.BinaryExpression - | TSESTree.Identifier, + | TSESTree.Identifier + | TSESTree.MemberExpression, ): void { // selector guarantees this cast const initialExpression = initialIdentifierOrNotEqualsExpr.parent as TSESTree.LogicalExpression; if (initialExpression.left !== initialIdentifierOrNotEqualsExpr) { - // the identifier is not the deepest left node + // the node(identifier or member expression) is not the deepest left node return; } if (!isValidChainTarget(initialIdentifierOrNotEqualsExpr, true)) { 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 fa3404bfe6f..80340828a74 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain.test.ts @@ -16,24 +16,44 @@ const baseCases = [ code: 'foo && foo.bar', output: 'foo?.bar', }, + { + code: 'foo.bar && foo.bar.baz', + output: 'foo.bar?.baz', + }, { code: 'foo && foo()', output: 'foo?.()', }, + { + code: 'foo.bar && foo.bar()', + output: 'foo.bar?.()', + }, { code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', output: 'foo?.bar?.baz?.buzz', }, { - // case with a jump (i.e. a non-nullish prop) + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + }, + // case with a jump (i.e. a non-nullish prop) + { code: 'foo && foo.bar && foo.bar.baz.buzz', output: 'foo?.bar?.baz.buzz', }, { - // case where for some reason there is a doubled up expression + code: 'foo.bar && foo.bar.baz.buzz', + output: 'foo.bar?.baz.buzz', + }, + // case where for some reason there is a doubled up expression + { code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', output: 'foo?.bar?.baz?.buzz', }, + { + code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', + output: 'foo.bar?.baz?.buzz', + }, // chained members with element access { code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', @@ -55,10 +75,18 @@ const baseCases = [ output: 'foo?.bar?.baz?.buzz?.()', }, { - // case with a jump (i.e. a non-nullish prop) + code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', + output: 'foo.bar?.baz?.buzz?.()', + }, + // case with a jump (i.e. a non-nullish prop) + { code: 'foo && foo.bar && foo.bar.baz.buzz()', output: 'foo?.bar?.baz.buzz()', }, + { + code: 'foo.bar && foo.bar.baz.buzz()', + output: 'foo.bar?.baz.buzz()', + }, { // case with a jump (i.e. a non-nullish prop) code: 'foo && foo.bar && foo.bar.baz.buzz && foo.bar.baz.buzz()', @@ -94,6 +122,10 @@ const baseCases = [ code: 'foo && foo?.() && foo?.().bar', output: 'foo?.()?.bar', }, + { + code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', + output: 'foo.bar?.()?.baz', + }, ].map( c => ({ @@ -220,8 +252,8 @@ ruleTester.run('prefer-optional-chain', rule, { }, ], }, + // case with inconsistent checks { - // case with inconsistent checks code: 'foo && foo.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;', output: null, @@ -237,6 +269,21 @@ ruleTester.run('prefer-optional-chain', rule, { }, ], }, + { + code: noFormat`foo.bar && foo.bar.baz != null && foo.bar.baz.qux !== undefined && foo.bar.baz.qux.buzz;`, + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo.bar?.baz?.qux?.buzz;', + }, + ], + }, + ], + }, // ensure essential whitespace isn't removed { code: 'foo && foo.bar(baz => );', @@ -404,6 +451,21 @@ foo?.bar(/* comment */a, }, ], }, + { + code: 'foo.bar && foo.bar?.();', + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo.bar?.();', + }, + ], + }, + ], + }, // using suggestion instead of autofix { code: diff --git a/packages/experimental-utils/src/eslint-utils/RuleTester.ts b/packages/experimental-utils/src/eslint-utils/RuleTester.ts index ce5674d9c65..50c3794328b 100644 --- a/packages/experimental-utils/src/eslint-utils/RuleTester.ts +++ b/packages/experimental-utils/src/eslint-utils/RuleTester.ts @@ -40,9 +40,7 @@ class RuleTester extends TSESLint.RuleTester { } private getFilename(options?: TSESLint.ParserOptions): string { if (options) { - const filename = `file.ts${ - options.ecmaFeatures && options.ecmaFeatures.jsx ? 'x' : '' - }`; + const filename = `file.ts${options.ecmaFeatures?.jsx ? 'x' : ''}`; if (options.project) { return path.join( options.tsconfigRootDir != null diff --git a/packages/parser/src/analyze-scope.ts b/packages/parser/src/analyze-scope.ts index 66a2167d41c..73d1d900275 100644 --- a/packages/parser/src/analyze-scope.ts +++ b/packages/parser/src/analyze-scope.ts @@ -875,8 +875,7 @@ export function analyzeScope( directive: false, nodejsScope: parserOptions.sourceType === 'script' && - (parserOptions.ecmaFeatures && - parserOptions.ecmaFeatures.globalReturn) === true, + parserOptions.ecmaFeatures?.globalReturn === true, impliedStrict: false, sourceType: parserOptions.sourceType, ecmaVersion: parserOptions.ecmaVersion ?? 2018, diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index f23bac14ecb..44279a76d97 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -375,7 +375,7 @@ export class Converter { return parameters.map(param => { const convertedParam = this.convertChild(param) as TSESTree.Parameter; - if (param.decorators && param.decorators.length) { + if (param.decorators?.length) { convertedParam.decorators = param.decorators.map(el => this.convertChild(el), ); @@ -1485,7 +1485,7 @@ export class Converter { ); } - if (superClass.types[0] && superClass.types[0].typeArguments) { + if (superClass.types[0]?.typeArguments) { result.superTypeParameters = this.convertTypeArgumentsToTypeParameters( superClass.types[0].typeArguments, superClass.types[0], diff --git a/packages/typescript-estree/src/create-program/createWatchProgram.ts b/packages/typescript-estree/src/create-program/createWatchProgram.ts index cf31a5c383d..bf703b38d93 100644 --- a/packages/typescript-estree/src/create-program/createWatchProgram.ts +++ b/packages/typescript-estree/src/create-program/createWatchProgram.ts @@ -111,7 +111,7 @@ function diagnosticReporter(diagnostic: ts.Diagnostic): void { */ function createHash(content: string): string { // No ts.sys in browser environments. - if (ts.sys && ts.sys.createHash) { + if (ts.sys?.createHash) { return ts.sys.createHash(content); } return content;