diff --git a/packages/ast-spec/src/expression/BinaryExpression/BinaryOperatorToText.ts b/packages/ast-spec/src/expression/BinaryExpression/BinaryOperatorToText.ts new file mode 100644 index 00000000000..b19373f54d2 --- /dev/null +++ b/packages/ast-spec/src/expression/BinaryExpression/BinaryOperatorToText.ts @@ -0,0 +1,35 @@ +import type { SyntaxKind } from 'typescript'; + +// the members of ts.BinaryOperator +export interface BinaryOperatorToText { + [SyntaxKind.InstanceOfKeyword]: 'instanceof'; + [SyntaxKind.InKeyword]: 'in'; + + // math + [SyntaxKind.AsteriskAsteriskToken]: '**'; + [SyntaxKind.AsteriskToken]: '*'; + [SyntaxKind.SlashToken]: '/'; + [SyntaxKind.PercentToken]: '%'; + [SyntaxKind.PlusToken]: '+'; + [SyntaxKind.MinusToken]: '-'; + + // bitwise + [SyntaxKind.AmpersandToken]: '&'; + [SyntaxKind.BarToken]: '|'; + [SyntaxKind.CaretToken]: '^'; + [SyntaxKind.LessThanLessThanToken]: '<<'; + [SyntaxKind.GreaterThanGreaterThanToken]: '>>'; + [SyntaxKind.GreaterThanGreaterThanGreaterThanToken]: '>>>'; + + // logical + [SyntaxKind.AmpersandAmpersandToken]: '&&'; + [SyntaxKind.BarBarToken]: '||'; + [SyntaxKind.LessThanToken]: '<'; + [SyntaxKind.LessThanEqualsToken]: '<='; + [SyntaxKind.GreaterThanToken]: '>'; + [SyntaxKind.GreaterThanEqualsToken]: '>='; + [SyntaxKind.EqualsEqualsToken]: '=='; + [SyntaxKind.EqualsEqualsEqualsToken]: '==='; + [SyntaxKind.ExclamationEqualsEqualsToken]: '!=='; + [SyntaxKind.ExclamationEqualsToken]: '!='; +} diff --git a/packages/ast-spec/src/expression/BinaryExpression/spec.ts b/packages/ast-spec/src/expression/BinaryExpression/spec.ts index fa43c88bcf5..d42f1d4e77f 100644 --- a/packages/ast-spec/src/expression/BinaryExpression/spec.ts +++ b/packages/ast-spec/src/expression/BinaryExpression/spec.ts @@ -2,10 +2,14 @@ import type { AST_NODE_TYPES } from '../../ast-node-types'; import type { BaseNode } from '../../base/BaseNode'; import type { PrivateIdentifier } from '../../special/PrivateIdentifier/spec'; import type { Expression } from '../../unions/Expression'; +import type { ValueOf } from '../../utils'; +import type { BinaryOperatorToText } from './BinaryOperatorToText'; + +export * from './BinaryOperatorToText'; export interface BinaryExpression extends BaseNode { type: AST_NODE_TYPES.BinaryExpression; - operator: string; + operator: ValueOf; left: Expression | PrivateIdentifier; right: Expression; } diff --git a/packages/ast-spec/tests/BinaryOperatorToText.type-test.ts b/packages/ast-spec/tests/BinaryOperatorToText.type-test.ts new file mode 100644 index 00000000000..95f993b9e4d --- /dev/null +++ b/packages/ast-spec/tests/BinaryOperatorToText.type-test.ts @@ -0,0 +1,20 @@ +import type { + AssignmentOperator, + BinaryOperator, + SyntaxKind, +} from 'typescript'; + +import type { BinaryOperatorToText } from '../src'; + +type BinaryOperatorWithoutInvalidTypes = Exclude< + BinaryOperator, + | AssignmentOperator // --> AssignmentExpression + | SyntaxKind.CommaToken // -> SequenceExpression + | SyntaxKind.QuestionQuestionToken // -> LogicalExpression +>; +type _Test = { + readonly [T in BinaryOperatorWithoutInvalidTypes]: BinaryOperatorToText[T]; + // If there are any BinaryOperator members that don't have a corresponding + // BinaryOperatorToText, then this line will error with "Type 'T' cannot + // be used to index type 'BinaryOperatorToText'." +}; diff --git a/packages/eslint-plugin/docs/rules/prefer-optional-chain.md b/packages/eslint-plugin/docs/rules/prefer-optional-chain.md index 4a9ada8b08f..6efb88b17ae 100644 --- a/packages/eslint-plugin/docs/rules/prefer-optional-chain.md +++ b/packages/eslint-plugin/docs/rules/prefer-optional-chain.md @@ -57,13 +57,204 @@ foo?.a?.b?.c?.d?.e; -:::note -There are a few edge cases where this rule will false positive. Use your best judgement when evaluating reported errors. -::: +## Options + +In the context of the descriptions below a "loose boolean" operand is any operand that implicitly coerces the value to a boolean. +Specifically the argument of the not operator (`!loose`) or a bare value in a logical expression (`loose && looser`). + +### `allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing` + +When this option is `true`, the rule will not provide an auto-fixer for cases where the return type of the expression would change. For example for the expression `!foo || foo.bar` the return type of the expression is `true | T`, however for the equivalent optional chain `foo?.bar` the return type of the expression is `undefined | T`. Thus changing the code from a logical expression to an optional chain expression has altered the type of the expression. + +In some cases this distinction _may_ matter - which is why these fixers are considered unsafe - they may break the build! For example in the following code: + +```ts +declare const foo: { bar: boolean } | null | undefined; +declare function acceptsBoolean(arg: boolean): void; + +// ✅ typechecks succesfully as the expression only returns `boolean` +acceptsBoolean(foo != null && foo.bar); + +// ❌ typechecks UNSUCCESSFULLY as the expression returns `boolean | undefined` +acceptsBoolean(foo != null && foo.bar); +``` + +This style of code isn't super common - which means having this option set to `true` _should_ be safe in most codebases. However we default it to `false` due to its unsafe nature. We have provided this option for convenience because it increases the autofix cases covered by the rule. If you set option to `true` the onus is entirely on you and your team to ensure that each fix is correct and safe and that it does not break the build. + +When this option is `false` unsafe cases will have suggestion fixers provided instead of auto-fixers - meaning you can manually apply the fix using your IDE tooling. + +### `checkAny` + +When this option is `true` the rule will check operands that are typed as `any` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkAny: true` + +```ts +declare const thing: any; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkAny: false` + +```ts +declare const thing: any; + +thing && thing.toString(); +``` + + + +### `checkUnknown` + +When this option is `true` the rule will check operands that are typed as `unknown` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkUnknown: true` + +```ts +declare const thing: unknown; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkUnknown: false` + +```ts +declare const thing: unknown; + +thing && thing.toString(); +``` + + + +### `checkString` + +When this option is `true` the rule will check operands that are typed as `string` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkString: true` + +```ts +declare const thing: string; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkString: false` + +```ts +declare const thing: string; + +thing && thing.toString(); +``` + + + +### `checkNumber` + +When this option is `true` the rule will check operands that are typed as `number` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkNumber: true` + +```ts +declare const thing: number; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkNumber: false` + +```ts +declare const thing: number; + +thing && thing.toString(); +``` + + + +### `checkBoolean` + +When this option is `true` the rule will check operands that are typed as `boolean` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkBoolean: true` + +```ts +declare const thing: boolean; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkBoolean: false` + +```ts +declare const thing: boolean; + +thing && thing.toString(); +``` + + + +### `checkBigInt` + +When this option is `true` the rule will check operands that are typed as `bigint` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `checkBigInt: true` + +```ts +declare const thing: bigint; + +thing && thing.toString(); +``` + +#### ✅ Correct for `checkBigInt: false` + +```ts +declare const thing: bigint; + +thing && thing.toString(); +``` + + + +### `requireNullish` + +When this option is `true` the rule will skip operands that are not typed with `null` and/or `undefined` when inspecting "loose boolean" operands. + + + +#### ❌ Incorrect for `requireNullish: true` + +```ts +declare const thing1: string | null; +thing1 && thing1.toString(); +``` + +#### ✅ Correct for `requireNullish: true` + +```ts +declare const thing1: string | null; +thing1?.toString(); + +declare const thing2: string; +thing2 && thing2.toString(); +``` + + ## When Not To Use It -If you don't mind using more explicit `&&`s, you don't need this rule. +If you don't mind using more explicit `&&`s/`||`s, you don't need this rule. ## Further Reading diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index d2f12978704..dca9f33ff7a 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -51,6 +51,7 @@ "generate:configs": "yarn tsx tools/generate-configs.ts", "lint": "nx lint", "test": "jest --coverage", + "test-single": "jest --no-coverage", "typecheck": "tsc -p tsconfig.json --noEmit" }, "dependencies": { @@ -58,6 +59,7 @@ "@typescript-eslint/scope-manager": "5.61.0", "@typescript-eslint/type-utils": "5.61.0", "@typescript-eslint/utils": "5.61.0", + "@typescript-eslint/visitor-keys": "5.61.0", "debug": "^4.3.4", "grapheme-splitter": "^1.0.4", "graphemer": "^1.4.0", diff --git a/packages/eslint-plugin/src/configs/disable-type-checked.ts b/packages/eslint-plugin/src/configs/disable-type-checked.ts index c03bdbfd9a9..38a7ffd079d 100644 --- a/packages/eslint-plugin/src/configs/disable-type-checked.ts +++ b/packages/eslint-plugin/src/configs/disable-type-checked.ts @@ -37,6 +37,7 @@ export = { '@typescript-eslint/non-nullable-type-assertion-style': 'off', '@typescript-eslint/prefer-includes': 'off', '@typescript-eslint/prefer-nullish-coalescing': 'off', + '@typescript-eslint/prefer-optional-chain': 'off', '@typescript-eslint/prefer-readonly': 'off', '@typescript-eslint/prefer-readonly-parameter-types': 'off', '@typescript-eslint/prefer-reduce-type-parameter': 'off', diff --git a/packages/eslint-plugin/src/configs/stylistic.ts b/packages/eslint-plugin/src/configs/stylistic.ts index fd95427bef4..863a50eecda 100644 --- a/packages/eslint-plugin/src/configs/stylistic.ts +++ b/packages/eslint-plugin/src/configs/stylistic.ts @@ -24,6 +24,5 @@ export = { '@typescript-eslint/prefer-for-of': 'error', '@typescript-eslint/prefer-function-type': 'error', '@typescript-eslint/prefer-namespace-keyword': 'error', - '@typescript-eslint/prefer-optional-chain': 'error', }, }; diff --git a/packages/eslint-plugin/src/rules/class-literal-property-style.ts b/packages/eslint-plugin/src/rules/class-literal-property-style.ts index fbe74bef0e4..a58fdf68baf 100644 --- a/packages/eslint-plugin/src/rules/class-literal-property-style.ts +++ b/packages/eslint-plugin/src/rules/class-literal-property-style.ts @@ -65,7 +65,7 @@ export default util.createRule({ if ( node.kind !== 'get' || !node.value.body || - !node.value.body.body.length + node.value.body.body.length === 0 ) { return; } diff --git a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts index d4a0efd56e5..1ac7e70bb9f 100644 --- a/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts +++ b/packages/eslint-plugin/src/rules/explicit-module-boundary-types.ts @@ -229,7 +229,7 @@ export default util.createRule({ * Checks if a function name is allowed and should not be checked. */ function isAllowedName(node: TSESTree.Node | undefined): boolean { - if (!node || !options.allowedNames || !options.allowedNames.length) { + if (!node || !options.allowedNames || options.allowedNames.length === 0) { return false; } diff --git a/packages/eslint-plugin/src/rules/indent.ts b/packages/eslint-plugin/src/rules/indent.ts index 94c1899f2d4..f2f3805d800 100644 --- a/packages/eslint-plugin/src/rules/indent.ts +++ b/packages/eslint-plugin/src/rules/indent.ts @@ -198,7 +198,7 @@ export default util.createRule({ // transform it to a BinaryExpression return rules['BinaryExpression, LogicalExpression']({ type: AST_NODE_TYPES.BinaryExpression, - operator: 'as', + operator: 'as' as any, left: node.expression, // the first typeAnnotation includes the as token right: node.typeAnnotation as any, @@ -217,7 +217,7 @@ export default util.createRule({ test: { parent: node, type: AST_NODE_TYPES.BinaryExpression, - operator: 'extends', + operator: 'extends' as any, left: node.checkType as any, right: node.extendsType as any, diff --git a/packages/eslint-plugin/src/rules/no-extraneous-class.ts b/packages/eslint-plugin/src/rules/no-extraneous-class.ts index 8c6c64e089c..21dda4c686d 100644 --- a/packages/eslint-plugin/src/rules/no-extraneous-class.ts +++ b/packages/eslint-plugin/src/rules/no-extraneous-class.ts @@ -72,9 +72,8 @@ export default util.createRule({ ): boolean => { return !!( allowWithDecorator && - node && - node.decorators && - node.decorators.length + node?.decorators && + node.decorators.length !== 0 ); }; diff --git a/packages/eslint-plugin/src/rules/prefer-includes.ts b/packages/eslint-plugin/src/rules/prefer-includes.ts index a51746de5e0..8676f43b1ef 100644 --- a/packages/eslint-plugin/src/rules/prefer-includes.ts +++ b/packages/eslint-plugin/src/rules/prefer-includes.ts @@ -160,8 +160,7 @@ export default createRule({ .getProperty('includes') ?.getDeclarations(); if ( - includesMethodDecl == null || - !includesMethodDecl.some(includesMethodDecl => + !includesMethodDecl?.some(includesMethodDecl => hasSameParameters(includesMethodDecl, instanceofMethodDecl), ) ) { diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/PreferOptionalChainOptions.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/PreferOptionalChainOptions.ts new file mode 100644 index 00000000000..b755c946395 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/PreferOptionalChainOptions.ts @@ -0,0 +1,14 @@ +export type PreferOptionalChainMessageIds = + | 'preferOptionalChain' + | 'optionalChainSuggest'; + +export interface PreferOptionalChainOptions { + checkAny?: boolean; + checkUnknown?: boolean; + checkString?: boolean; + checkNumber?: boolean; + checkBoolean?: boolean; + checkBigInt?: boolean; + requireNullish?: boolean; + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing?: boolean; +} diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts new file mode 100644 index 00000000000..39b04504965 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/analyzeChain.ts @@ -0,0 +1,568 @@ +import type { + ParserServicesWithTypeInformation, + TSESTree, +} from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import type { + ReportDescriptor, + ReportFixFunction, + RuleContext, + SourceCode, +} from '@typescript-eslint/utils/ts-eslint'; +import { unionTypeParts } from 'ts-api-utils'; +import * as ts from 'typescript'; + +import * as util from '../../util'; +import { compareNodes, NodeComparisonResult } from './compareNodes'; +import type { ValidOperand } from './gatherLogicalOperands'; +import { NullishComparisonType } from './gatherLogicalOperands'; +import type { + PreferOptionalChainMessageIds, + PreferOptionalChainOptions, +} from './PreferOptionalChainOptions'; + +function includesType( + parserServices: ParserServicesWithTypeInformation, + node: TSESTree.Node, + typeFlagIn: ts.TypeFlags, +): boolean { + const typeFlag = typeFlagIn | ts.TypeFlags.Any | ts.TypeFlags.Unknown; + const types = unionTypeParts(parserServices.getTypeAtLocation(node)); + for (const type of types) { + if (util.isTypeFlagSet(type, typeFlag)) { + return true; + } + } + return false; +} + +// I hate that these functions are identical aside from the enum values used +// I can't think of a good way to reuse the code here in a way that will preserve +// the type safety and simplicity. + +type OperandAnalyzer = ( + parserServices: ParserServicesWithTypeInformation, + operand: ValidOperand, + index: number, + chain: readonly ValidOperand[], +) => readonly [ValidOperand] | readonly [ValidOperand, ValidOperand] | null; +const analyzeAndChainOperand: OperandAnalyzer = ( + parserServices, + operand, + index, + chain, +) => { + switch (operand.comparisonType) { + case NullishComparisonType.Boolean: + case NullishComparisonType.NotEqualNullOrUndefined: + return [operand]; + + case NullishComparisonType.NotStrictEqualNull: { + // handle `x !== null && x !== undefined` + const nextOperand = chain[index + 1] as ValidOperand | undefined; + if ( + nextOperand?.comparisonType === + NullishComparisonType.NotStrictEqualUndefined && + compareNodes(operand.comparedName, nextOperand.comparedName) === + NodeComparisonResult.Equal + ) { + return [operand, nextOperand]; + } + if ( + includesType( + parserServices, + operand.comparedName, + ts.TypeFlags.Undefined, + ) + ) { + // we know the next operand is not an `undefined` check and that this + // operand includes `undefined` - which means that making this an + // optional chain would change the runtime behavior of the expression + return null; + } + + return [operand]; + } + + case NullishComparisonType.NotStrictEqualUndefined: { + // handle `x !== undefined && x !== null` + const nextOperand = chain[index + 1] as ValidOperand | undefined; + if ( + nextOperand?.comparisonType === + NullishComparisonType.NotStrictEqualNull && + compareNodes(operand.comparedName, nextOperand.comparedName) === + NodeComparisonResult.Equal + ) { + return [operand, nextOperand]; + } + if ( + includesType(parserServices, operand.comparedName, ts.TypeFlags.Null) + ) { + // we know the next operand is not a `null` check and that this + // operand includes `null` - which means that making this an + // optional chain would change the runtime behavior of the expression + return null; + } + + return [operand]; + } + + default: + return null; + } +}; +const analyzeOrChainOperand: OperandAnalyzer = ( + parserServices, + operand, + index, + chain, +) => { + switch (operand.comparisonType) { + case NullishComparisonType.NotBoolean: + case NullishComparisonType.EqualNullOrUndefined: + return [operand]; + + case NullishComparisonType.StrictEqualNull: { + // handle `x === null || x === undefined` + const nextOperand = chain[index + 1] as ValidOperand | undefined; + if ( + nextOperand?.comparisonType === + NullishComparisonType.StrictEqualUndefined && + compareNodes(operand.comparedName, nextOperand.comparedName) === + NodeComparisonResult.Equal + ) { + return [operand, nextOperand]; + } + if ( + includesType( + parserServices, + operand.comparedName, + ts.TypeFlags.Undefined, + ) + ) { + // we know the next operand is not an `undefined` check and that this + // operand includes `undefined` - which means that making this an + // optional chain would change the runtime behavior of the expression + return null; + } + + return [operand]; + } + + case NullishComparisonType.StrictEqualUndefined: { + // handle `x === undefined || x === null` + const nextOperand = chain[index + 1] as ValidOperand | undefined; + if ( + nextOperand?.comparisonType === NullishComparisonType.StrictEqualNull && + compareNodes(operand.comparedName, nextOperand.comparedName) === + NodeComparisonResult.Equal + ) { + return [operand, nextOperand]; + } + if ( + includesType(parserServices, operand.comparedName, ts.TypeFlags.Null) + ) { + // we know the next operand is not a `null` check and that this + // operand includes `null` - which means that making this an + // optional chain would change the runtime behavior of the expression + return null; + } + + return [operand]; + } + + default: + return null; + } +}; + +function getFixer( + sourceCode: SourceCode, + parserServices: ParserServicesWithTypeInformation, + operator: '&&' | '||', + options: PreferOptionalChainOptions, + chain: ValidOperand[], +): + | { + suggest: NonNullable< + ReportDescriptor['suggest'] + >; + } + | { + fix: NonNullable['fix']>; + } { + const lastOperand = chain[chain.length - 1]; + + let useSuggestionFixer: boolean; + if ( + options.allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing === + true + ) { + // user has opted-in to the unsafe behavior + useSuggestionFixer = false; + } else { + // optional chain specifically will union `undefined` into the final type + // so we need to make sure that there is at least one operand that includes + // `undefined`, or else we're going to change the final type - which is + // unsafe and might cause downstream type errors. + + if ( + lastOperand.comparisonType === + NullishComparisonType.EqualNullOrUndefined || + lastOperand.comparisonType === + NullishComparisonType.NotEqualNullOrUndefined || + lastOperand.comparisonType === + NullishComparisonType.StrictEqualUndefined || + lastOperand.comparisonType === + NullishComparisonType.NotStrictEqualUndefined || + (operator === '||' && + lastOperand.comparisonType === NullishComparisonType.NotBoolean) + ) { + // we know the last operand is an equality check - so the change in types + // DOES NOT matter and will not change the runtime result or cause a type + // check error + useSuggestionFixer = false; + } else { + useSuggestionFixer = true; + + for (const operand of chain) { + if ( + includesType(parserServices, operand.node, ts.TypeFlags.Undefined) + ) { + useSuggestionFixer = false; + break; + } + } + + // TODO - we could further reduce the false-positive rate of this check by + // checking for cases where the change in types don't matter like + // the test location of an if/while/etc statement. + // but it's quite complex to do this without false-negatives, so + // for now we'll just be over-eager with our matching. + // + // it's MUCH better to false-positive here and only provide a + // suggestion fixer, rather than false-negative and autofix to + // broken code. + } + } + + // In its most naive form we could just slap `?.` for every single part of the + // chain. However this would be undesirable because it'd create unnecessary + // conditions in the user's code where there were none before - and it would + // cause errors with rules like our `no-unnecessary-condition`. + // + // Instead we want to include the minimum number of `?.` required to correctly + // unify the code into a single chain. Naively you might think that we can + // just take the final operand add `?.` after the locations from the previous + // operands - however this won't be correct either because earlier operands + // can include a necessary `?.` that's not needed or included in a later + // operand. + // + // So instead what we need to do is to start at the first operand and + // iteratively diff it against the next operand, and add the difference to the + // first operand. + // + // eg + // `foo && foo.bar && foo.bar.baz?.bam && foo.bar.baz.bam()` + // 1) `foo` + // 2) diff(`foo`, `foo.bar`) = `.bar` + // 3) result = `foo?.bar` + // 4) diff(`foo.bar`, `foo.bar.baz?.bam`) = `.baz?.bam` + // 5) result = `foo?.bar?.baz?.bam` + // 6) diff(`foo.bar.baz?.bam`, `foo.bar.baz.bam()`) = `()` + // 7) result = `foo?.bar?.baz?.bam?.()` + + const parts = []; + for (const current of chain) { + const nextOperand = flattenChainExpression( + sourceCode, + current.comparedName, + ); + const diff = nextOperand.slice(parts.length); + if (diff.length > 0) { + if (parts.length > 0) { + // we need to make the first operand of the diff optional so it matches the + // logic before merging + // foo.bar && foo.bar.baz + // diff = .baz + // result = foo.bar?.baz + diff[0].optional = true; + } + parts.push(...diff); + } + } + + let newCode = parts + .map(part => { + let str = ''; + if (part.optional) { + str += '?.'; + } else { + if (part.nonNull) { + str += '!'; + } + if (part.requiresDot) { + str += '.'; + } + } + if ( + part.precedence !== util.OperatorPrecedence.Invalid && + part.precedence < util.OperatorPrecedence.Member + ) { + str += `(${part.text})`; + } else { + str += part.text; + } + return str; + }) + .join(''); + + if (lastOperand.node.type === AST_NODE_TYPES.BinaryExpression) { + // retain the ending comparison for cases like + // x && x.a != null + // x && typeof x.a !== 'undefined' + const operator = lastOperand.node.operator; + const { left, right } = (() => { + if (lastOperand.isYoda) { + const unaryOperator = + lastOperand.node.right.type === AST_NODE_TYPES.UnaryExpression + ? lastOperand.node.right.operator + ' ' + : ''; + + return { + left: sourceCode.getText(lastOperand.node.left), + right: unaryOperator + newCode, + }; + } else { + const unaryOperator = + lastOperand.node.left.type === AST_NODE_TYPES.UnaryExpression + ? lastOperand.node.left.operator + ' ' + : ''; + return { + left: unaryOperator + newCode, + right: sourceCode.getText(lastOperand.node.right), + }; + } + })(); + + newCode = `${left} ${operator} ${right}`; + } else if (lastOperand.comparisonType === NullishComparisonType.NotBoolean) { + newCode = `!${newCode}`; + } + + const fix: ReportFixFunction = fixer => + fixer.replaceTextRange( + [chain[0].node.range[0], lastOperand.node.range[1]], + newCode, + ); + + return useSuggestionFixer + ? { suggest: [{ fix, messageId: 'optionalChainSuggest' }] } + : { fix }; + + interface FlattenedChain { + nonNull: boolean; + optional: boolean; + precedence: util.OperatorPrecedence; + requiresDot: boolean; + text: string; + } + function flattenChainExpression( + sourceCode: SourceCode, + node: TSESTree.Node, + ): FlattenedChain[] { + switch (node.type) { + case AST_NODE_TYPES.ChainExpression: + return flattenChainExpression(sourceCode, node.expression); + + case AST_NODE_TYPES.CallExpression: { + const argumentsText = (() => { + const closingParenToken = util.nullThrows( + sourceCode.getLastToken(node), + util.NullThrowsReasons.MissingToken( + 'closing parenthesis', + node.type, + ), + ); + const openingParenToken = util.nullThrows( + sourceCode.getFirstTokenBetween( + node.typeArguments ?? node.callee, + closingParenToken, + util.isOpeningParenToken, + ), + util.NullThrowsReasons.MissingToken( + 'opening parenthesis', + node.type, + ), + ); + return sourceCode.text.substring( + openingParenToken.range[0], + closingParenToken.range[1], + ); + })(); + + const typeArgumentsText = (() => { + if (node.typeArguments == null) { + return ''; + } + + return sourceCode.getText(node.typeArguments); + })(); + + return [ + ...flattenChainExpression(sourceCode, node.callee), + { + nonNull: false, + optional: node.optional, + // no precedence for this + precedence: util.OperatorPrecedence.Invalid, + requiresDot: false, + text: typeArgumentsText + argumentsText, + }, + ]; + } + + case AST_NODE_TYPES.MemberExpression: { + const propertyText = sourceCode.getText(node.property); + return [ + ...flattenChainExpression(sourceCode, node.object), + { + nonNull: node.object.type === AST_NODE_TYPES.TSNonNullExpression, + optional: node.optional, + precedence: node.computed + ? // computed is already wrapped in [] so no need to wrap in () as well + util.OperatorPrecedence.Invalid + : util.getOperatorPrecedenceForNode(node.property), + requiresDot: !node.computed, + text: node.computed ? `[${propertyText}]` : propertyText, + }, + ]; + } + + case AST_NODE_TYPES.TSNonNullExpression: + return flattenChainExpression(sourceCode, node.expression); + + default: + return [ + { + nonNull: false, + optional: false, + precedence: util.getOperatorPrecedenceForNode(node), + requiresDot: false, + text: sourceCode.getText(node), + }, + ]; + } + } +} + +export function analyzeChain( + context: RuleContext< + PreferOptionalChainMessageIds, + [PreferOptionalChainOptions] + >, + sourceCode: SourceCode, + parserServices: ParserServicesWithTypeInformation, + options: PreferOptionalChainOptions, + operator: TSESTree.LogicalExpression['operator'], + chain: ValidOperand[], +): void { + // need at least 2 operands in a chain for it to be a chain + if ( + chain.length <= 1 || + /* istanbul ignore next -- previous checks make this unreachable, but keep it for exhaustiveness check */ + operator === '??' + ) { + return; + } + + const analyzeOperand = (() => { + switch (operator) { + case '&&': + return analyzeAndChainOperand; + + case '||': + return analyzeOrChainOperand; + } + })(); + + let subChain: ValidOperand[] = []; + const maybeReportThenReset = ( + newChainSeed?: readonly ValidOperand[], + ): void => { + if (subChain.length > 1) { + context.report({ + messageId: 'preferOptionalChain', + loc: { + start: subChain[0].node.loc.start, + end: subChain[subChain.length - 1].node.loc.end, + }, + ...getFixer(sourceCode, parserServices, operator, options, subChain), + }); + } + + // we've reached the end of a chain of logical expressions + // i.e. the current operand doesn't belong to the previous chain. + // + // we don't want to throw away the current operand otherwise we will skip it + // and that can cause us to miss chains. So instead we seed the new chain + // with the current operand + // + // eg this means we can catch cases like: + // unrelated != null && foo != null && foo.bar != null; + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ first "chain" + // ^^^^^^^^^^^ newChainSeed + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ second chain + subChain = newChainSeed ? [...newChainSeed] : []; + }; + + for (let i = 0; i < chain.length; i += 1) { + const lastOperand = subChain[subChain.length - 1] as + | ValidOperand + | undefined; + const operand = chain[i]; + + const validatedOperands = analyzeOperand(parserServices, operand, i, chain); + if (!validatedOperands) { + // TODO - #7170 + // check if the name is a superset/equal - if it is, then it likely + // intended to be part of the chain and something we should include in the + // report, eg + // foo == null || foo.bar; + // ^^^^^^^^^^^ valid OR chain + // ^^^^^^^ invalid OR chain logical, but still part of + // the chain for combination purposes + + maybeReportThenReset(); + continue; + } + // in case multiple operands were consumed - make sure to correctly increment the index + i += validatedOperands.length - 1; + + const currentOperand = validatedOperands[0]; + if (lastOperand) { + const comparisonResult = compareNodes( + lastOperand.comparedName, + // purposely inspect and push the last operand because the prior operands don't matter + // this also means we won't false-positive in cases like + // foo !== null && foo !== undefined + validatedOperands[validatedOperands.length - 1].comparedName, + ); + if (comparisonResult === NodeComparisonResult.Subset) { + // the operands are comparable, so we can continue searching + subChain.push(currentOperand); + } else if (comparisonResult === NodeComparisonResult.Invalid) { + maybeReportThenReset(validatedOperands); + } else if (comparisonResult === NodeComparisonResult.Equal) { + // purposely don't push this case because the node is a no-op and if + // we consider it then we might report on things like + // foo && foo + } + } else { + subChain.push(currentOperand); + } + } + + // check the leftovers + maybeReportThenReset(); +} diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/compareNodes.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/compareNodes.ts new file mode 100644 index 00000000000..d9dce486ec9 --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/compareNodes.ts @@ -0,0 +1,412 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { visitorKeys } from '@typescript-eslint/visitor-keys'; + +export const enum NodeComparisonResult { + /** the two nodes are comparably the same */ + Equal = 'Equal', + /** the left node is a subset of the right node */ + Subset = 'Subset', + /** the left node is not the same or is a superset of the right node */ + Invalid = 'Invalid', +} + +function compareArrays( + arrayA: unknown[], + arrayB: unknown[], +): NodeComparisonResult.Equal | NodeComparisonResult.Invalid { + if (arrayA.length !== arrayB.length) { + return NodeComparisonResult.Invalid; + } + + const result = arrayA.every((elA, idx) => { + const elB = arrayB[idx]; + if (elA == null || elB == null) { + return elA === elB; + } + return compareUnknownValues(elA, elB) === NodeComparisonResult.Equal; + }); + if (result) { + return NodeComparisonResult.Equal; + } + return NodeComparisonResult.Invalid; +} + +function isValidNode(x: unknown): x is TSESTree.Node { + return ( + typeof x === 'object' && + x != null && + 'type' in x && + typeof x.type === 'string' + ); +} +function isValidChainExpressionToLookThrough( + node: TSESTree.Node, +): node is TSESTree.ChainExpression { + return ( + !( + node.parent?.type === AST_NODE_TYPES.MemberExpression && + node.parent.object === node + ) && + !( + node.parent?.type === AST_NODE_TYPES.CallExpression && + node.parent.callee === node + ) && + node.type === AST_NODE_TYPES.ChainExpression + ); +} +function compareUnknownValues( + valueA: unknown, + valueB: unknown, +): NodeComparisonResult { + /* istanbul ignore if -- not possible for us to test this - it's just a sanity safeguard */ + if (valueA == null || valueB == null) { + if (valueA !== valueB) { + return NodeComparisonResult.Invalid; + } + return NodeComparisonResult.Equal; + } + + /* istanbul ignore if -- not possible for us to test this - it's just a sanity safeguard */ + if (!isValidNode(valueA) || !isValidNode(valueB)) { + return NodeComparisonResult.Invalid; + } + + return compareNodes(valueA, valueB); +} +function compareByVisiting( + nodeA: TSESTree.Node, + nodeB: TSESTree.Node, +): NodeComparisonResult.Equal | NodeComparisonResult.Invalid { + const currentVisitorKeys = visitorKeys[nodeA.type]; + /* istanbul ignore if -- not possible for us to test this - it's just a sanity safeguard */ + if (currentVisitorKeys == null) { + // we don't know how to visit this node, so assume it's invalid to avoid false-positives / broken fixers + return NodeComparisonResult.Invalid; + } + + if (currentVisitorKeys.length === 0) { + // assume nodes with no keys are constant things like keywords + return NodeComparisonResult.Equal; + } + + for (const key of currentVisitorKeys) { + // @ts-expect-error - dynamic access but it's safe + const nodeAChildOrChildren = nodeA[key] as unknown; + // @ts-expect-error - dynamic access but it's safe + const nodeBChildOrChildren = nodeB[key] as unknown; + + if (Array.isArray(nodeAChildOrChildren)) { + const arrayA = nodeAChildOrChildren as unknown[]; + const arrayB = nodeBChildOrChildren as unknown[]; + + const result = compareArrays(arrayA, arrayB); + if (result !== NodeComparisonResult.Equal) { + return NodeComparisonResult.Invalid; + } + // fallthrough to the next key as the key was "equal" + } else { + const result = compareUnknownValues( + nodeAChildOrChildren, + nodeBChildOrChildren, + ); + if (result !== NodeComparisonResult.Equal) { + return NodeComparisonResult.Invalid; + } + // fallthrough to the next key as the key was "equal" + } + } + + return NodeComparisonResult.Equal; +} +type CompareNodesArgument = TSESTree.Node | null | undefined; +function compareNodesUncached( + nodeA: TSESTree.Node, + nodeB: TSESTree.Node, +): NodeComparisonResult { + if (nodeA.type !== nodeB.type) { + // special cases where nodes are allowed to be non-equal + + // look through a chain expression node at the top-level because it only + // exists to delimit the end of an optional chain + // + // a?.b && a.b.c + // ^^^^ ChainExpression, MemberExpression + // ^^^^^ MemberExpression + // + // except for in this class of cases + // (a?.b).c && a.b.c + // because the parentheses have runtime meaning (sad face) + if (isValidChainExpressionToLookThrough(nodeA)) { + return compareNodes(nodeA.expression, nodeB); + } + if (isValidChainExpressionToLookThrough(nodeB)) { + return compareNodes(nodeA, nodeB.expression); + } + + // look through the type-only non-null assertion because its existence could + // possibly be replaced by an optional chain instead + // + // a.b! && a.b.c + // ^^^^ TSNonNullExpression + if (nodeA.type === AST_NODE_TYPES.TSNonNullExpression) { + return compareNodes(nodeA.expression, nodeB); + } + if (nodeB.type === AST_NODE_TYPES.TSNonNullExpression) { + return compareNodes(nodeA, nodeB.expression); + } + + // special case for subset optional chains where the node types don't match, + // but we want to try comparing by discarding the "extra" code + // + // a && a.b + // ^ compare this + // a && a() + // ^ compare this + // a.b && a.b() + // ^^^ compare this + // a() && a().b + // ^^^ compare this + // import.meta && import.meta.b + // ^^^^^^^^^^^ compare this + if ( + nodeA.type === AST_NODE_TYPES.CallExpression || + nodeA.type === AST_NODE_TYPES.Identifier || + nodeA.type === AST_NODE_TYPES.MemberExpression || + nodeA.type === AST_NODE_TYPES.MetaProperty + ) { + switch (nodeB.type) { + case AST_NODE_TYPES.MemberExpression: + if (nodeB.property.type === AST_NODE_TYPES.PrivateIdentifier) { + // Private identifiers in optional chaining is not currently allowed + // TODO - handle this once TS supports it (https://github.com/microsoft/TypeScript/issues/42734) + return NodeComparisonResult.Invalid; + } + if ( + compareNodes(nodeA, nodeB.object) !== NodeComparisonResult.Invalid + ) { + return NodeComparisonResult.Subset; + } + return NodeComparisonResult.Invalid; + + case AST_NODE_TYPES.CallExpression: + if ( + compareNodes(nodeA, nodeB.callee) !== NodeComparisonResult.Invalid + ) { + return NodeComparisonResult.Subset; + } + return NodeComparisonResult.Invalid; + + default: + return NodeComparisonResult.Invalid; + } + } + + return NodeComparisonResult.Invalid; + } + + switch (nodeA.type) { + // these expressions create a new instance each time - so it makes no sense to compare the chain + case AST_NODE_TYPES.ArrayExpression: + case AST_NODE_TYPES.ArrowFunctionExpression: + case AST_NODE_TYPES.ClassExpression: + case AST_NODE_TYPES.FunctionExpression: + case AST_NODE_TYPES.JSXElement: + case AST_NODE_TYPES.JSXFragment: + case AST_NODE_TYPES.NewExpression: + case AST_NODE_TYPES.ObjectExpression: + return NodeComparisonResult.Invalid; + + // chaining from assignments could change the value irrevocably - so it makes no sense to compare the chain + case AST_NODE_TYPES.AssignmentExpression: + return NodeComparisonResult.Invalid; + + case AST_NODE_TYPES.CallExpression: { + const nodeBCall = nodeB as typeof nodeA; + + // check for cases like + // foo() && foo()(bar) + // ^^^^^ nodeA + // ^^^^^^^^^^ nodeB + // we don't want to check the arguments in this case + const aSubsetOfB = compareNodes(nodeA, nodeBCall.callee); + if (aSubsetOfB !== NodeComparisonResult.Invalid) { + return NodeComparisonResult.Subset; + } + + const calleeCompare = compareNodes(nodeA.callee, nodeBCall.callee); + if (calleeCompare !== NodeComparisonResult.Equal) { + return NodeComparisonResult.Invalid; + } + + // NOTE - we purposely ignore optional flag because for our purposes + // foo?.bar() && foo.bar?.()?.baz + // or + // foo.bar() && foo?.bar?.()?.baz + // are going to be exactly the same + + const argumentCompare = compareArrays( + nodeA.arguments, + nodeBCall.arguments, + ); + if (argumentCompare !== NodeComparisonResult.Equal) { + return NodeComparisonResult.Invalid; + } + + const typeParamCompare = compareNodes( + nodeA.typeArguments, + nodeBCall.typeArguments, + ); + if (typeParamCompare === NodeComparisonResult.Equal) { + return NodeComparisonResult.Equal; + } + + return NodeComparisonResult.Invalid; + } + + case AST_NODE_TYPES.ChainExpression: + // special case handling for ChainExpression because it's allowed to be a subset + return compareNodes(nodeA, (nodeB as typeof nodeA).expression); + + case AST_NODE_TYPES.Identifier: + case AST_NODE_TYPES.PrivateIdentifier: + if (nodeA.name === (nodeB as typeof nodeA).name) { + return NodeComparisonResult.Equal; + } + return NodeComparisonResult.Invalid; + + case AST_NODE_TYPES.Literal: { + const nodeBLiteral = nodeB as typeof nodeA; + if ( + nodeA.raw === nodeBLiteral.raw && + nodeA.value === nodeBLiteral.value + ) { + return NodeComparisonResult.Equal; + } + return NodeComparisonResult.Invalid; + } + + case AST_NODE_TYPES.MemberExpression: { + const nodeBMember = nodeB as typeof nodeA; + + if (nodeBMember.property.type === AST_NODE_TYPES.PrivateIdentifier) { + // Private identifiers in optional chaining is not currently allowed + // TODO - handle this once TS supports it (https://github.com/microsoft/TypeScript/issues/42734) + return NodeComparisonResult.Invalid; + } + + // check for cases like + // foo.bar && foo.bar.baz + // ^^^^^^^ nodeA + // ^^^^^^^^^^^ nodeB + // result === Equal + // + // foo.bar && foo.bar.baz.bam + // ^^^^^^^ nodeA + // ^^^^^^^^^^^^^^^ nodeB + // result === Subset + // + // we don't want to check the property in this case + const aSubsetOfB = compareNodes(nodeA, nodeBMember.object); + if (aSubsetOfB !== NodeComparisonResult.Invalid) { + return NodeComparisonResult.Subset; + } + + if (nodeA.computed !== nodeBMember.computed) { + return NodeComparisonResult.Invalid; + } + + // NOTE - we purposely ignore optional flag because for our purposes + // foo?.bar && foo.bar?.baz + // or + // foo.bar && foo?.bar?.baz + // are going to be exactly the same + + const objectCompare = compareNodes(nodeA.object, nodeBMember.object); + if (objectCompare !== NodeComparisonResult.Equal) { + return NodeComparisonResult.Invalid; + } + + return compareNodes(nodeA.property, nodeBMember.property); + } + case AST_NODE_TYPES.TSTemplateLiteralType: + case AST_NODE_TYPES.TemplateLiteral: { + const nodeBTemplate = nodeB as typeof nodeA; + const areQuasisEqual = + nodeA.quasis.length === nodeBTemplate.quasis.length && + nodeA.quasis.every((elA, idx) => { + const elB = nodeBTemplate.quasis[idx]; + return elA.value.cooked === elB.value.cooked; + }); + if (!areQuasisEqual) { + return NodeComparisonResult.Invalid; + } + + return NodeComparisonResult.Equal; + } + + case AST_NODE_TYPES.TemplateElement: { + const nodeBElement = nodeB as typeof nodeA; + if (nodeA.value.cooked === nodeBElement.value.cooked) { + return NodeComparisonResult.Equal; + } + return NodeComparisonResult.Invalid; + } + + // these aren't actually valid expressions. + // https://github.com/typescript-eslint/typescript-eslint/blob/20d7caee35ab84ae6381fdf04338c9e2b9e2bc48/packages/ast-spec/src/unions/Expression.ts#L37-L43 + case AST_NODE_TYPES.ArrayPattern: + case AST_NODE_TYPES.ObjectPattern: + /* istanbul ignore next */ + return NodeComparisonResult.Invalid; + + // update expression returns a number and also changes the value each time - so it makes no sense to compare the chain + case AST_NODE_TYPES.UpdateExpression: + return NodeComparisonResult.Invalid; + + // yield returns the value passed to the `next` function, so it may not be the same each time - so it makes no sense to compare the chain + case AST_NODE_TYPES.YieldExpression: + return NodeComparisonResult.Invalid; + + // general-case automatic handling of nodes to save us implementing every + // single case by hand. This just iterates the visitor keys to recursively + // check the children. + // + // Any specific logic cases or short-circuits should be listed as separate + // cases so that they don't fall into this generic handling + default: + return compareByVisiting(nodeA, nodeB); + } +} +const COMPARE_NODES_CACHE = new WeakMap< + TSESTree.Node, + WeakMap +>(); +/** + * Compares two nodes' ASTs to determine if the A is equal to or a subset of B + */ +export function compareNodes( + nodeA: CompareNodesArgument, + nodeB: CompareNodesArgument, +): NodeComparisonResult { + if (nodeA == null || nodeB == null) { + if (nodeA !== nodeB) { + return NodeComparisonResult.Invalid; + } + return NodeComparisonResult.Equal; + } + + const cached = COMPARE_NODES_CACHE.get(nodeA)?.get(nodeB); + if (cached) { + return cached; + } + + const result = compareNodesUncached(nodeA, nodeB); + let mapA = COMPARE_NODES_CACHE.get(nodeA); + if (mapA == null) { + mapA = new WeakMap(); + COMPARE_NODES_CACHE.set(nodeA, mapA); + } + mapA.set(nodeB, result); + return result; +} diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/gatherLogicalOperands.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/gatherLogicalOperands.ts new file mode 100644 index 00000000000..39f3a96f32d --- /dev/null +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain-utils/gatherLogicalOperands.ts @@ -0,0 +1,371 @@ +import type { + ParserServicesWithTypeInformation, + TSESTree, +} from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import { + isBigIntLiteralType, + isBooleanLiteralType, + isNumberLiteralType, + isStringLiteralType, + unionTypeParts, +} from 'ts-api-utils'; +import * as ts from 'typescript'; + +import * as util from '../../util'; +import type { PreferOptionalChainOptions } from './PreferOptionalChainOptions'; + +const enum ComparisonValueType { + Null = 'Null', // eslint-disable-line @typescript-eslint/internal/prefer-ast-types-enum + Undefined = 'Undefined', + UndefinedStringLiteral = 'UndefinedStringLiteral', +} +export const enum OperandValidity { + Valid = 'Valid', + Invalid = 'Invalid', +} +export const enum NullishComparisonType { + /** `x != null`, `x != undefined` */ + NotEqualNullOrUndefined = 'NotEqualNullOrUndefined', + /** `x == null`, `x == undefined` */ + EqualNullOrUndefined = 'EqualNullOrUndefined', + + /** `x !== null` */ + NotStrictEqualNull = 'NotStrictEqualNull', + /** `x === null` */ + StrictEqualNull = 'StrictEqualNull', + + /** `x !== undefined`, `typeof x !== 'undefined'` */ + NotStrictEqualUndefined = 'NotStrictEqualUndefined', + /** `x === undefined`, `typeof x === 'undefined'` */ + StrictEqualUndefined = 'StrictEqualUndefined', + + /** `!x` */ + NotBoolean = 'NotBoolean', + /** `x` */ + Boolean = 'Boolean', // eslint-disable-line @typescript-eslint/internal/prefer-ast-types-enum +} +export interface ValidOperand { + type: OperandValidity.Valid; + comparedName: TSESTree.Node; + comparisonType: NullishComparisonType; + isYoda: boolean; + node: TSESTree.Expression; +} +export interface InvalidOperand { + type: OperandValidity.Invalid; +} +type Operand = ValidOperand | InvalidOperand; + +const NULLISH_FLAGS = ts.TypeFlags.Null | ts.TypeFlags.Undefined; +function isValidFalseBooleanCheckType( + node: TSESTree.Node, + operator: TSESTree.LogicalExpression['operator'], + checkType: 'true' | 'false', + parserServices: ParserServicesWithTypeInformation, + options: PreferOptionalChainOptions, +): boolean { + const type = parserServices.getTypeAtLocation(node); + const types = unionTypeParts(type); + + const disallowFalseyLiteral = + (operator === '||' && checkType === 'false') || + (operator === '&&' && checkType === 'true'); + if (disallowFalseyLiteral) { + /* + ``` + declare const x: false | {a: string}; + x && x.a; + !x || x.a; + ``` + + We don't want to consider these two cases because the boolean expression + narrows out the non-nullish falsy cases - so converting the chain to `x?.a` + would introduce a build error + */ + if ( + types.some(t => isBooleanLiteralType(t) && t.intrinsicName === 'false') || + types.some(t => isStringLiteralType(t) && t.value === '') || + types.some(t => isNumberLiteralType(t) && t.value === 0) || + types.some(t => isBigIntLiteralType(t) && t.value.base10Value === '0') + ) { + return false; + } + } + + if (options.requireNullish === true) { + return types.some(t => util.isTypeFlagSet(t, NULLISH_FLAGS)); + } + + let allowedFlags = NULLISH_FLAGS | ts.TypeFlags.Object; + if (options.checkAny === true) { + allowedFlags |= ts.TypeFlags.Any; + } + if (options.checkUnknown === true) { + allowedFlags |= ts.TypeFlags.Unknown; + } + if (options.checkString === true) { + allowedFlags |= ts.TypeFlags.StringLike; + } + if (options.checkNumber === true) { + allowedFlags |= ts.TypeFlags.NumberLike; + } + if (options.checkBoolean === true) { + allowedFlags |= ts.TypeFlags.BooleanLike; + } + if (options.checkBigInt === true) { + allowedFlags |= ts.TypeFlags.BigIntLike; + } + return types.every(t => util.isTypeFlagSet(t, allowedFlags)); +} + +export function gatherLogicalOperands( + node: TSESTree.LogicalExpression, + parserServices: ParserServicesWithTypeInformation, + options: PreferOptionalChainOptions, +): { + operands: Operand[]; + newlySeenLogicals: Set; +} { + const result: Operand[] = []; + const { operands, newlySeenLogicals } = flattenLogicalOperands(node); + + for (const operand of operands) { + switch (operand.type) { + case AST_NODE_TYPES.BinaryExpression: { + // check for "yoda" style logical: null != x + + const { comparedExpression, comparedValue, isYoda } = (() => { + // non-yoda checks are by far the most common, so check for them first + const comparedValueRight = getComparisonValueType(operand.right); + if (comparedValueRight) { + return { + comparedExpression: operand.left, + comparedValue: comparedValueRight, + isYoda: false, + }; + } else { + return { + comparedExpression: operand.right, + comparedValue: getComparisonValueType(operand.left), + isYoda: true, + }; + } + })(); + + if (comparedValue === ComparisonValueType.UndefinedStringLiteral) { + if ( + comparedExpression.type === AST_NODE_TYPES.UnaryExpression && + comparedExpression.operator === 'typeof' + ) { + // typeof x === 'undefined' + result.push({ + type: OperandValidity.Valid, + comparedName: comparedExpression.argument, + comparisonType: operand.operator.startsWith('!') + ? NullishComparisonType.NotStrictEqualUndefined + : NullishComparisonType.StrictEqualUndefined, + isYoda, + node: operand, + }); + continue; + } + + // y === 'undefined' + result.push({ type: OperandValidity.Invalid }); + continue; + } + + switch (operand.operator) { + case '!=': + case '==': + if ( + comparedValue === ComparisonValueType.Null || + comparedValue === ComparisonValueType.Undefined + ) { + // x == null, x == undefined + result.push({ + type: OperandValidity.Valid, + comparedName: comparedExpression, + comparisonType: operand.operator.startsWith('!') + ? NullishComparisonType.NotEqualNullOrUndefined + : NullishComparisonType.EqualNullOrUndefined, + isYoda, + node: operand, + }); + continue; + } + // x == something :( + result.push({ type: OperandValidity.Invalid }); + continue; + + case '!==': + case '===': { + const comparedName = comparedExpression; + switch (comparedValue) { + case ComparisonValueType.Null: + result.push({ + type: OperandValidity.Valid, + comparedName, + comparisonType: operand.operator.startsWith('!') + ? NullishComparisonType.NotStrictEqualNull + : NullishComparisonType.StrictEqualNull, + isYoda, + node: operand, + }); + continue; + + case ComparisonValueType.Undefined: + result.push({ + type: OperandValidity.Valid, + comparedName, + comparisonType: operand.operator.startsWith('!') + ? NullishComparisonType.NotStrictEqualUndefined + : NullishComparisonType.StrictEqualUndefined, + isYoda, + node: operand, + }); + continue; + + default: + // x === something :( + result.push({ type: OperandValidity.Invalid }); + continue; + } + } + } + + result.push({ type: OperandValidity.Invalid }); + continue; + } + + case AST_NODE_TYPES.UnaryExpression: + if ( + operand.operator === '!' && + isValidFalseBooleanCheckType( + operand.argument, + node.operator, + 'false', + parserServices, + options, + ) + ) { + result.push({ + type: OperandValidity.Valid, + comparedName: operand.argument, + comparisonType: NullishComparisonType.NotBoolean, + isYoda: false, + node: operand, + }); + continue; + } + result.push({ type: OperandValidity.Invalid }); + continue; + + case AST_NODE_TYPES.LogicalExpression: + // explicitly ignore the mixed logical expression cases + result.push({ type: OperandValidity.Invalid }); + continue; + + default: + if ( + isValidFalseBooleanCheckType( + operand, + node.operator, + 'true', + parserServices, + options, + ) + ) { + result.push({ + type: OperandValidity.Valid, + comparedName: operand, + comparisonType: NullishComparisonType.Boolean, + isYoda: false, + node: operand, + }); + } else { + result.push({ type: OperandValidity.Invalid }); + } + continue; + } + } + + return { + operands: result, + newlySeenLogicals, + }; + + /* + The AST is always constructed such the first element is always the deepest element. + I.e. for this code: `foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz` + The AST will look like this: + { + left: { + left: { + left: foo + right: foo.bar + } + right: foo.bar.baz + } + right: foo.bar.baz.buzz + } + + So given any logical expression, we can perform a depth-first traversal to get + the operands in order. + + Note that this function purposely does not inspect mixed logical expressions + like `foo || foo.bar && foo.bar.baz` - separate selector + */ + function flattenLogicalOperands(node: TSESTree.LogicalExpression): { + operands: TSESTree.Expression[]; + newlySeenLogicals: Set; + } { + const operands: TSESTree.Expression[] = []; + const newlySeenLogicals = new Set([node]); + + const stack: TSESTree.Expression[] = [node.right, node.left]; + let current: TSESTree.Expression | undefined; + while ((current = stack.pop())) { + if ( + current.type === AST_NODE_TYPES.LogicalExpression && + current.operator === node.operator + ) { + newlySeenLogicals.add(current); + stack.push(current.right); + stack.push(current.left); + } else { + operands.push(current); + } + } + + return { + operands, + newlySeenLogicals, + }; + } + + function getComparisonValueType( + node: TSESTree.Node, + ): ComparisonValueType | null { + switch (node.type) { + case AST_NODE_TYPES.Literal: + // eslint-disable-next-line eqeqeq -- intentional exact comparison against null + if (node.value === null && node.raw === 'null') { + return ComparisonValueType.Null; + } + if (node.value === 'undefined') { + return ComparisonValueType.UndefinedStringLiteral; + } + return null; + + case AST_NODE_TYPES.Identifier: + if (node.name === 'undefined') { + return ComparisonValueType.Undefined; + } + return null; + } + + return null; + } +} diff --git a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts index e0662cd0348..f88e37cb737 100644 --- a/packages/eslint-plugin/src/rules/prefer-optional-chain.ts +++ b/packages/eslint-plugin/src/rules/prefer-optional-chain.ts @@ -1,36 +1,24 @@ -import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import type { TSESTree } from '@typescript-eslint/utils'; import { AST_NODE_TYPES } from '@typescript-eslint/utils'; +import type { RuleFix } from '@typescript-eslint/utils/ts-eslint'; import * as ts from 'typescript'; import * as util from '../util'; - -type ValidChainTarget = - | TSESTree.BinaryExpression - | TSESTree.CallExpression - | TSESTree.ChainExpression - | TSESTree.Identifier - | TSESTree.MemberExpression - | TSESTree.MetaProperty - | TSESTree.PrivateIdentifier - | TSESTree.ThisExpression; - -/* -The AST is always constructed such the first element is always the deepest element. -I.e. for this code: `foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz` -The AST will look like this: -{ - left: { - left: { - left: foo - right: foo.bar - } - right: foo.bar.baz - } - right: foo.bar.baz.buzz -} -*/ - -export default util.createRule({ +import { analyzeChain } from './prefer-optional-chain-utils/analyzeChain'; +import type { ValidOperand } from './prefer-optional-chain-utils/gatherLogicalOperands'; +import { + gatherLogicalOperands, + OperandValidity, +} from './prefer-optional-chain-utils/gatherLogicalOperands'; +import type { + PreferOptionalChainMessageIds, + PreferOptionalChainOptions, +} from './prefer-optional-chain-utils/PreferOptionalChainOptions'; + +export default util.createRule< + [PreferOptionalChainOptions], + PreferOptionalChainMessageIds +>({ name: 'prefer-optional-chain', meta: { type: 'suggestion', @@ -38,21 +26,84 @@ export default util.createRule({ description: 'Enforce using concise optional chain expressions instead of chained logical ands, negated logical ors, or empty objects', recommended: 'stylistic', + requiresTypeChecking: true, }, + fixable: 'code', hasSuggestions: true, messages: { preferOptionalChain: "Prefer using an optional chain expression instead, as it's more concise and easier to read.", optionalChainSuggest: 'Change to an optional chain.', }, - schema: [], + schema: [ + { + type: 'object', + additionalProperties: false, + properties: { + checkAny: { + type: 'boolean', + description: + 'Check operands that are typed as `any` when inspecting "loose boolean" operands.', + }, + checkUnknown: { + type: 'boolean', + description: + 'Check operands that are typed as `unknown` when inspecting "loose boolean" operands.', + }, + checkString: { + type: 'boolean', + description: + 'Check operands that are typed as `string` when inspecting "loose boolean" operands.', + }, + checkNumber: { + type: 'boolean', + description: + 'Check operands that are typed as `number` when inspecting "loose boolean" operands.', + }, + checkBoolean: { + type: 'boolean', + description: + 'Check operands that are typed as `boolean` when inspecting "loose boolean" operands.', + }, + checkBigInt: { + type: 'boolean', + description: + 'Check operands that are typed as `bigint` when inspecting "loose boolean" operands.', + }, + requireNullish: { + type: 'boolean', + description: + 'Skip operands that are not typed with `null` and/or `undefined` when inspecting "loose boolean" operands.', + }, + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing: { + type: 'boolean', + description: + 'Allow autofixers that will change the return type of the expression. This option is considered unsafe as it may break the build.', + }, + }, + }, + ], }, - defaultOptions: [], - create(context) { + defaultOptions: [ + { + checkAny: true, + checkUnknown: true, + checkString: true, + checkNumber: true, + checkBoolean: true, + checkBigInt: true, + requireNullish: false, + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing: false, + }, + ], + create(context, [options]) { const sourceCode = context.getSourceCode(); - const services = util.getParserServices(context, true); + const parserServices = util.getParserServices(context); + + const seenLogicals = new Set(); return { + // specific handling for `(foo ?? {}).bar` / `(foo || {}).bar` 'LogicalExpression[operator="||"], LogicalExpression[operator="??"]'( node: TSESTree.LogicalExpression, ): void { @@ -71,10 +122,12 @@ export default util.createRule({ return; } + seenLogicals.add(node); + function isLeftSideLowerPrecedence(): boolean { - const logicalTsNode = services.esTreeNodeToTSNodeMap.get(node); + const logicalTsNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const leftTsNode = services.esTreeNodeToTSNodeMap.get(leftNode); + const leftTsNode = parserServices.esTreeNodeToTSNodeMap.get(leftNode); const operator = ts.isBinaryExpression(logicalTsNode) ? logicalTsNode.operatorToken.kind : ts.SyntaxKind.Unknown; @@ -87,11 +140,11 @@ export default util.createRule({ } context.report({ node: parentNode, - messageId: 'optionalChainSuggest', + messageId: 'preferOptionalChain', suggest: [ { messageId: 'optionalChainSuggest', - fix: (fixer): TSESLint.RuleFix => { + fix: (fixer): RuleFix => { const leftNodeText = sourceCode.getText(leftNode); // Any node that is made of an operator with higher or equal precedence, const maybeWrappedLeftNode = isLeftSideLowerPrecedence() @@ -112,558 +165,53 @@ export default util.createRule({ ], }); }, - [[ - 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > Identifier', - 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > MemberExpression', - 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > ChainExpression > MemberExpression', - 'LogicalExpression[operator="||"] > UnaryExpression[operator="!"] > MetaProperty', - ].join(',')]( - initialIdentifierOrNotEqualsExpr: - | TSESTree.Identifier - | TSESTree.MemberExpression - | TSESTree.MetaProperty, - ): void { - // selector guarantees this cast - const initialExpression = ( - initialIdentifierOrNotEqualsExpr.parent.type === - AST_NODE_TYPES.ChainExpression - ? initialIdentifierOrNotEqualsExpr.parent.parent - : initialIdentifierOrNotEqualsExpr.parent - )!.parent as TSESTree.LogicalExpression; - - if ( - initialExpression.left.type !== AST_NODE_TYPES.UnaryExpression || - initialExpression.left.argument !== initialIdentifierOrNotEqualsExpr - ) { - // the node(identifier or member expression) is not the deepest left node - return; - } - - // walk up the tree to figure out how many logical expressions we can include - let previous: TSESTree.LogicalExpression = initialExpression; - let current: TSESTree.Node = initialExpression; - let previousLeftText = getText(initialIdentifierOrNotEqualsExpr); - let optionallyChainedCode = previousLeftText; - let expressionCount = 1; - while (current.type === AST_NODE_TYPES.LogicalExpression) { - if ( - current.right.type !== AST_NODE_TYPES.UnaryExpression || - !isValidChainTarget( - current.right.argument, - // only allow unary '!' with identifiers for the first chain - !foo || !foo() - expressionCount === 1, - ) - ) { - break; - } - const { rightText, shouldBreak } = breakIfInvalid({ - rightNode: current.right.argument, - previousLeftText, - }); - if (shouldBreak) { - break; - } - let invalidOptionallyChainedPrivateProperty; - ({ - invalidOptionallyChainedPrivateProperty, - expressionCount, - previousLeftText, - optionallyChainedCode, - previous, - current, - } = normalizeRepeatingPatterns( - rightText, - expressionCount, - previousLeftText, - optionallyChainedCode, - previous, - current, - )); - if (invalidOptionallyChainedPrivateProperty) { - return; - } - } - - reportIfMoreThanOne({ - expressionCount, - previous, - optionallyChainedCode, - sourceCode, - context, - shouldHandleChainedAnds: false, - }); - }, - [[ - 'LogicalExpression[operator="&&"] > Identifier', - 'LogicalExpression[operator="&&"] > MemberExpression', - 'LogicalExpression[operator="&&"] > ChainExpression > MemberExpression', - 'LogicalExpression[operator="&&"] > MetaProperty', - 'LogicalExpression[operator="&&"] > BinaryExpression[operator="!=="]', - 'LogicalExpression[operator="&&"] > BinaryExpression[operator="!="]', - ].join(',')]( - initialIdentifierOrNotEqualsExpr: - | TSESTree.BinaryExpression - | TSESTree.Identifier - | TSESTree.MemberExpression - | TSESTree.MetaProperty, + 'LogicalExpression[operator!="??"]'( + node: TSESTree.LogicalExpression, ): void { - // selector guarantees this cast - const initialExpression = ( - initialIdentifierOrNotEqualsExpr.parent?.type === - AST_NODE_TYPES.ChainExpression - ? initialIdentifierOrNotEqualsExpr.parent.parent - : initialIdentifierOrNotEqualsExpr.parent - ) as TSESTree.LogicalExpression; - - if (initialExpression.left !== initialIdentifierOrNotEqualsExpr) { - // the node(identifier or member expression) is not the deepest left node - return; - } - if (!isValidChainTarget(initialIdentifierOrNotEqualsExpr, true)) { + if (seenLogicals.has(node)) { return; } - // walk up the tree to figure out how many logical expressions we can include - let previous: TSESTree.LogicalExpression = initialExpression; - let current: TSESTree.Node = initialExpression; - let previousLeftText = getText(initialIdentifierOrNotEqualsExpr); - let optionallyChainedCode = previousLeftText; - let expressionCount = 1; - while (current.type === AST_NODE_TYPES.LogicalExpression) { - if ( - !isValidChainTarget( - current.right, - // only allow identifiers for the first chain - foo && foo() - expressionCount === 1, - ) - ) { - break; - } - const { rightText, shouldBreak } = breakIfInvalid({ - rightNode: current.right, - previousLeftText, - }); - if (shouldBreak) { - break; - } - - let invalidOptionallyChainedPrivateProperty; - ({ - invalidOptionallyChainedPrivateProperty, - expressionCount, - previousLeftText, - optionallyChainedCode, - previous, - current, - } = normalizeRepeatingPatterns( - rightText, - expressionCount, - previousLeftText, - optionallyChainedCode, - previous, - current, - )); - if (invalidOptionallyChainedPrivateProperty) { - return; - } - } - - reportIfMoreThanOne({ - expressionCount, - previous, - optionallyChainedCode, - sourceCode, - context, - shouldHandleChainedAnds: true, - }); - }, - }; - - interface BreakIfInvalidResult { - leftText: string; - rightText: string; - shouldBreak: boolean; - } - - interface BreakIfInvalidOptions { - previousLeftText: string; - rightNode: ValidChainTarget; - } - - function breakIfInvalid({ - previousLeftText, - rightNode, - }: BreakIfInvalidOptions): BreakIfInvalidResult { - let shouldBreak = false; - - const rightText = getText(rightNode); - // can't just use startsWith because of cases like foo && fooBar.baz; - const matchRegex = new RegExp( - `^${ - // escape regex characters - previousLeftText.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') - }[^a-zA-Z0-9_$]`, - ); - if ( - !matchRegex.test(rightText) && - // handle redundant cases like foo.bar && foo.bar - previousLeftText !== rightText - ) { - shouldBreak = true; - } - return { shouldBreak, leftText: previousLeftText, rightText }; - } - - function getText(node: ValidChainTarget): string { - if (node.type === AST_NODE_TYPES.BinaryExpression) { - return getText( - // isValidChainTarget ensures this is type safe - node.left as ValidChainTarget, - ); - } - - if (node.type === AST_NODE_TYPES.CallExpression) { - const calleeText = getText( - // isValidChainTarget ensures this is type safe - node.callee as ValidChainTarget, - ); - - // ensure that the call arguments are left untouched, or else we can break cases that _need_ whitespace: - // - JSX: - // - Unary Operators: typeof foo, await bar, delete baz - const closingParenToken = util.nullThrows( - sourceCode.getLastToken(node), - util.NullThrowsReasons.MissingToken('closing parenthesis', node.type), + const { operands, newlySeenLogicals } = gatherLogicalOperands( + node, + parserServices, + options, ); - const openingParenToken = util.nullThrows( - sourceCode.getFirstTokenBetween( - node.callee, - closingParenToken, - util.isOpeningParenToken, - ), - util.NullThrowsReasons.MissingToken('opening parenthesis', node.type), - ); - - const argumentsText = sourceCode.text.substring( - openingParenToken.range[0], - closingParenToken.range[1], - ); - - return `${calleeText}${argumentsText}`; - } - - if ( - node.type === AST_NODE_TYPES.Identifier || - node.type === AST_NODE_TYPES.PrivateIdentifier - ) { - return node.name; - } - - if (node.type === AST_NODE_TYPES.MetaProperty) { - return `${node.meta.name}.${node.property.name}`; - } - if (node.type === AST_NODE_TYPES.ThisExpression) { - return 'this'; - } - - if (node.type === AST_NODE_TYPES.ChainExpression) { - /* istanbul ignore if */ if ( - node.expression.type === AST_NODE_TYPES.TSNonNullExpression - ) { - // this shouldn't happen - return ''; + for (const logical of newlySeenLogicals) { + seenLogicals.add(logical); } - return getText(node.expression); - } - - if (node.object.type === AST_NODE_TYPES.TSNonNullExpression) { - // Not supported mixing with TSNonNullExpression - return ''; - } - - return getMemberExpressionText(node); - } - - /** - * Gets a normalized representation of the given MemberExpression - */ - function getMemberExpressionText(node: TSESTree.MemberExpression): string { - let objectText: string; - // cases should match the list in ALLOWED_MEMBER_OBJECT_TYPES - switch (node.object.type) { - 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: - return ''; - } - - let propertyText: string; - if (node.computed) { - // cases should match the list in ALLOWED_COMPUTED_PROP_TYPES - switch (node.property.type) { - case AST_NODE_TYPES.Identifier: - propertyText = getText(node.property); - break; - - case AST_NODE_TYPES.Literal: - case AST_NODE_TYPES.TemplateLiteral: - case AST_NODE_TYPES.BinaryExpression: - propertyText = sourceCode.getText(node.property); - break; - - case AST_NODE_TYPES.MemberExpression: - propertyText = getMemberExpressionText(node.property); - break; - - /* istanbul ignore next */ - default: - return ''; + let currentChain: ValidOperand[] = []; + for (const operand of operands) { + if (operand.type === OperandValidity.Invalid) { + analyzeChain( + context, + sourceCode, + parserServices, + options, + node.operator, + currentChain, + ); + currentChain = []; + } else { + currentChain.push(operand); + } } - return `${objectText}${node.optional ? '?.' : ''}[${propertyText}]`; - } else { - // cases should match the list in ALLOWED_NON_COMPUTED_PROP_TYPES - switch (node.property.type) { - 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); + // make sure to check whatever's left + if (currentChain.length > 0) { + analyzeChain( + context, + sourceCode, + parserServices, + options, + node.operator, + currentChain, + ); } - - return `${objectText}${node.optional ? '?.' : '.'}${propertyText}`; - } - } + }, + }; }, }); - -const ALLOWED_MEMBER_OBJECT_TYPES: ReadonlySet = new Set([ - AST_NODE_TYPES.CallExpression, - AST_NODE_TYPES.Identifier, - 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.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 { - expressionCount: number; - previous: TSESTree.LogicalExpression; - optionallyChainedCode: string; - sourceCode: Readonly; - context: Readonly< - TSESLint.RuleContext< - 'optionalChainSuggest' | 'preferOptionalChain', - never[] - > - >; - shouldHandleChainedAnds: boolean; -} - -function reportIfMoreThanOne({ - expressionCount, - previous, - optionallyChainedCode, - sourceCode, - context, - shouldHandleChainedAnds, -}: ReportIfMoreThanOneOptions): void { - if (expressionCount > 1) { - if ( - shouldHandleChainedAnds && - previous.right.type === AST_NODE_TYPES.BinaryExpression - ) { - let operator = previous.right.operator; - if ( - previous.right.operator === '!==' && - // TODO(#4820): Use the type checker to know whether this is `null` - previous.right.right.type === AST_NODE_TYPES.Literal && - previous.right.right.raw === 'null' - ) { - // case like foo !== null && foo.bar !== null - operator = '!='; - } - // case like foo && foo.bar !== someValue - optionallyChainedCode += ` ${operator} ${sourceCode.getText( - previous.right.right, - )}`; - } - - context.report({ - node: previous, - messageId: 'preferOptionalChain', - suggest: [ - { - messageId: 'optionalChainSuggest', - fix: (fixer): TSESLint.RuleFix[] => [ - fixer.replaceText( - previous, - `${shouldHandleChainedAnds ? '' : '!'}${optionallyChainedCode}`, - ), - ], - }, - ], - }); - } -} - -interface NormalizedPattern { - invalidOptionallyChainedPrivateProperty: boolean; - expressionCount: number; - previousLeftText: string; - optionallyChainedCode: string; - previous: TSESTree.LogicalExpression; - current: TSESTree.Node; -} - -function normalizeRepeatingPatterns( - rightText: string, - expressionCount: number, - previousLeftText: string, - optionallyChainedCode: string, - previous: TSESTree.Node, - 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; - previousLeftText = rightText; - - /* - Diff the left and right text to construct the fix string - There are the following cases: - - 1) - rightText === 'foo.bar.baz.buzz' - leftText === 'foo.bar.baz' - diff === '.buzz' - - 2) - rightText === 'foo.bar.baz.buzz()' - leftText === 'foo.bar.baz' - diff === '.buzz()' - - 3) - rightText === 'foo.bar.baz.buzz()' - leftText === 'foo.bar.baz.buzz' - diff === '()' - - 4) - rightText === 'foo.bar.baz[buzz]' - leftText === 'foo.bar.baz' - diff === '[buzz]' - - 5) - rightText === 'foo.bar.baz?.buzz' - leftText === 'foo.bar.baz' - 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; - } else { - const needsDot = diff.startsWith('(') || diff.startsWith('['); - optionallyChainedCode += `?${needsDot ? '.' : ''}${diff}`; - } - } - - previous = current as TSESTree.LogicalExpression; - current = util.nullThrows( - current.parent, - util.NullThrowsReasons.MissingParent, - ); - return { - invalidOptionallyChainedPrivateProperty, - expressionCount, - previousLeftText, - optionallyChainedCode, - previous, - current, - }; -} - -function isValidChainTarget( - node: TSESTree.Node, - allowIdentifier: boolean, -): node is ValidChainTarget { - if (node.type === AST_NODE_TYPES.ChainExpression) { - return isValidChainTarget(node.expression, allowIdentifier); - } - - if (node.type === AST_NODE_TYPES.MemberExpression) { - const isObjectValid = - ALLOWED_MEMBER_OBJECT_TYPES.has(node.object.type) && - // make sure to validate the expression is of our expected structure - isValidChainTarget(node.object, true); - const isPropertyValid = node.computed - ? ALLOWED_COMPUTED_PROP_TYPES.has(node.property.type) && - // make sure to validate the member expression is of our expected structure - (node.property.type === AST_NODE_TYPES.MemberExpression - ? isValidChainTarget(node.property, allowIdentifier) - : true) - : ALLOWED_NON_COMPUTED_PROP_TYPES.has(node.property.type); - - return isObjectValid && isPropertyValid; - } - - if (node.type === AST_NODE_TYPES.CallExpression) { - return isValidChainTarget(node.callee, allowIdentifier); - } - - if ( - allowIdentifier && - (node.type === AST_NODE_TYPES.Identifier || - node.type === AST_NODE_TYPES.ThisExpression || - node.type === AST_NODE_TYPES.MetaProperty) - ) { - return true; - } - - /* - special case for the following, where we only want the left - - foo !== null - - foo != null - - foo !== undefined - - foo != undefined - */ - return ( - node.type === AST_NODE_TYPES.BinaryExpression && - ['!==', '!='].includes(node.operator) && - isValidChainTarget(node.left, allowIdentifier) && - (util.isUndefinedIdentifier(node.right) || util.isNullLiteral(node.right)) - ); -} diff --git a/packages/eslint-plugin/src/util/getOperatorPrecedence.ts b/packages/eslint-plugin/src/util/getOperatorPrecedence.ts index b7a9d75fb15..abbcb9191a5 100644 --- a/packages/eslint-plugin/src/util/getOperatorPrecedence.ts +++ b/packages/eslint-plugin/src/util/getOperatorPrecedence.ts @@ -1,3 +1,5 @@ +import type { TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import { SyntaxKind } from 'typescript'; export enum OperatorPrecedence { @@ -192,12 +194,115 @@ export enum OperatorPrecedence { Invalid = -1, } +export function getOperatorPrecedenceForNode( + node: TSESTree.Node, +): OperatorPrecedence { + switch (node.type) { + case AST_NODE_TYPES.SpreadElement: + case AST_NODE_TYPES.RestElement: + return OperatorPrecedence.Spread; + + case AST_NODE_TYPES.YieldExpression: + return OperatorPrecedence.Yield; + + case AST_NODE_TYPES.ConditionalExpression: + return OperatorPrecedence.Conditional; + + case AST_NODE_TYPES.SequenceExpression: + return OperatorPrecedence.Comma; + + case AST_NODE_TYPES.AssignmentExpression: + case AST_NODE_TYPES.BinaryExpression: + case AST_NODE_TYPES.LogicalExpression: + switch (node.operator) { + case '==': + case '+=': + case '-=': + case '**=': + case '*=': + case '/=': + case '%=': + case '<<=': + case '>>=': + case '>>>=': + case '&=': + case '^=': + case '|=': + case '||=': + case '&&=': + case '??=': + return OperatorPrecedence.Assignment; + + default: + return getBinaryOperatorPrecedence(node.operator); + } + + case AST_NODE_TYPES.TSTypeAssertion: + case AST_NODE_TYPES.TSNonNullExpression: + case AST_NODE_TYPES.UnaryExpression: + case AST_NODE_TYPES.AwaitExpression: + return OperatorPrecedence.Unary; + + case AST_NODE_TYPES.UpdateExpression: + // TODO: Should prefix `++` and `--` be moved to the `Update` precedence? + if (node.prefix) { + return OperatorPrecedence.Unary; + } + return OperatorPrecedence.Update; + + case AST_NODE_TYPES.ChainExpression: + return getOperatorPrecedenceForNode(node.expression); + + case AST_NODE_TYPES.CallExpression: + return OperatorPrecedence.LeftHandSide; + + case AST_NODE_TYPES.NewExpression: + return node.arguments.length > 0 + ? OperatorPrecedence.Member + : OperatorPrecedence.LeftHandSide; + + case AST_NODE_TYPES.TaggedTemplateExpression: + case AST_NODE_TYPES.MemberExpression: + case AST_NODE_TYPES.MetaProperty: + return OperatorPrecedence.Member; + + case AST_NODE_TYPES.TSAsExpression: + return OperatorPrecedence.Relational; + + case AST_NODE_TYPES.ThisExpression: + case AST_NODE_TYPES.Super: + case AST_NODE_TYPES.Identifier: + case AST_NODE_TYPES.PrivateIdentifier: + case AST_NODE_TYPES.Literal: + case AST_NODE_TYPES.ArrayExpression: + case AST_NODE_TYPES.ObjectExpression: + case AST_NODE_TYPES.FunctionExpression: + case AST_NODE_TYPES.ArrowFunctionExpression: + case AST_NODE_TYPES.ClassExpression: + case AST_NODE_TYPES.TemplateLiteral: + case AST_NODE_TYPES.JSXElement: + case AST_NODE_TYPES.JSXFragment: + // we don't have nodes for these cases + // case SyntaxKind.ParenthesizedExpression: + // case SyntaxKind.OmittedExpression: + return OperatorPrecedence.Primary; + + default: + return OperatorPrecedence.Invalid; + } +} + +type ValueOf = T[keyof T]; +type TSESTreeOperatorKind = + | ValueOf + | ValueOf; export function getOperatorPrecedence( nodeKind: SyntaxKind, operatorKind: SyntaxKind, hasArguments?: boolean, ): OperatorPrecedence { switch (nodeKind) { + // A list of comma-separated expressions. This node is only created by transformations. case SyntaxKind.CommaListExpression: return OperatorPrecedence.Comma; @@ -298,46 +403,83 @@ export function getOperatorPrecedence( } export function getBinaryOperatorPrecedence( - kind: SyntaxKind, + kind: SyntaxKind | TSESTreeOperatorKind, ): OperatorPrecedence { switch (kind) { case SyntaxKind.QuestionQuestionToken: + case '??': return OperatorPrecedence.Coalesce; + case SyntaxKind.BarBarToken: + case '||': return OperatorPrecedence.LogicalOR; + case SyntaxKind.AmpersandAmpersandToken: + case '&&': return OperatorPrecedence.LogicalAND; + case SyntaxKind.BarToken: + case '|': return OperatorPrecedence.BitwiseOR; + case SyntaxKind.CaretToken: + case '^': return OperatorPrecedence.BitwiseXOR; + case SyntaxKind.AmpersandToken: + case '&': return OperatorPrecedence.BitwiseAND; + case SyntaxKind.EqualsEqualsToken: + case '==': case SyntaxKind.ExclamationEqualsToken: + case '!=': case SyntaxKind.EqualsEqualsEqualsToken: + case '===': case SyntaxKind.ExclamationEqualsEqualsToken: + case '!==': return OperatorPrecedence.Equality; + case SyntaxKind.LessThanToken: + case '<': case SyntaxKind.GreaterThanToken: + case '>': case SyntaxKind.LessThanEqualsToken: + case '<=': case SyntaxKind.GreaterThanEqualsToken: + case '>=': case SyntaxKind.InstanceOfKeyword: + case 'instanceof': case SyntaxKind.InKeyword: + case 'in': case SyntaxKind.AsKeyword: + // case 'as': -- we don't have a token for this return OperatorPrecedence.Relational; + case SyntaxKind.LessThanLessThanToken: + case '<<': case SyntaxKind.GreaterThanGreaterThanToken: + case '>>': case SyntaxKind.GreaterThanGreaterThanGreaterThanToken: + case '>>>': return OperatorPrecedence.Shift; + case SyntaxKind.PlusToken: + case '+': case SyntaxKind.MinusToken: + case '-': return OperatorPrecedence.Additive; + case SyntaxKind.AsteriskToken: + case '*': case SyntaxKind.SlashToken: + case '/': case SyntaxKind.PercentToken: + case '%': return OperatorPrecedence.Multiplicative; + case SyntaxKind.AsteriskAsteriskToken: + case '**': return OperatorPrecedence.Exponentiation; } diff --git a/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts b/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts index 3951b90b5f6..4ea5f6cc087 100644 --- a/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts +++ b/packages/eslint-plugin/tests/rules/prefer-optional-chain/base-cases.ts @@ -1,228 +1,283 @@ -import type { TSESLint } from '@typescript-eslint/utils'; +import type { InvalidTestCase } from '@typescript-eslint/utils/ts-eslint'; -import type rule from '../../../src/rules/prefer-optional-chain'; import type { - InferMessageIdsTypeFromRule, - InferOptionsTypeFromRule, -} from '../../../src/util'; + PreferOptionalChainMessageIds, + PreferOptionalChainOptions, +} from '../../../src/rules/prefer-optional-chain-utils/PreferOptionalChainOptions'; -type InvalidTestCase = TSESLint.InvalidTestCase< - InferMessageIdsTypeFromRule, - InferOptionsTypeFromRule ->; +type MutateFn = (c: string) => string; +type BaseCaseCreator = (args: { + operator: '&&' | '||'; + mutateCode?: MutateFn; + mutateOutput?: MutateFn; + mutateDeclaration?: MutateFn; + useSuggestionFixer?: true; + skipIds?: number[]; +}) => InvalidTestCase< + PreferOptionalChainMessageIds, + [PreferOptionalChainOptions] +>[]; -interface BaseCase { - canReplaceAndWithOr: boolean; - output: string; - code: string; -} - -const mapper = (c: BaseCase): InvalidTestCase => ({ - code: c.code.trim(), - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: c.output.trim(), - }, - ], - }, - ], -}); - -const baseCases: BaseCase[] = [ - // chained members - { - code: 'foo && foo.bar', - output: 'foo?.bar', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz', - output: 'foo.bar?.baz', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo()', - output: 'foo?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar()', - output: 'foo.bar?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo?.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz', - output: 'foo?.bar?.baz.buzz', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz.buzz', - output: 'foo.bar?.baz.buzz', - canReplaceAndWithOr: true, - }, - // 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', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz && foo.bar.baz.buzz', - output: 'foo.bar?.baz?.buzz', - canReplaceAndWithOr: true, - }, - // chained members with element access - { - code: 'foo && foo[bar] && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar]?.baz?.buzz', - canReplaceAndWithOr: true, - }, - { +const RawBaseCases = (operator: '&&' | '||') => + [ + // chained members + { + id: 1, + declaration: 'declare const foo: {bar: number} | null | undefined;', + chain: `foo ${operator} foo.bar;`, + outputChain: 'foo?.bar;', + }, + { + id: 2, + declaration: + 'declare const foo: {bar: {baz: number} | null | undefined};', + chain: `foo.bar ${operator} foo.bar.baz;`, + outputChain: 'foo.bar?.baz;', + }, + { + id: 3, + declaration: 'declare const foo: (() => number) | null | undefined;', + chain: `foo ${operator} foo();`, + outputChain: 'foo?.();', + }, + { + id: 4, + declaration: + 'declare const foo: {bar: (() => number) | null | undefined};', + chain: `foo.bar ${operator} foo.bar();`, + outputChain: 'foo.bar?.();', + }, + { + id: 5, + declaration: + 'declare const foo: {bar: {baz: {buzz: number} | null | undefined} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo?.bar?.baz?.buzz;', + }, + { + id: 6, + declaration: + 'declare const foo: {bar: {baz: {buzz: number} | null | undefined} | null | undefined};', + chain: `foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo.bar?.baz?.buzz;', + }, // case with a jump (i.e. a non-nullish prop) - code: 'foo && foo[bar].baz && foo[bar].baz.buzz', - output: 'foo?.[bar].baz?.buzz', - canReplaceAndWithOr: true, - }, - // case with a property access in computed property - { - code: 'foo && foo[bar.baz] && foo[bar.baz].buzz', - output: 'foo?.[bar.baz]?.buzz', - canReplaceAndWithOr: true, - }, - // case with this keyword - { - code: 'foo[this.bar] && foo[this.bar].baz', - output: 'foo[this.bar]?.baz', - canReplaceAndWithOr: true, - }, - // chained calls - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz && foo.bar.baz.buzz && foo.bar.baz.buzz()', - output: 'foo.bar?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - // case with a jump (i.e. a non-nullish prop) - { - code: 'foo && foo.bar && foo.bar.baz.buzz()', - output: 'foo?.bar?.baz.buzz()', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar.baz.buzz()', - output: 'foo.bar?.baz.buzz()', - canReplaceAndWithOr: true, - }, - { + { + id: 7, + declaration: + 'declare const foo: {bar: {baz: {buzz: number}} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo?.bar?.baz.buzz;', + }, + { + id: 8, + declaration: + 'declare const foo: {bar: {baz: {buzz: number}} | null | undefined};', + chain: `foo.bar ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo.bar?.baz.buzz;', + }, + // case where for some reason there is a doubled up expression + { + id: 9, + declaration: + 'declare const foo: {bar: {baz: {buzz: number} | null | undefined} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo?.bar?.baz?.buzz;', + }, + { + id: 10, + declaration: + 'declare const foo: {bar: {baz: {buzz: number} | null | undefined} | null | undefined} | null | undefined;', + chain: `foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz;`, + outputChain: 'foo.bar?.baz?.buzz;', + }, + // chained members with element access + { + id: 11, + declaration: [ + 'declare const bar: string;', + 'declare const foo: {[k: string]: {baz: {buzz: number} | null | undefined} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo[bar] ${operator} foo[bar].baz ${operator} foo[bar].baz.buzz;`, + outputChain: 'foo?.[bar]?.baz?.buzz;', + }, + { + id: 12, + // case with a jump (i.e. a non-nullish prop) + declaration: [ + 'declare const bar: string;', + 'declare const foo: {[k: string]: {baz: {buzz: number} | null | undefined} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo[bar].baz ${operator} foo[bar].baz.buzz;`, + outputChain: 'foo?.[bar].baz?.buzz;', + }, + // case with a property access in computed property + { + id: 13, + declaration: [ + 'declare const bar: {baz: string};', + 'declare const foo: {[k: string]: {buzz: number} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo[bar.baz] ${operator} foo[bar.baz].buzz;`, + outputChain: 'foo?.[bar.baz]?.buzz;', + }, + // chained calls + { + id: 14, + declaration: + 'declare const foo: {bar: {baz: {buzz: () => number} | null | undefined} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz();`, + outputChain: 'foo?.bar?.baz?.buzz();', + }, + { + id: 15, + declaration: + 'declare const foo: {bar: {baz: {buzz: (() => number) | null | undefined} | null | undefined} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz ${operator} foo.bar.baz.buzz();`, + outputChain: 'foo?.bar?.baz?.buzz?.();', + }, + { + id: 16, + declaration: + 'declare const foo: {bar: {baz: {buzz: (() => number) | null | undefined} | null | undefined} | null | undefined};', + chain: `foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz.buzz ${operator} foo.bar.baz.buzz();`, + outputChain: '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()', - output: 'foo?.bar?.baz.buzz?.()', - canReplaceAndWithOr: true, - }, - { - // case with a call expr inside the chain for some inefficient reason - code: 'foo && foo.bar() && foo.bar().baz && foo.bar().baz.buzz && foo.bar().baz.buzz()', - output: 'foo?.bar()?.baz?.buzz?.()', - canReplaceAndWithOr: true, - }, - // chained calls with element access - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo.bar && foo.bar.baz && foo.bar.baz[buzz] && foo.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - canReplaceAndWithOr: true, - }, - // (partially) pre-optional chained - { - code: 'foo && foo?.bar && foo?.bar.baz && foo?.bar.baz[buzz] && foo?.bar.baz[buzz]()', - output: 'foo?.bar?.baz?.[buzz]?.()', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo?.bar.baz && foo?.bar.baz[buzz]', - output: 'foo?.bar.baz?.[buzz]', - canReplaceAndWithOr: true, - }, - { - code: 'foo && foo?.() && foo?.().bar', - output: 'foo?.()?.bar', - canReplaceAndWithOr: true, - }, - { - code: 'foo.bar && foo.bar?.() && foo.bar?.().baz', - output: 'foo.bar?.()?.baz', - canReplaceAndWithOr: true, - }, - { - code: 'foo !== null && foo.bar !== null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo != null && foo.bar != null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo != null && foo.bar !== null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, - { - code: 'foo !== null && foo.bar != null', - output: 'foo?.bar != null', - canReplaceAndWithOr: false, - }, -]; - -interface Selector { - all(): InvalidTestCase[]; - select>( - key: K, - value: BaseCase[K], - ): Selector; -} + { + id: 17, + declaration: + 'declare const foo: {bar: {baz: {buzz: () => number}} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz.buzz();`, + outputChain: 'foo?.bar?.baz.buzz();', + }, + { + id: 18, + declaration: + 'declare const foo: {bar: {baz: {buzz: () => number}} | null | undefined};', + chain: `foo.bar ${operator} foo.bar.baz.buzz();`, + outputChain: 'foo.bar?.baz.buzz();', + }, + { + id: 19, + // case with a jump (i.e. a non-nullish prop) + declaration: + 'declare const foo: {bar: {baz: {buzz: (() => number) | null | undefined}} | null | undefined} | null | undefined;', + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz.buzz ${operator} foo.bar.baz.buzz();`, + outputChain: 'foo?.bar?.baz.buzz?.();', + }, + { + id: 20, + // case with a call expr inside the chain for some inefficient reason + declaration: + 'declare const foo: {bar: () => ({baz: {buzz: (() => number) | null | undefined} | null | undefined}) | null | undefined};', + chain: `foo.bar ${operator} foo.bar() ${operator} foo.bar().baz ${operator} foo.bar().baz.buzz ${operator} foo.bar().baz.buzz();`, + outputChain: 'foo.bar?.()?.baz?.buzz?.();', + }, + // chained calls with element access + { + id: 21, + declaration: [ + 'declare const buzz: string;', + 'declare const foo: {bar: {baz: {[k: string]: () => number} | null | undefined} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz[buzz]();`, + outputChain: 'foo?.bar?.baz?.[buzz]();', + }, + { + id: 22, + declaration: [ + 'declare const buzz: string;', + 'declare const foo: {bar: {baz: {[k: string]: (() => number) | null | undefined} | null | undefined} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo.bar ${operator} foo.bar.baz ${operator} foo.bar.baz[buzz] ${operator} foo.bar.baz[buzz]();`, + outputChain: 'foo?.bar?.baz?.[buzz]?.();', + }, + // (partially) pre-optional chained + { + id: 23, + declaration: [ + 'declare const buzz: string;', + 'declare const foo: {bar: {baz: {[k: string]: (() => number) | null | undefined} | null | undefined} | null | undefined} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo?.bar ${operator} foo?.bar.baz ${operator} foo?.bar.baz[buzz] ${operator} foo?.bar.baz[buzz]();`, + outputChain: 'foo?.bar?.baz?.[buzz]?.();', + }, + { + id: 24, + declaration: [ + 'declare const buzz: string;', + 'declare const foo: {bar: {baz: {[k: string]: number} | null | undefined}} | null | undefined;', + ].join('\n'), + chain: `foo ${operator} foo?.bar.baz ${operator} foo?.bar.baz[buzz];`, + outputChain: 'foo?.bar.baz?.[buzz];', + }, + { + id: 25, + declaration: + 'declare const foo: (() => ({bar: number} | null | undefined)) | null | undefined;', + chain: `foo ${operator} foo?.() ${operator} foo?.().bar;`, + outputChain: 'foo?.()?.bar;', + }, + { + id: 26, + declaration: + 'declare const foo: {bar: () => ({baz: number} | null | undefined)};', + chain: `foo.bar ${operator} foo.bar?.() ${operator} foo.bar?.().baz;`, + outputChain: 'foo.bar?.()?.baz;', + }, + ] as const; -const selector = (cases: BaseCase[]): Selector => ({ - all: () => cases.map(mapper), - select: >( - key: K, - value: BaseCase[K], - ): Selector => { - const selectedCases = baseCases.filter(c => c[key] === value); - return selector(selectedCases); - }, -}); +export const identity: MutateFn = c => c; +export const BaseCases: BaseCaseCreator = ({ + operator, + mutateCode = identity, + mutateOutput = mutateCode, + mutateDeclaration = identity, + useSuggestionFixer = false, + skipIds = [], +}) => { + const skipIdsSet = new Set(skipIds); + const skipSpecifiedIds: ( + arg: ReturnType[number], + ) => boolean = + skipIds.length === 0 + ? (): boolean => true + : ({ id }): boolean => !skipIdsSet.has(id); -const { all, select } = selector(baseCases); + return RawBaseCases(operator) + .filter(skipSpecifiedIds) + .map( + ({ + id, + declaration: originalDeclaration, + chain, + outputChain, + }): InvalidTestCase< + PreferOptionalChainMessageIds, + [PreferOptionalChainOptions] + > => { + const declaration = mutateDeclaration(originalDeclaration); + const code = `// ${id}\n${declaration}\n${mutateCode(chain)}`; + const output = `// ${id}\n${declaration}\n${mutateOutput(outputChain)}`; -export { all, select }; + return { + code, + output: useSuggestionFixer ? null : output, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: !useSuggestionFixer + ? null + : [ + { + messageId: 'optionalChainSuggest', + output, + }, + ], + }, + ], + }; + }, + ); +}; 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 3a2c6cd8d45..852296721a7 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 @@ -1,1274 +1,2134 @@ import { noFormat, RuleTester } from '@typescript-eslint/rule-tester'; import rule from '../../../src/rules/prefer-optional-chain'; -import * as BaseCases from './base-cases'; +import { getFixturesRootDir } from '../../RuleTester'; +import { BaseCases, identity } from './base-cases'; const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', + parserOptions: { + project: './tsconfig.json', + tsconfigRootDir: getFixturesRootDir(), + }, }); -ruleTester.run('prefer-optional-chain', rule, { - valid: [ - '!a || !b;', - '!a || a.b;', - '!a && a.b;', - '!a && !a.b;', - '!a.b || a.b?.();', - '!a.b || a.b();', - '!foo() || !foo().bar;', - - 'foo || {};', - 'foo || ({} as any);', - '(foo || {})?.bar;', - '(foo || { bar: 1 }).bar;', - '(undefined && (foo || {})).bar;', - 'foo ||= bar;', - 'foo ||= bar || {};', - 'foo ||= bar?.baz;', - 'foo ||= bar?.baz || {};', - 'foo ||= bar?.baz?.buzz;', - '(foo1 ? foo2 : foo3 || {}).foo4;', - '(foo = 2 || {}).bar;', - 'func(foo || {}).bar;', - 'foo ?? {};', - '(foo ?? {})?.bar;', - 'foo ||= bar ?? {};', - 'foo && bar;', - 'foo && foo;', - 'foo || bar;', - 'foo ?? bar;', - 'foo || foo.bar;', - 'foo ?? foo.bar;', - "file !== 'index.ts' && file.endsWith('.ts');", - 'nextToken && sourceCode.isSpaceBetweenTokens(prevToken, nextToken);', - 'result && this.options.shouldPreserveNodeMaps;', - 'foo && fooBar.baz;', - '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;', - 'foo && foo[typeof bar] && foo[typeof bar].baz;', - '!foo[1 + 1] || !foo[1 + 2];', - '!foo[1 + 1] || !foo[1 + 1].foo;', - '!foo || !foo[bar as string] || !foo[bar as string].baz;', - '!foo || !foo[1 + 2] || !foo[1 + 2].baz;', - '!foo || !foo[typeof bar] || !foo[typeof bar].baz;', - // currently do not handle 'this' as the first part of a chain - 'this && this.foo;', - '!this || !this.foo;', - // intentionally do not handle mixed TSNonNullExpression in properties - '!entity.__helper!.__initialized || options.refresh;', - '!foo!.bar || !foo!.bar.baz;', - '!foo!.bar!.baz || !foo!.bar!.baz!.paz;', - '!foo.bar!.baz || !foo.bar!.baz!.paz;', - 'import.meta || true;', - 'import.meta || import.meta.foo;', - '!import.meta && false;', - '!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(), - // it should ignore whitespace in the expressions - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/\./g, '. '), - })), - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/\./g, '.\n'), - })), - // it should ignore parts of the expression that aren't part of the expression chain - ...BaseCases.all().map(c => ({ - ...c, - code: `${c.code} && bing`, - errors: [ - { - ...c.errors[0], - suggestions: [ - { - ...c.errors[0].suggestions![0], - output: `${c.errors[0].suggestions![0].output} && bing`, - }, - ], - }, - ], - })), - ...BaseCases.all().map(c => ({ - ...c, - code: `${c.code} && bing.bong`, - errors: [ - { - ...c.errors[0], - suggestions: [ - { - ...c.errors[0].suggestions![0], - output: `${c.errors[0].suggestions![0].output} && bing.bong`, - }, - ], - }, - ], - })), - // strict nullish equality checks x !== null && x.y !== null - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/&&/g, '!== null &&'), - })), - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/&&/g, '!= null &&'), - })), - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/&&/g, '!== undefined &&'), - })), - ...BaseCases.all().map(c => ({ - ...c, - code: c.code.replace(/&&/g, '!= undefined &&'), - })), - - // replace && with ||: foo && foo.bar -> !foo || !foo.bar - ...BaseCases.select('canReplaceAndWithOr', true) - .all() - .map(c => ({ - ...c, - code: c.code.replace(/(^|\s)foo/g, '$1!foo').replace(/&&/g, '||'), +describe('|| {}', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [ + 'foo || {};', + 'foo || ({} as any);', + '(foo || {})?.bar;', + '(foo || { bar: 1 }).bar;', + '(undefined && (foo || {})).bar;', + 'foo ||= bar || {};', + 'foo ||= bar?.baz || {};', + '(foo1 ? foo2 : foo3 || {}).foo4;', + '(foo = 2 || {}).bar;', + 'func(foo || {}).bar;', + 'foo ?? {};', + '(foo ?? {})?.bar;', + 'foo ||= bar ?? {};', + ], + invalid: [ + { + code: '(foo || {}).bar;', errors: [ { - ...c.errors[0], + messageId: 'preferOptionalChain', + column: 1, + endColumn: 16, suggestions: [ { - ...c.errors[0].suggestions![0], - output: `!${c.errors[0].suggestions![0].output}`, + messageId: 'optionalChainSuggest', + output: 'foo?.bar;', }, ], }, ], - })), - - // two errors - { - code: noFormat`foo && foo.bar && foo.bar.baz || baz && baz.bar && baz.bar.foo`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `foo?.bar?.baz || baz && baz.bar && baz.bar.foo`, - }, - ], - }, - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `foo && foo.bar && foo.bar.baz || baz?.bar?.foo`, - }, - ], - }, - ], - }, - // case with inconsistent checks - { - code: 'foo && foo.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar?.baz?.buzz;', - }, - ], - }, - ], - }, - { - 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 => );', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar(baz => );', - }, - ], - }, - ], - parserOptions: { - ecmaFeatures: { - jsx: true, - }, }, - }, - { - code: 'foo && foo.bar(baz => typeof baz);', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar(baz => typeof baz);', - }, - ], - }, - ], - }, - { - code: noFormat`foo && foo["some long string"] && foo["some long string"].baz`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `foo?.["some long string"]?.baz`, - }, - ], - }, - ], - }, - { - code: noFormat`foo && foo[\`some long string\`] && foo[\`some long string\`].baz`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `foo?.[\`some long string\`]?.baz`, - }, - ], - }, - ], - }, - { - code: "foo && foo['some long string'] && foo['some long string'].baz;", - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: "foo?.['some long string']?.baz;", - }, - ], - }, - ], - }, - // should preserve comments in a call expression - { - code: noFormat` -foo && foo.bar(/* comment */a, - // comment2 - b, ); - `, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: ` -foo?.bar(/* comment */a, - // comment2 - b, ); - `, - }, - ], - }, - ], - }, - // ensure binary expressions that are the last expression do not get removed - { - code: 'foo && foo.bar != null;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar != null;', - }, - ], - }, - ], - }, - { - code: 'foo && foo.bar != undefined;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar != undefined;', - }, - ], - }, - ], - }, - { - code: 'foo && foo.bar != null && baz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar != null && baz;', - }, - ], - }, - ], - }, - // case with this keyword at the start of expression - { - code: 'this.bar && this.bar.baz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'this.bar?.baz;', - }, - ], - }, - ], - }, - // other weird cases - { - code: 'foo && foo?.();', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.();', - }, - ], - }, - ], - }, - { - code: 'foo.bar && foo.bar?.();', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo.bar?.();', - }, - ], - }, - ], - }, - // using suggestion instead of autofix - { - code: 'foo && foo.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - line: 1, - column: 1, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar?.baz?.buzz;', - }, - ], - }, - ], - }, - { - code: 'foo && foo.bar(baz => );', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - line: 1, - column: 1, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar(baz => );', - }, - ], - }, - ], - parserOptions: { - ecmaFeatures: { - jsx: true, - }, + { + code: noFormat`(foo || ({})).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 18, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo?.bar;', + }, + ], + }, + ], }, - }, - { - code: '(foo || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 16, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(foo || ({})).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 18, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(await foo || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 22, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(await foo)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo1?.foo2 || {}).foo3;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 24, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo1?.foo2?.foo3;', - }, - ], - }, - ], - }, - { - code: '((() => foo())() || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 28, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(() => foo())()?.bar;', - }, - ], - }, - ], - }, - { - code: 'const foo = (bar || {}).baz;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 13, - endColumn: 28, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'const foo = bar?.baz;', - }, - ], - }, - ], - }, - { - code: '(foo.bar || {})[baz];', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 21, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo.bar?.[baz];', - }, - ], - }, - ], - }, - { - code: '((foo1 || {}).foo2 || {}).foo3;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 31, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1 || {}).foo2?.foo3;', - }, - ], - }, - { - messageId: 'optionalChainSuggest', - column: 2, - endColumn: 19, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1?.foo2 || {}).foo3;', - }, - ], - }, - ], - }, - { - code: '(foo || undefined || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo || undefined)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo() || bar || {}).baz;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 25, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo() || bar)?.baz;', - }, - ], - }, - ], - }, - { - code: '((foo1 ? foo2 : foo3) || {}).foo4;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 34, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1 ? foo2 : foo3)?.foo4;', - }, - ], - }, - ], - }, - { - code: noFormat`if (foo) { (foo || {}).bar; }`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 12, - endColumn: 27, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `if (foo) { foo?.bar; }`, - }, - ], - }, - ], - }, - { - code: noFormat`if ((foo || {}).bar) { foo.bar; }`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 5, - endColumn: 20, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `if (foo?.bar) { foo.bar; }`, - }, - ], - }, - ], - }, - { - code: noFormat`(undefined && foo || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 29, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(undefined && foo)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo ?? {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 16, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(foo ?? ({})).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 18, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(await foo ?? {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 22, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(await foo)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo1?.foo2 ?? {}).foo3;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 24, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo1?.foo2?.foo3;', - }, - ], - }, - ], - }, - { - code: '((() => foo())() ?? {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 28, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(() => foo())()?.bar;', - }, - ], - }, - ], - }, - { - code: 'const foo = (bar ?? {}).baz;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 13, - endColumn: 28, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'const foo = bar?.baz;', - }, - ], - }, - ], - }, - { - code: '(foo.bar ?? {})[baz];', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 21, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'foo.bar?.[baz];', - }, - ], - }, - ], - }, - { - code: '((foo1 ?? {}).foo2 ?? {}).foo3;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 31, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1 ?? {}).foo2?.foo3;', - }, - ], - }, - { - messageId: 'optionalChainSuggest', - column: 2, - endColumn: 19, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1?.foo2 ?? {}).foo3;', - }, - ], - }, - ], - }, - { - code: '(foo ?? undefined ?? {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo ?? undefined)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo() ?? bar ?? {}).baz;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 25, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo() ?? bar)?.baz;', - }, - ], - }, - ], - }, - { - code: '((foo1 ? foo2 : foo3) ?? {}).foo4;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 34, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo1 ? foo2 : foo3)?.foo4;', - }, - ], - }, - ], - }, - { - code: noFormat`if (foo) { (foo ?? {}).bar; }`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 12, - endColumn: 27, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `if (foo) { foo?.bar; }`, - }, - ], - }, - ], - }, - { - code: noFormat`if ((foo ?? {}).bar) { foo.bar; }`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 5, - endColumn: 20, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `if (foo?.bar) { foo.bar; }`, - }, - ], - }, - ], - }, - { - code: noFormat`(undefined && foo ?? {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 29, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(undefined && foo)?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(a > b || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 18, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(a > b)?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`(((typeof x) as string) || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 35, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: `((typeof x) as string)?.bar;`, - }, - ], - }, - ], - }, - { - code: '(void foo() || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 23, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(void foo())?.bar;', - }, - ], - }, - ], - }, - { - code: '((a ? b : c) || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 24, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(a ? b : c)?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`((a instanceof Error) || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 33, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(a instanceof Error)?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`((a << b) || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 21, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(a << b)?.bar;', - }, - ], - }, - ], - }, - { - code: noFormat`((foo ** 2) || {}).bar;`, - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 23, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo ** 2)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo ** 2 || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 21, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo ** 2)?.bar;', - }, - ], - }, - ], - }, - { - code: '(foo++ || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 18, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(foo++)?.bar;', - }, - ], - }, - ], - }, - { - code: '(+foo || {}).bar;', - errors: [ - { - messageId: 'optionalChainSuggest', - column: 1, - endColumn: 17, - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '(+foo)?.bar;', - }, - ], - }, - ], - }, - { - code: '(this || {}).foo;', - errors: [ - { - messageId: 'optionalChainSuggest', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: 'this?.foo;', - }, - ], - }, - ], - }, - // case with this keyword at the start of expression - { - code: '!this.bar || !this.bar.baz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!this.bar?.baz;', - }, - ], - }, - ], - }, - { - code: '!a.b || !a.b();', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!a.b?.();', - }, - ], - }, - ], - }, - { - code: '!foo.bar || !foo.bar.baz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!foo.bar?.baz;', - }, - ], - }, - ], - }, - { - code: '!foo[bar] || !foo[bar]?.[baz];', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!foo[bar]?.[baz];', - }, - ], - }, - ], - }, - { - code: '!foo || !foo?.bar.baz;', - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: '!foo?.bar.baz;', - }, - ], - }, - ], - }, - // two errors - { - code: noFormat`(!foo || !foo.bar || !foo.bar.baz) && (!baz || !baz.bar || !baz.bar.foo);`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: noFormat`(!foo?.bar?.baz) && (!baz || !baz.bar || !baz.bar.foo);`, - }, - ], - }, - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: noFormat`(!foo || !foo.bar || !foo.bar.baz) && (!baz?.bar?.foo);`, - }, - ], - }, - ], - }, - { - code: ` - class Foo { - constructor() { - new.target && new.target.length; + { + code: noFormat`(await foo || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 22, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(await foo)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo1?.foo2 || {}).foo3;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 24, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo1?.foo2?.foo3;', + }, + ], + }, + ], + }, + { + code: '((() => foo())() || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 28, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(() => foo())()?.bar;', + }, + ], + }, + ], + }, + { + code: 'const foo = (bar || {}).baz;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 13, + endColumn: 28, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'const foo = bar?.baz;', + }, + ], + }, + ], + }, + { + code: '(foo.bar || {})[baz];', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 21, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo.bar?.[baz];', + }, + ], + }, + ], + }, + { + code: '((foo1 || {}).foo2 || {}).foo3;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 31, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1 || {}).foo2?.foo3;', + }, + ], + }, + { + messageId: 'preferOptionalChain', + column: 2, + endColumn: 19, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1?.foo2 || {}).foo3;', + }, + ], + }, + ], + }, + { + code: '(foo || undefined || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo || undefined)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo() || bar || {}).baz;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 25, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo() || bar)?.baz;', + }, + ], + }, + ], + }, + { + code: '((foo1 ? foo2 : foo3) || {}).foo4;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 34, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1 ? foo2 : foo3)?.foo4;', + }, + ], + }, + ], + }, + { + code: ` + if (foo) { + (foo || {}).bar; } - } - `, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: ` - class Foo { - constructor() { - new.target?.length; + `, + errors: [ + { + messageId: 'preferOptionalChain', + column: 13, + endColumn: 28, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + if (foo) { + foo?.bar; } - } - `, - }, - ], - }, - ], - }, - { - code: noFormat`import.meta && import.meta?.baz;`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: noFormat`import.meta?.baz;`, - }, - ], - }, - ], - }, - { - code: noFormat`!import.meta || !import.meta?.baz;`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: noFormat`!import.meta?.baz;`, - }, - ], - }, - ], - }, - { - code: noFormat`import.meta && import.meta?.() && import.meta?.().baz;`, - output: null, - errors: [ - { - messageId: 'preferOptionalChain', - suggestions: [ - { - messageId: 'optionalChainSuggest', - output: noFormat`import.meta?.()?.baz;`, - }, - ], - }, - ], - }, - ], + `, + }, + ], + }, + ], + }, + { + code: ` + if ((foo || {}).bar) { + foo.bar; + } + `, + errors: [ + { + messageId: 'preferOptionalChain', + column: 15, + endColumn: 30, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + if (foo?.bar) { + foo.bar; + } + `, + }, + ], + }, + ], + }, + { + code: noFormat`(undefined && foo || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 29, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(undefined && foo)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo ?? {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 16, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`(foo ?? ({})).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 18, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`(await foo ?? {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 22, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(await foo)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo1?.foo2 ?? {}).foo3;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 24, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo1?.foo2?.foo3;', + }, + ], + }, + ], + }, + { + code: '((() => foo())() ?? {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 28, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(() => foo())()?.bar;', + }, + ], + }, + ], + }, + { + code: 'const foo = (bar ?? {}).baz;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 13, + endColumn: 28, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'const foo = bar?.baz;', + }, + ], + }, + ], + }, + { + code: '(foo.bar ?? {})[baz];', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 21, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'foo.bar?.[baz];', + }, + ], + }, + ], + }, + { + code: '((foo1 ?? {}).foo2 ?? {}).foo3;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 31, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1 ?? {}).foo2?.foo3;', + }, + ], + }, + { + messageId: 'preferOptionalChain', + column: 2, + endColumn: 19, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1?.foo2 ?? {}).foo3;', + }, + ], + }, + ], + }, + { + code: '(foo ?? undefined ?? {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo ?? undefined)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo() ?? bar ?? {}).baz;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 25, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo() ?? bar)?.baz;', + }, + ], + }, + ], + }, + { + code: '((foo1 ? foo2 : foo3) ?? {}).foo4;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 34, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo1 ? foo2 : foo3)?.foo4;', + }, + ], + }, + ], + }, + { + code: noFormat`if (foo) { (foo ?? {}).bar; }`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 12, + endColumn: 27, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'if (foo) { foo?.bar; }', + }, + ], + }, + ], + }, + { + code: noFormat`if ((foo ?? {}).bar) { foo.bar; }`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 5, + endColumn: 20, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'if (foo?.bar) { foo.bar; }', + }, + ], + }, + ], + }, + { + code: noFormat`(undefined && foo ?? {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 29, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(undefined && foo)?.bar;', + }, + ], + }, + ], + }, + { + code: '(a > b || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 18, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(a > b)?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`(((typeof x) as string) || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 35, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '((typeof x) as string)?.bar;', + }, + ], + }, + ], + }, + { + code: '(void foo() || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 23, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(void foo())?.bar;', + }, + ], + }, + ], + }, + { + code: '((a ? b : c) || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 24, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(a ? b : c)?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`((a instanceof Error) || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 33, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(a instanceof Error)?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`((a << b) || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 21, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(a << b)?.bar;', + }, + ], + }, + ], + }, + { + code: noFormat`((foo ** 2) || {}).bar;`, + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 23, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo ** 2)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo ** 2 || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 21, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo ** 2)?.bar;', + }, + ], + }, + ], + }, + { + code: '(foo++ || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 18, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(foo++)?.bar;', + }, + ], + }, + ], + }, + { + code: '(+foo || {}).bar;', + errors: [ + { + messageId: 'preferOptionalChain', + column: 1, + endColumn: 17, + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: '(+foo)?.bar;', + }, + ], + }, + ], + }, + { + code: '(this || {}).foo;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: 'this?.foo;', + }, + ], + }, + ], + }, + ], + }); +}); + +describe('hand-crafted cases', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [ + '!a || !b;', + '!a || a.b;', + '!a && a.b;', + '!a && !a.b;', + '!a.b || a.b?.();', + '!a.b || a.b();', + 'foo ||= bar;', + 'foo ||= bar?.baz;', + 'foo ||= bar?.baz?.buzz;', + 'foo && bar;', + 'foo && foo;', + 'foo || bar;', + 'foo ?? bar;', + 'foo || foo.bar;', + 'foo ?? foo.bar;', + "file !== 'index.ts' && file.endsWith('.ts');", + 'nextToken && sourceCode.isSpaceBetweenTokens(prevToken, nextToken);', + 'result && this.options.shouldPreserveNodeMaps;', + 'foo && fooBar.baz;', + 'match && match$1 !== undefined;', + "typeof foo === 'number' && foo.toFixed();", + "foo === 'undefined' && foo.length;", + 'foo == bar && foo.bar == null;', + 'foo === 1 && foo.toFixed();', + // call arguments are considered + 'foo.bar(a) && foo.bar(a, b).baz;', + // type parameters are considered + 'foo.bar() && foo.bar().baz;', + // array elements are considered + '[1, 2].length && [1, 2, 3].length.toFixed();', + noFormat`[1,].length && [1, 2].length.toFixed();`, + // short-circuiting chains are considered + '(foo?.a).b && foo.a.b.c;', + '(foo?.a)() && foo.a().b;', + '(foo?.a)() && foo.a()();', + // looks like a chain, but isn't actually a chain - just a pair of strict nullish checks + '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';", + // computed properties should be interrogated and correctly ignored + '!foo[1 + 1] || !foo[1 + 2];', + '!foo[1 + 1] || !foo[1 + 2].foo;', + // currently do not handle 'this' as the first part of a chain + 'this && this.foo;', + '!this || !this.foo;', + '!entity.__helper!.__initialized || options.refresh;', + 'import.meta || true;', + 'import.meta || import.meta.foo;', + '!import.meta && false;', + '!import.meta && !import.meta.foo;', + 'new.target || new.target.length;', + '!new.target || true;', + // Do not handle direct optional chaining on private properties because this TS limitation (https://github.com/microsoft/TypeScript/issues/42734) + 'foo && foo.#bar;', + '!foo || !foo.#bar;', + // weird non-constant cases are ignored + '({}) && {}.toString();', + '[] && [].length;', + '(() => {}) && (() => {}).name;', + '(function () {}) && function () {}.name;', + '(class Foo {}) && class Foo {}.constructor;', + "new Map().get('a') && new Map().get('a').what;", + { + code: '
&& (
).wtf;', + parserOptions: { ecmaFeatures: { jsx: true } }, + filename: 'react.tsx', + }, + { + code: '<> && (<>).wtf;', + parserOptions: { ecmaFeatures: { jsx: true } }, + filename: 'react.tsx', + }, + 'foo[x++] && foo[x++].bar;', + 'foo[yield x] && foo[yield x].bar;', + 'a = b && (a = b).wtf;', + // TODO - should we handle this? + '(x || y) != null && (x || y).foo;', + // TODO - should we handle this? + '(await foo) && (await foo).bar;', + { + code: ` + declare const x: string; + x && x.length; + `, + options: [ + { + requireNullish: true, + }, + ], + }, + { + code: ` + declare const x: string | number | boolean | object; + x && x.toString(); + `, + options: [ + { + requireNullish: true, + }, + ], + }, + { + code: ` + declare const x: any; + x && x.length; + `, + options: [ + { + checkAny: false, + }, + ], + }, + { + code: ` + declare const x: bigint; + x && x.length; + `, + options: [ + { + checkBigInt: false, + }, + ], + }, + { + code: ` + declare const x: boolean; + x && x.length; + `, + options: [ + { + checkBoolean: false, + }, + ], + }, + { + code: ` + declare const x: number; + x && x.length; + `, + options: [ + { + checkNumber: false, + }, + ], + }, + { + code: ` + declare const x: string; + x && x.length; + `, + options: [ + { + checkString: false, + }, + ], + }, + { + code: ` + declare const x: unknown; + x && x.length; + `, + options: [ + { + checkUnknown: false, + }, + ], + }, + '(x = {}) && (x.y = true) != null && x.y.toString();', + "('x' as `${'x'}`) && ('x' as `${'x'}`).length;", + '`x` && `x`.length;', + '`x${a}` && `x${a}`.length;', + + // falsy unions should be ignored + ` + declare const x: false | { a: string }; + x && x.a; + `, + ` + declare const x: false | { a: string }; + !x || x.a; + `, + ` + declare const x: '' | { a: string }; + x && x.a; + `, + ` + declare const x: '' | { a: string }; + !x || x.a; + `, + ` + declare const x: 0 | { a: string }; + x && x.a; + `, + ` + declare const x: 0 | { a: string }; + !x || x.a; + `, + ` + declare const x: 0n | { a: string }; + x && x.a; + `, + ` + declare const x: 0n | { a: string }; + !x || x.a; + `, + ], + invalid: [ + // two errors + { + code: noFormat`foo && foo.bar && foo.bar.baz || baz && baz.bar && baz.bar.foo`, + output: 'foo?.bar?.baz || baz?.bar?.foo', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // case with inconsistent checks should "break" the chain + { + code: 'foo && foo.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;', + output: + 'foo?.bar != null && foo.bar.baz !== undefined && foo.bar.baz.buzz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + foo.bar && + foo.bar.baz != null && + foo.bar.baz.qux !== undefined && + foo.bar.baz.qux.buzz; + `, + output: ` + foo.bar?.baz != null && + foo.bar.baz.qux !== undefined && + foo.bar.baz.qux.buzz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // ensure essential whitespace isn't removed + { + code: 'foo && foo.bar(baz => );', + output: 'foo?.bar(baz => );', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + filename: 'react.tsx', + }, + { + code: 'foo && foo.bar(baz => typeof baz);', + output: 'foo?.bar(baz => typeof baz);', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: "foo && foo['some long string'] && foo['some long string'].baz;", + output: "foo?.['some long string']?.baz;", + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo[`some long string`] && foo[`some long string`].baz;', + output: 'foo?.[`some long string`]?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo[`some ${long} string`] && foo[`some ${long} string`].baz;', + output: 'foo?.[`some ${long} string`]?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // complex computed properties should be handled correctly + { + code: 'foo && foo[bar as string] && foo[bar as string].baz;', + output: 'foo?.[bar as string]?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo[1 + 2] && foo[1 + 2].baz;', + output: 'foo?.[1 + 2]?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo[typeof bar] && foo[typeof bar].baz;', + output: 'foo?.[typeof bar]?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo.bar(a) && foo.bar(a, b).baz;', + output: 'foo?.bar(a) && foo.bar(a, b).baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo() && foo()(bar);', + output: 'foo()?.(bar);', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // type parameters are considered + { + code: 'foo && foo() && foo().bar;', + output: 'foo?.()?.bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo() && foo().bar;', + output: 'foo?.() && foo().bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // should preserve comments in a call expression + { + code: noFormat` + foo && foo.bar(/* comment */a, + // comment2 + b, ); + `, + output: ` + foo?.bar(/* comment */a, + // comment2 + b, ); + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // ensure binary expressions that are the last expression do not get removed + // these get autofixers because the trailing binary means the type doesn't matter + { + code: 'foo && foo.bar != null;', + output: 'foo?.bar != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo.bar != undefined;', + output: 'foo?.bar != undefined;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo.bar != null && baz;', + output: 'foo?.bar != null && baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // case with this keyword at the start of expression + { + code: 'this.bar && this.bar.baz;', + output: 'this.bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // other weird cases + { + code: 'foo && foo?.();', + output: 'foo?.();', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo.bar && foo.bar?.();', + output: 'foo.bar?.();', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo.bar(baz => );', + output: 'foo?.bar(baz => );', + errors: [ + { + messageId: 'preferOptionalChain', + line: 1, + column: 1, + suggestions: null, + }, + ], + parserOptions: { + ecmaFeatures: { + jsx: true, + }, + }, + filename: 'react.tsx', + }, + // case with this keyword at the start of expression + { + code: '!this.bar || !this.bar.baz;', + output: '!this.bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!a.b || !a.b();', + output: '!a.b?.();', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo.bar || !foo.bar.baz;', + output: '!foo.bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo[bar] || !foo[bar]?.[baz];', + output: '!foo[bar]?.[baz];', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo || !foo?.bar.baz;', + output: '!foo?.bar.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // two errors + { + code: '(!foo || !foo.bar || !foo.bar.baz) && (!baz || !baz.bar || !baz.bar.foo);', + output: '(!foo?.bar?.baz) && (!baz?.bar?.foo);', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + class Foo { + constructor() { + new.target && new.target.length; + } + } + `, + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + class Foo { + constructor() { + new.target?.length; + } + } + `, + }, + ], + }, + ], + }, + { + code: 'import.meta && import.meta?.baz;', + output: 'import.meta?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!import.meta || !import.meta?.baz;', + output: '!import.meta?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'import.meta && import.meta?.() && import.meta?.().baz;', + output: 'import.meta?.()?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // non-null expressions + { + code: '!foo() || !foo().bar;', + output: '!foo()?.bar;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo!.bar || !foo!.bar.baz;', + output: '!foo!.bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo!.bar!.baz || !foo!.bar!.baz!.paz;', + output: '!foo!.bar!.baz?.paz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: '!foo.bar!.baz || !foo.bar!.baz!.paz;', + output: '!foo.bar!.baz?.paz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + declare const foo: { bar: string } | null; + foo !== null && foo.bar !== null; + `, + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + declare const foo: { bar: string } | null; + foo?.bar !== null; + `, + }, + ], + }, + ], + }, + { + code: 'foo != null && foo.bar != null;', + output: 'foo?.bar != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + declare const foo: { bar: string | null } | null; + foo != null && foo.bar !== null; + `, + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + declare const foo: { bar: string | null } | null; + foo?.bar !== null; + `, + }, + ], + }, + ], + }, + { + code: ` + declare const foo: { bar: string | null } | null; + foo !== null && foo.bar != null; + `, + output: ` + declare const foo: { bar: string | null } | null; + foo?.bar != null; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // https://github.com/typescript-eslint/typescript-eslint/issues/6332 + { + code: 'unrelated != null && foo != null && foo.bar != null;', + output: 'unrelated != null && foo?.bar != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'unrelated1 != null && unrelated2 != null && foo != null && foo.bar != null;', + output: 'unrelated1 != null && unrelated2 != null && foo?.bar != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1461 + { + code: 'foo1 != null && foo1.bar != null && foo2 != null && foo2.bar != null;', + output: 'foo1?.bar != null && foo2?.bar != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo && foo.a && bar && bar.a;', + output: 'foo?.a && bar?.a;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // randomly placed optional chain tokens are ignored + { + code: 'foo.bar.baz != null && foo?.bar?.baz.bam != null;', + output: 'foo.bar.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo?.bar.baz != null && foo.bar?.baz.bam != null;', + output: 'foo?.bar.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo?.bar?.baz != null && foo.bar.baz.bam != null;', + output: 'foo?.bar?.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // randomly placed non-null assertions are retained as long as they're in an earlier operand + { + code: 'foo.bar.baz != null && foo!.bar!.baz.bam != null;', + output: 'foo.bar.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo!.bar.baz != null && foo.bar!.baz.bam != null;', + output: 'foo!.bar.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: 'foo!.bar!.baz != null && foo.bar.baz.bam != null;', + output: 'foo!.bar!.baz?.bam != null;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // mixed binary checks are followed and flagged + { + code: ` + a && + a.b != null && + a.b.c !== undefined && + a.b.c !== null && + a.b.c.d != null && + a.b.c.d.e !== null && + a.b.c.d.e !== undefined && + a.b.c.d.e.f != undefined && + typeof a.b.c.d.e.f.g !== 'undefined' && + a.b.c.d.e.f.g !== null && + a.b.c.d.e.f.g.h; + `, + output: ` + a?.b?.c?.d?.e?.f?.g?.h; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + !a || + a.b == null || + a.b.c === undefined || + a.b.c === null || + a.b.c.d == null || + a.b.c.d.e === null || + a.b.c.d.e === undefined || + a.b.c.d.e.f == undefined || + typeof a.b.c.d.e.f.g === 'undefined' || + a.b.c.d.e.f.g === null || + !a.b.c.d.e.f.g.h; + `, + output: ` + !a?.b?.c?.d?.e?.f?.g?.h; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + !a || + a.b == null || + a.b.c === null || + a.b.c === undefined || + a.b.c.d == null || + a.b.c.d.e === null || + a.b.c.d.e === undefined || + a.b.c.d.e.f == undefined || + typeof a.b.c.d.e.f.g === 'undefined' || + a.b.c.d.e.f.g === null || + !a.b.c.d.e.f.g.h; + `, + output: ` + !a?.b?.c?.d?.e?.f?.g?.h; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // yoda checks are flagged + { + code: 'undefined !== foo && null !== foo && null != foo.bar && foo.bar.baz;', + output: 'foo?.bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + null != foo && + 'undefined' !== typeof foo.bar && + null !== foo.bar && + foo.bar.baz; + `, + output: ` + foo?.bar?.baz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + null != foo && + 'undefined' !== typeof foo.bar && + null !== foo.bar && + null != foo.bar.baz; + `, + output: ` + null != foo?.bar?.baz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // We should retain the split strict equals check if it's the last operand + { + code: ` + null != foo && + 'undefined' !== typeof foo.bar && + null !== foo.bar && + null !== foo.bar.baz && + 'undefined' !== typeof foo.bar.baz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + null !== foo?.bar?.baz && + 'undefined' !== typeof foo.bar.baz; + `, + }, + ], + }, + ], + }, + { + code: ` + foo != null && + typeof foo.bar !== 'undefined' && + foo.bar !== null && + foo.bar.baz !== null && + typeof foo.bar.baz !== 'undefined'; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + foo?.bar?.baz !== null && + typeof foo.bar.baz !== 'undefined'; + `, + }, + ], + }, + ], + }, + { + code: ` + null != foo && + 'undefined' !== typeof foo.bar && + null !== foo.bar && + null !== foo.bar.baz && + undefined !== foo.bar.baz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + null !== foo?.bar?.baz && + undefined !== foo.bar.baz; + `, + }, + ], + }, + ], + }, + { + code: ` + foo != null && + typeof foo.bar !== 'undefined' && + foo.bar !== null && + foo.bar.baz !== null && + foo.bar.baz !== undefined; + `, + output: null, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + foo?.bar?.baz !== null && + foo.bar.baz !== undefined; + `, + }, + ], + }, + ], + }, + { + code: ` + null != foo && + 'undefined' !== typeof foo.bar && + null !== foo.bar && + undefined !== foo.bar.baz && + null !== foo.bar.baz; + `, + output: ` + undefined !== foo?.bar?.baz && + null !== foo.bar.baz; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + foo != null && + typeof foo.bar !== 'undefined' && + foo.bar !== null && + foo.bar.baz !== undefined && + foo.bar.baz !== null; + `, + output: ` + foo?.bar?.baz !== undefined && + foo.bar.baz !== null; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // await + { + code: '(await foo).bar && (await foo).bar.baz;', + output: '(await foo).bar?.baz;', + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + // TODO - should we handle this case and expand the range, or should we leave this as is? + { + code: ` + !a || + a.b == null || + a.b.c === undefined || + a.b.c === null || + a.b.c.d == null || + a.b.c.d.e === null || + a.b.c.d.e === undefined || + a.b.c.d.e.f == undefined || + a.b.c.d.e.f.g == null || + a.b.c.d.e.f.g.h; + `, + output: ` + a?.b?.c?.d?.e?.f?.g == null || + a.b.c.d.e.f.g.h; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + + { + code: ` + declare const foo: { bar: number } | null | undefined; + foo && foo.bar != null; + `, + output: ` + declare const foo: { bar: number } | null | undefined; + foo?.bar != null; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + declare const foo: { bar: number } | undefined; + foo && typeof foo.bar !== 'undefined'; + `, + output: ` + declare const foo: { bar: number } | undefined; + typeof foo?.bar !== 'undefined'; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + declare const foo: { bar: number } | undefined; + foo && 'undefined' !== typeof foo.bar; + `, + output: ` + declare const foo: { bar: number } | undefined; + 'undefined' !== typeof foo?.bar; + `, + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + + // allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing + { + code: ` + declare const foo: { bar: number } | null | undefined; + foo != undefined && foo.bar; + `, + output: ` + declare const foo: { bar: number } | null | undefined; + foo?.bar; + `, + options: [ + { + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing: + true, + }, + ], + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: null, + }, + ], + }, + { + code: ` + declare const foo: { bar: number } | null | undefined; + foo != undefined && foo.bar; + `, + output: null, + options: [ + { + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing: + false, + }, + ], + errors: [ + { + messageId: 'preferOptionalChain', + suggestions: [ + { + messageId: 'optionalChainSuggest', + output: ` + declare const foo: { bar: number } | null | undefined; + foo?.bar; + `, + }, + ], + }, + ], + }, + ], + }); +}); + +describe('base cases', () => { + describe('and', () => { + describe('boolean', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: [ + ...BaseCases({ + operator: '&&', + }), + // it should ignore parts of the expression that aren't part of the expression chain + ...BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/;$/, ' && bing;'), + }), + ...BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/;$/, ' && bing.bong;'), + }), + ], + }); + }); + + describe('strict nullish equality checks', () => { + describe('!== null', () => { + ruleTester.run('prefer-optional-chain', rule, { + // with the `| null | undefined` type - `!== null` doesn't cover the + // `undefined` case - so optional chaining is not a valid conversion + valid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!== null &&'), + mutateOutput: identity, + }), + // but if the type is just `| null` - then it covers the cases and is + // a valid conversion + invalid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!== null &&'), + mutateOutput: identity, + mutateDeclaration: c => c.replace(/\| undefined/g, ''), + useSuggestionFixer: true, + }), + }); + }); + + describe('!= null', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!= null &&'), + mutateOutput: identity, + useSuggestionFixer: true, + }), + }); + }); + + describe('!== undefined', () => { + ruleTester.run('prefer-optional-chain', rule, { + // with the `| null | undefined` type - `!== undefined` doesn't cover the + // `null` case - so optional chaining is not a valid conversion + valid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!== undefined &&'), + mutateOutput: identity, + }), + // but if the type is just `| undefined` - then it covers the cases and is + // a valid conversion + invalid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!== undefined &&'), + mutateOutput: identity, + mutateDeclaration: c => c.replace(/\| null/g, ''), + useSuggestionFixer: true, + }), + }); + }); + + describe('!= undefined', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/&&/g, '!= undefined &&'), + mutateOutput: identity, + useSuggestionFixer: true, + }), + }); + }); + }); + }); + + describe('or', () => { + describe('boolean', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: BaseCases({ + operator: '||', + mutateCode: c => `!${c.replace(/\|\|/g, '|| !')}`, + mutateOutput: c => `!${c}`, + }), + }); + }); + + describe('strict nullish equality checks', () => { + describe('=== null', () => { + ruleTester.run('prefer-optional-chain', rule, { + // with the `| null | undefined` type - `=== null` doesn't cover the + // `undefined` case - so optional chaining is not a valid conversion + valid: BaseCases({ + operator: '||', + mutateCode: c => c.replace(/\|\|/g, '=== null ||'), + mutateOutput: identity, + }), + // but if the type is just `| null` - then it covers the cases and is + // a valid conversion + invalid: BaseCases({ + operator: '||', + mutateCode: c => + c + .replace(/\|\|/g, '=== null ||') + // SEE TODO AT THE BOTTOM OF THE RULE + // We need to ensure the final operand is also a "valid" `||` check + .replace(/;$/, ' === null;'), + mutateOutput: c => c.replace(/;$/, ' === null;'), + mutateDeclaration: c => c.replace(/\| undefined/g, ''), + useSuggestionFixer: true, + }), + }); + }); + + describe('== null', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: BaseCases({ + operator: '||', + mutateCode: c => + c + .replace(/\|\|/g, '== null ||') + // SEE TODO AT THE BOTTOM OF THE RULE + // We need to ensure the final operand is also a "valid" `||` check + .replace(/;$/, ' == null;'), + mutateOutput: c => c.replace(/;$/, ' == null;'), + }), + }); + }); + + describe('=== undefined', () => { + ruleTester.run('prefer-optional-chain', rule, { + // with the `| null | undefined` type - `=== undefined` doesn't cover the + // `null` case - so optional chaining is not a valid conversion + valid: BaseCases({ + operator: '||', + mutateCode: c => c.replace(/\|\|/g, '=== undefined ||'), + mutateOutput: identity, + }), + // but if the type is just `| undefined` - then it covers the cases and is + // a valid conversion + invalid: BaseCases({ + operator: '||', + mutateCode: c => + c + .replace(/\|\|/g, '=== undefined ||') + // SEE TODO AT THE BOTTOM OF THE RULE + // We need to ensure the final operand is also a "valid" `||` check + .replace(/;$/, ' === undefined;'), + mutateOutput: c => c.replace(/;$/, ' === undefined;'), + mutateDeclaration: c => c.replace(/\| null/g, ''), + }), + }); + }); + + describe('== undefined', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: BaseCases({ + operator: '||', + mutateCode: c => + c + .replace(/\|\|/g, '== undefined ||') + // SEE TODO AT THE BOTTOM OF THE RULE + // We need to ensure the final operand is also a "valid" `||` check + .replace(/;$/, ' == undefined;'), + mutateOutput: c => c.replace(/;$/, ' == undefined;'), + }), + }); + }); + }); + }); + + describe('should ignore spacing sanity checks', () => { + ruleTester.run('prefer-optional-chain', rule, { + valid: [], + invalid: [ + // it should ignore whitespace in the expressions + ...BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/\./g, '. '), + // note - the rule will use raw text for computed expressions - so we + // need to ensure that the spacing for the computed member + // expressions is retained for correct fixer matching + mutateOutput: c => + c.replace(/(\[.+\])/g, m => m.replace(/\./g, '. ')), + }), + ...BaseCases({ + operator: '&&', + mutateCode: c => c.replace(/\./g, '.\n'), + mutateOutput: c => + c.replace(/(\[.+\])/g, m => m.replace(/\./g, '.\n')), + }), + ], + }); + }); }); diff --git a/packages/eslint-plugin/tests/schema-snapshots/prefer-optional-chain.shot b/packages/eslint-plugin/tests/schema-snapshots/prefer-optional-chain.shot index 4f5b7623fd8..d46855fc7ca 100644 --- a/packages/eslint-plugin/tests/schema-snapshots/prefer-optional-chain.shot +++ b/packages/eslint-plugin/tests/schema-snapshots/prefer-optional-chain.shot @@ -4,11 +4,69 @@ exports[`Rule schemas should be convertible to TS types for documentation purpos " # SCHEMA: -[] +[ + { + "additionalProperties": false, + "properties": { + "allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing": { + "description": "Allow autofixers that will change the return type of the expression. This option is considered unsafe as it may break the build.", + "type": "boolean" + }, + "checkAny": { + "description": "Check operands that are typed as \`any\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "checkBigInt": { + "description": "Check operands that are typed as \`bigint\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "checkBoolean": { + "description": "Check operands that are typed as \`boolean\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "checkNumber": { + "description": "Check operands that are typed as \`number\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "checkString": { + "description": "Check operands that are typed as \`string\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "checkUnknown": { + "description": "Check operands that are typed as \`unknown\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + }, + "requireNullish": { + "description": "Skip operands that are not typed with \`null\` and/or \`undefined\` when inspecting \\"loose boolean\\" operands.", + "type": "boolean" + } + }, + "type": "object" + } +] # TYPES: -/** No options declared */ -type Options = [];" +type Options = [ + { + /** Allow autofixers that will change the return type of the expression. This option is considered unsafe as it may break the build. */ + allowPotentiallyUnsafeFixesThatModifyTheReturnTypeIKnowWhatImDoing?: boolean; + /** Check operands that are typed as \`any\` when inspecting "loose boolean" operands. */ + checkAny?: boolean; + /** Check operands that are typed as \`bigint\` when inspecting "loose boolean" operands. */ + checkBigInt?: boolean; + /** Check operands that are typed as \`boolean\` when inspecting "loose boolean" operands. */ + checkBoolean?: boolean; + /** Check operands that are typed as \`number\` when inspecting "loose boolean" operands. */ + checkNumber?: boolean; + /** Check operands that are typed as \`string\` when inspecting "loose boolean" operands. */ + checkString?: boolean; + /** Check operands that are typed as \`unknown\` when inspecting "loose boolean" operands. */ + checkUnknown?: boolean; + /** Skip operands that are not typed with \`null\` and/or \`undefined\` when inspecting "loose boolean" operands. */ + requireNullish?: boolean; + }, +]; +" `; diff --git a/packages/scope-manager/src/referencer/VisitorBase.ts b/packages/scope-manager/src/referencer/VisitorBase.ts index 3432f35f5e2..e21bfc030a7 100644 --- a/packages/scope-manager/src/referencer/VisitorBase.ts +++ b/packages/scope-manager/src/referencer/VisitorBase.ts @@ -67,7 +67,7 @@ abstract class VisitorBase { * Dispatching node. */ visit(node: TSESTree.Node | null | undefined): void { - if (node == null || node.type == null) { + if (node?.type == null) { return; } diff --git a/packages/typescript-estree/src/convert.ts b/packages/typescript-estree/src/convert.ts index 734134b578b..7c1018f7a8a 100644 --- a/packages/typescript-estree/src/convert.ts +++ b/packages/typescript-estree/src/convert.ts @@ -2022,10 +2022,10 @@ export class Converter { ); return result; } else { - const type = getBinaryExpressionType(node.operatorToken); + const expressionType = getBinaryExpressionType(node.operatorToken); if ( this.allowPattern && - type === AST_NODE_TYPES.AssignmentExpression + expressionType.type === AST_NODE_TYPES.AssignmentExpression ) { return this.createNode(node, { type: AST_NODE_TYPES.AssignmentPattern, @@ -2041,12 +2041,11 @@ export class Converter { | TSESTree.BinaryExpression | TSESTree.LogicalExpression >(node, { - type, - operator: getTextForTokenKind(node.operatorToken.kind), + ...expressionType, left: this.converter( node.left, node, - type === AST_NODE_TYPES.AssignmentExpression, + expressionType.type === AST_NODE_TYPES.AssignmentExpression, ), right: this.convertChild(node.right), }); diff --git a/packages/typescript-estree/src/node-utils.ts b/packages/typescript-estree/src/node-utils.ts index 74e53c5df21..e9bb2c44523 100644 --- a/packages/typescript-estree/src/node-utils.ts +++ b/packages/typescript-estree/src/node-utils.ts @@ -10,36 +10,84 @@ const isAtLeast50 = typescriptVersionIsAtLeast['5.0']; const SyntaxKind = ts.SyntaxKind; -const LOGICAL_OPERATORS: ( - | ts.LogicalOperator - | ts.SyntaxKind.QuestionQuestionToken -)[] = [ +type LogicalOperatorKind = + | ts.SyntaxKind.AmpersandAmpersandToken + | ts.SyntaxKind.BarBarToken + | ts.SyntaxKind.QuestionQuestionToken; +const LOGICAL_OPERATORS: ReadonlySet = new Set([ SyntaxKind.BarBarToken, SyntaxKind.AmpersandAmpersandToken, SyntaxKind.QuestionQuestionToken, -]; +]); -interface TokenToText extends TSESTree.PunctuatorTokenToText { +interface TokenToText + extends TSESTree.PunctuatorTokenToText, + TSESTree.BinaryOperatorToText { [SyntaxKind.ImportKeyword]: 'import'; - [SyntaxKind.InKeyword]: 'in'; - [SyntaxKind.InstanceOfKeyword]: 'instanceof'; [SyntaxKind.NewKeyword]: 'new'; [SyntaxKind.KeyOfKeyword]: 'keyof'; [SyntaxKind.ReadonlyKeyword]: 'readonly'; [SyntaxKind.UniqueKeyword]: 'unique'; } +type AssignmentOperatorKind = keyof TSESTree.AssignmentOperatorToText; +const ASSIGNMENT_OPERATORS: ReadonlySet = new Set([ + ts.SyntaxKind.EqualsToken, + ts.SyntaxKind.PlusEqualsToken, + ts.SyntaxKind.MinusEqualsToken, + ts.SyntaxKind.AsteriskEqualsToken, + ts.SyntaxKind.AsteriskAsteriskEqualsToken, + ts.SyntaxKind.SlashEqualsToken, + ts.SyntaxKind.PercentEqualsToken, + ts.SyntaxKind.LessThanLessThanEqualsToken, + ts.SyntaxKind.GreaterThanGreaterThanEqualsToken, + ts.SyntaxKind.GreaterThanGreaterThanGreaterThanEqualsToken, + ts.SyntaxKind.AmpersandEqualsToken, + ts.SyntaxKind.BarEqualsToken, + ts.SyntaxKind.BarBarEqualsToken, + ts.SyntaxKind.AmpersandAmpersandEqualsToken, + ts.SyntaxKind.QuestionQuestionEqualsToken, + ts.SyntaxKind.CaretEqualsToken, +]); + +type BinaryOperatorKind = keyof TSESTree.BinaryOperatorToText; +const BINARY_OPERATORS: ReadonlySet = new Set([ + SyntaxKind.InstanceOfKeyword, + SyntaxKind.InKeyword, + SyntaxKind.AsteriskAsteriskToken, + SyntaxKind.AsteriskToken, + SyntaxKind.SlashToken, + SyntaxKind.PercentToken, + SyntaxKind.PlusToken, + SyntaxKind.MinusToken, + SyntaxKind.AmpersandToken, + SyntaxKind.BarToken, + SyntaxKind.CaretToken, + SyntaxKind.LessThanLessThanToken, + SyntaxKind.GreaterThanGreaterThanToken, + SyntaxKind.GreaterThanGreaterThanGreaterThanToken, + SyntaxKind.AmpersandAmpersandToken, + SyntaxKind.BarBarToken, + SyntaxKind.LessThanToken, + SyntaxKind.LessThanEqualsToken, + SyntaxKind.GreaterThanToken, + SyntaxKind.GreaterThanEqualsToken, + SyntaxKind.EqualsEqualsToken, + SyntaxKind.EqualsEqualsEqualsToken, + SyntaxKind.ExclamationEqualsEqualsToken, + SyntaxKind.ExclamationEqualsToken, +]); + /** * Returns true if the given ts.Token is the assignment operator * @param operator the operator token * @returns is assignment */ -export function isAssignmentOperator( - operator: ts.Token, -): boolean { - return ( - operator.kind >= SyntaxKind.FirstAssignment && - operator.kind <= SyntaxKind.LastAssignment +function isAssignmentOperator( + operator: ts.BinaryOperatorToken, +): operator is ts.Token { + return (ASSIGNMENT_OPERATORS as ReadonlySet).has( + operator.kind, ); } @@ -48,12 +96,21 @@ export function isAssignmentOperator( * @param operator the operator token * @returns is a logical operator */ -export function isLogicalOperator( - operator: ts.Token, -): boolean { - return (LOGICAL_OPERATORS as ts.SyntaxKind[]).includes(operator.kind); +export function isLogicalOperator( + operator: ts.BinaryOperatorToken, +): operator is ts.Token { + return (LOGICAL_OPERATORS as ReadonlySet).has(operator.kind); +} + +export function isESTreeBinaryOperator( + operator: ts.BinaryOperatorToken, +): operator is ts.Token { + return (BINARY_OPERATORS as ReadonlySet).has(operator.kind); } +type TokenForTokenKind = T extends keyof TokenToText + ? TokenToText[T] + : string | undefined; /** * Returns the string form of the given TSToken SyntaxKind * @param kind the token's SyntaxKind @@ -61,7 +118,7 @@ export function isLogicalOperator( */ export function getTextForTokenKind( kind: T, -): T extends keyof TokenToText ? TokenToText[T] : string | undefined { +): TokenForTokenKind { return ts.tokenToString(kind) as T extends keyof TokenToText ? TokenToText[T] : string | undefined; @@ -131,7 +188,7 @@ export function isComment(node: ts.Node): boolean { * @param node the TypeScript node * @returns is JSDoc comment */ -export function isJSDocComment(node: ts.Node): node is ts.JSDoc { +function isJSDocComment(node: ts.Node): node is ts.JSDoc { return node.kind === SyntaxKind.JSDocComment; } @@ -140,18 +197,39 @@ export function isJSDocComment(node: ts.Node): node is ts.JSDoc { * @param operator the operator token * @returns the binary expression type */ -export function getBinaryExpressionType( - operator: ts.Token, -): - | AST_NODE_TYPES.AssignmentExpression - | AST_NODE_TYPES.BinaryExpression - | AST_NODE_TYPES.LogicalExpression { +export function getBinaryExpressionType(operator: ts.BinaryOperatorToken): + | { + type: AST_NODE_TYPES.AssignmentExpression; + operator: TokenForTokenKind; + } + | { + type: AST_NODE_TYPES.BinaryExpression; + operator: TokenForTokenKind; + } + | { + type: AST_NODE_TYPES.LogicalExpression; + operator: TokenForTokenKind; + } { if (isAssignmentOperator(operator)) { - return AST_NODE_TYPES.AssignmentExpression; + return { + type: AST_NODE_TYPES.AssignmentExpression, + operator: getTextForTokenKind(operator.kind), + }; } else if (isLogicalOperator(operator)) { - return AST_NODE_TYPES.LogicalExpression; + return { + type: AST_NODE_TYPES.LogicalExpression, + operator: getTextForTokenKind(operator.kind), + }; + } else if (isESTreeBinaryOperator(operator)) { + return { + type: AST_NODE_TYPES.BinaryExpression, + operator: getTextForTokenKind(operator.kind), + }; } - return AST_NODE_TYPES.BinaryExpression; + + throw new Error( + `Unexpected binary operator ${ts.tokenToString(operator.kind)}`, + ); } /** @@ -233,7 +311,7 @@ export function getRange( * @param node the ts.Node * @returns is a token */ -export function isToken(node: ts.Node): node is ts.Token { +function isToken(node: ts.Node): node is ts.Token { return ( node.kind >= SyntaxKind.FirstToken && node.kind <= SyntaxKind.LastToken ); diff --git a/packages/typescript-estree/src/simple-traverse.ts b/packages/typescript-estree/src/simple-traverse.ts index 67b56d02c38..bd6c3741c2c 100644 --- a/packages/typescript-estree/src/simple-traverse.ts +++ b/packages/typescript-estree/src/simple-traverse.ts @@ -3,10 +3,13 @@ import { visitorKeys } from '@typescript-eslint/visitor-keys'; import type { TSESTree } from './ts-estree'; -// eslint-disable-next-line @typescript-eslint/no-explicit-any -function isValidNode(x: any): x is TSESTree.Node { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - return x != null && typeof x === 'object' && typeof x.type === 'string'; +function isValidNode(x: unknown): x is TSESTree.Node { + return ( + typeof x === 'object' && + x != null && + 'type' in x && + typeof x.type === 'string' + ); } function getVisitorKeysForNode( diff --git a/packages/utils/src/eslint-utils/getParserServices.ts b/packages/utils/src/eslint-utils/getParserServices.ts index d0e6a9eb394..4b65afe3cf6 100644 --- a/packages/utils/src/eslint-utils/getParserServices.ts +++ b/packages/utils/src/eslint-utils/getParserServices.ts @@ -7,21 +7,54 @@ import type { const ERROR_MESSAGE = 'You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser.'; -type GetParserServicesResult = T extends true - ? ParserServices - : ParserServicesWithTypeInformation; - /** - * Try to retrieve typescript parser service from context + * Try to retrieve type-aware parser service from context. + * This **_will_** throw if it is not available. + */ +function getParserServices< + TMessageIds extends string, + TOptions extends readonly unknown[], +>( + context: Readonly>, +): ParserServicesWithTypeInformation; +/** + * Try to retrieve type-aware parser service from context. + * This **_will_** throw if it is not available. */ function getParserServices< TMessageIds extends string, TOptions extends readonly unknown[], - TAllowWithoutFullTypeInformation extends boolean = false, >( context: Readonly>, - allowWithoutFullTypeInformation: TAllowWithoutFullTypeInformation = false as TAllowWithoutFullTypeInformation, -): GetParserServicesResult { + allowWithoutFullTypeInformation: false, +): ParserServicesWithTypeInformation; +/** + * Try to retrieve type-aware parser service from context. + * This **_will not_** throw if it is not available. + */ +function getParserServices< + TMessageIds extends string, + TOptions extends readonly unknown[], +>( + context: Readonly>, + allowWithoutFullTypeInformation: true, +): ParserServices; +/** + * Try to retrieve type-aware parser service from context. + * This may or may not throw if it is not available, depending on if `allowWithoutFullTypeInformation` is `true` + */ +function getParserServices< + TMessageIds extends string, + TOptions extends readonly unknown[], +>( + context: Readonly>, + allowWithoutFullTypeInformation: boolean, +): ParserServices; + +function getParserServices( + context: Readonly>, + allowWithoutFullTypeInformation = false, +): ParserServices { // This check is unnecessary if the user is using the latest version of our parser. // // However the world isn't perfect: @@ -33,8 +66,7 @@ function getParserServices< // This check allows us to handle bad user setups whilst providing a nice user-facing // error message explaining the problem. if ( - context.parserServices == null || - context.parserServices.esTreeNodeToTSNodeMap == null || + context.parserServices?.esTreeNodeToTSNodeMap == null || context.parserServices.tsNodeToESTreeNodeMap == null ) { throw new Error(ERROR_MESSAGE); @@ -49,7 +81,7 @@ function getParserServices< throw new Error(ERROR_MESSAGE); } - return context.parserServices as GetParserServicesResult; + return context.parserServices; } export { getParserServices }; diff --git a/packages/visitor-keys/src/visitor-keys.ts b/packages/visitor-keys/src/visitor-keys.ts index eaff7e7ccc2..0435304688f 100644 --- a/packages/visitor-keys/src/visitor-keys.ts +++ b/packages/visitor-keys/src/visitor-keys.ts @@ -109,11 +109,27 @@ type AdditionalKeys = { >]: readonly GetNodeTypeKeys[]; }; -/** - * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! IMPORTANT NOTE !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! - * - * The key arrays should be sorted in the order in which you would want to visit - * the child keys - don't just sort them alphabetically. +/* + ********************************** IMPORTANT NOTE ******************************** + * * + * The key arrays should be sorted in the order in which you would want to visit * + * the child keys. * + * * + * DO NOT SORT THEM ALPHABETICALLY! * + * * + * They should be sorted in the order that they appear in the source code. * + * For example: * + * * + * class Foo extends Bar { prop: 1 } * + * ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ClassDeclaration * + * ^^^ id ^^^ superClass * + * ^^^^^^^^^^^ body * + * * + * It would be incorrect to provide the visitor keys ['body', 'id', 'superClass'] * + * because the body comes AFTER everything else in the source code. * + * Instead the correct ordering would be ['id', 'superClass', 'body']. * + * * + ********************************************************************************** */ const SharedVisitorKeys = (() => {