diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md index 422e80fd2433..db784b951a34 100644 --- a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md +++ b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md @@ -92,7 +92,7 @@ foo ?? 'a string'; foo ?? 'a string'; foo ?? 'a string'; -const foo: ?string = 'bar'; +const foo: string | undefined = 'bar'; foo ?? 'a string'; foo ?? 'a string'; diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts index a94090787f22..4e7305d3ae6e 100644 --- a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts +++ b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts @@ -36,7 +36,7 @@ export default util.createRule({ preferNullish: 'Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.', preferNullishOverTernary: - 'Prefer using nullish coalescing operator (`??`) instead of ternary expression, as it is simpler to read.', + 'Prefer using nullish coalescing operator (`??`) instead of a ternary expression, as it is simpler to read.', suggestNullish: 'Fix to nullish coalescing operator (`??`).', }, schema: [ @@ -279,7 +279,7 @@ function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean { } /** - * This is for cases where we check both undefined and null. + * This is for ternary expressions where we check both undefined and null. * example: foo === undefined && foo === null ? 'a string' : foo * output: foo ?? 'a string' */ @@ -313,23 +313,22 @@ function isFixableExplicitTernary({ return false; } - const isIdentifier = isEqualIndentierCurry(identifier); - const hasUndefinedCheck = - (isIdentifier(left.left) && isUndefined(left.right)) || - (isIdentifier(left.right) && isUndefined(left.left)) || - (isIdentifier(right.left) && isUndefined(right.right)) || - (isIdentifier(right.right) && isUndefined(right.left)); + (util.isNodeEqual(identifier, left.left) && util.isUndefined(left.right)) || + (util.isNodeEqual(identifier, left.right) && util.isUndefined(left.left)) || + (util.isNodeEqual(identifier, right.left) && + util.isUndefined(right.right)) || + (util.isNodeEqual(identifier, right.right) && util.isUndefined(right.left)); if (!hasUndefinedCheck) { return false; } const hasNullCheck = - (isIdentifier(left.left) && isNull(left.right)) || - (isIdentifier(left.right) && isNull(left.left)) || - (isIdentifier(right.left) && isNull(right.right)) || - (isIdentifier(right.right) && isNull(right.left)); + (util.isNodeEqual(identifier, left.left) && util.isNull(left.right)) || + (util.isNodeEqual(identifier, left.right) && util.isNull(left.left)) || + (util.isNodeEqual(identifier, right.left) && util.isNull(right.right)) || + (util.isNodeEqual(identifier, right.right) && util.isNull(right.left)); if (!hasNullCheck) { return false; @@ -338,6 +337,11 @@ function isFixableExplicitTernary({ return true; } +/* + * this is for ternary expressions where == or != is used. + * example: foo == null ? 'a string' : a + * example: foo != undefined ? a : 'a string' + */ function isFixableLooseTernary({ node, identifier, @@ -359,21 +363,26 @@ function isFixableLooseTernary({ return false; } - const isIdentifier = isEqualIndentierCurry(identifier); - - if (isIdentifier(right) && (isNull(left) || isUndefined(left))) { + if ( + util.isNodeEqual(identifier, right) && + (util.isNull(left) || util.isUndefined(left)) + ) { return true; } - if (isIdentifier(left) && (isNull(right) || isUndefined(right))) { + if ( + util.isNodeEqual(identifier, left) && + (util.isNull(right) || util.isUndefined(right)) + ) { return true; } + return false; } /** - * This is for cases where we check either undefined or null and fall back to - * using type information to ensure that our checks are correct. + * This is for ternary expressions where we check only undefined or null and + * need to type information to ensure that our checks are still correct. * example: const foo:? string = 'bar'; * foo !== undefined ? foo : 'a string'; * output: foo ?? 'a string' @@ -398,9 +407,12 @@ function isFixableImplicitTernary({ if (operator !== requiredOperator) { return false; } - const isIdentifier = isEqualIndentierCurry(identifier); - const i = isIdentifier(left) ? left : isIdentifier(right) ? right : null; + const i = util.isNodeEqual(identifier, left) + ? left + : util.isNodeEqual(identifier, right) + ? right + : null; if (!i) { return false; } @@ -417,100 +429,20 @@ function isFixableImplicitTernary({ const hasUndefinedType = (flags & ts.TypeFlags.Undefined) !== 0; if ( - (hasUndefinedType && hasNullType) || - (!hasUndefinedType && !hasNullType) + hasNullType && + !hasUndefinedType && + (util.isNull(right) || util.isNull(left)) ) { - return false; - } - - if (hasNullType && (isNull(right) || isNull(left))) { return true; } - if (hasUndefinedType && (isUndefined(right) || isUndefined(left))) { - return true; - } - - return false; -} - -function isEqualIndentierCurry( - a: TSESTree.Identifier | TSESTree.MemberExpression, -) { - if (a.type === AST_NODE_TYPES.Identifier) { - return function (b: any): boolean { - if (a.type !== b.type) { - return false; - } - return !!a.name && !!b.name && a.name === b.name; - }; - } - return function (b: any): boolean { - if (a.type !== b.type) { - return false; - } - return isEqualMemberExpression(a, b); - }; -} -function isEqualMemberExpression( - a: TSESTree.MemberExpression, - b: TSESTree.MemberExpression, -): boolean { - return ( - isEqualMemberExpressionProperty(a.property, b.property) && - isEqualMemberExpressionObject(a.object, b.object) - ); -} - -function isEqualMemberExpressionProperty( - a: TSESTree.MemberExpression['property'], - b: TSESTree.MemberExpression['property'], -): boolean { - if (a.type !== b.type) { - return false; - } - if (a.type === AST_NODE_TYPES.ThisExpression) { - return true; - } if ( - a.type === AST_NODE_TYPES.Literal || - a.type === AST_NODE_TYPES.Identifier + hasUndefinedType && + !hasNullType && + (util.isUndefined(right) || util.isUndefined(left)) ) { - return ( - // @ts-ignore - (!!a.name && !!b.name && a.name === b.name) || - // @ts-ignore - (!!a.value && !!b.value && a.value === b.value) - ); - } - if (a.type === AST_NODE_TYPES.MemberExpression) { - return isEqualMemberExpression(a, b as typeof a); - } - return false; -} - -function isEqualMemberExpressionObject(a: any, b: any): boolean { - if (a.type !== b.type) { - return false; - } - if (a.type === AST_NODE_TYPES.ThisExpression) { return true; } - if (a.type === AST_NODE_TYPES.Identifier) { - return a.name === b.name; - } - if (a.type === AST_NODE_TYPES.MemberExpression) { - return isEqualMemberExpression(a, b); - } - return false; -} -function isUndefined( - i: TSESTree.Expression | TSESTree.PrivateIdentifier, -): boolean { - return i.type === AST_NODE_TYPES.Identifier && i.name === 'undefined'; -} - -function isNull(i: TSESTree.Expression | TSESTree.PrivateIdentifier): boolean { - return i.type === AST_NODE_TYPES.Literal && i.value === null; + return false; } diff --git a/packages/eslint-plugin/src/util/index.ts b/packages/eslint-plugin/src/util/index.ts index b2932466388d..8dbff44a6713 100644 --- a/packages/eslint-plugin/src/util/index.ts +++ b/packages/eslint-plugin/src/util/index.ts @@ -9,6 +9,9 @@ export * from './getThisExpression'; export * from './getWrappingFixer'; export * from './misc'; export * from './objectIterators'; +export * from './isNull'; +export * from './isUndefined'; +export * from './isNodeEqual'; // this is done for convenience - saves migrating all of the old rules export * from '@typescript-eslint/type-utils'; diff --git a/packages/eslint-plugin/src/util/isNodeEqual.ts b/packages/eslint-plugin/src/util/isNodeEqual.ts new file mode 100644 index 000000000000..2bb4b8761585 --- /dev/null +++ b/packages/eslint-plugin/src/util/isNodeEqual.ts @@ -0,0 +1,77 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; +/* + * Supported: + * Identifier Literal MemberExpression ThisExpression + * Currently not supported: + * ArrayExpression ArrayPattern ArrowFunctionExpression AssignmentExpression + * AssignmentPattern AwaitExpression BinaryExpression BlockStatement + * BreakStatement CallExpression CatchClause ChainExpression ClassBody + * ClassDeclaration ClassExpression ConditionalExpression ContinueStatement + * DebuggerStatement Decorator DoWhileStatement EmptyStatement + * ExportAllDeclaration ExportDefaultDeclaration ExportNamedDeclaration + * ExportSpecifier ExpressionStatement ForInStatement ForOfStatement + * ForStatement FunctionDeclaration FunctionExpression IfStatement + * ImportAttribute ImportDeclaration ImportDefaultSpecifier ImportExpression + * ImportNamespaceSpecifier ImportSpecifier JSXAttribute JSXClosingElement + * JSXClosingFragment JSXElement JSXEmptyExpression JSXExpressionContainer + * JSXFragment JSXIdentifier JSXMemberExpression JSXNamespacedName + * JSXOpeningElement JSXOpeningFragment JSXSpreadAttribute JSXSpreadChild + * JSXText LabeledStatement LogicalExpression MetaProperty MethodDefinition + * NewExpression ObjectExpression ObjectPattern PrivateIdentifier Program + * Property PropertyDefinition RestElement ReturnStatement SequenceExpression + * SpreadElement StaticBlock Super SwitchCase SwitchStatement + * TaggedTemplateExpression TemplateElement TemplateLiteral ThrowStatement + * TryStatement TSAbstractKeyword TSAbstractMethodDefinition + * TSAbstractPropertyDefinition TSAnyKeyword TSArrayType TSAsExpression + * TSAsyncKeyword TSBigIntKeyword TSBooleanKeyword TSCallSignatureDeclaration + * TSClassImplements TSConditionalType TSConstructorType + * TSConstructSignatureDeclaration TSDeclareFunction TSDeclareKeyword + * TSEmptyBodyFunctionExpression TSEnumDeclaration TSEnumMember + * TSExportAssignment TSExportKeyword TSExternalModuleReference + * TSFunctionType TSImportEqualsDeclaration TSImportType TSIndexedAccessType + * TSIndexSignature TSInferType TSInterfaceBody TSInterfaceDeclaration + * TSInterfaceHeritage TSIntersectionType TSIntrinsicKeyword TSLiteralType + * TSMappedType TSMethodSignature TSModuleBlock TSModuleDeclaration + * TSNamedTupleMember TSNamespaceExportDeclaration TSNeverKeyword + * TSNonNullExpression TSNullKeyword TSNumberKeyword TSObjectKeyword + * TSOptionalType TSParameterProperty TSPrivateKeyword TSPropertySignature + * TSProtectedKeyword TSPublicKeyword TSQualifiedName TSReadonlyKeyword + * TSRestType TSStaticKeyword TSStringKeyword TSSymbolKeyword + * TSTemplateLiteralType TSThisType TSTupleType TSTypeAliasDeclaration + * TSTypeAnnotation TSTypeAssertion TSTypeLiteral TSTypeOperator + * TSTypeParameter TSTypeParameterDeclaration TSTypeParameterInstantiation + * TSTypePredicate TSTypeQuery TSTypeReference TSUndefinedKeyword TSUnionType + * TSUnknownKeyword TSVoidKeyword UnaryExpression UpdateExpression + * VariableDeclaration VariableDeclarator WhileStatement WithStatement + * YieldExpression + */ + +export function isNodeEqual(a: TSESTree.Node, b: TSESTree.Node): boolean { + if (a.type !== b.type) { + return false; + } + if ( + a.type === AST_NODE_TYPES.ThisExpression && + b.type === AST_NODE_TYPES.ThisExpression + ) { + return true; + } + if (a.type === AST_NODE_TYPES.Literal && b.type === AST_NODE_TYPES.Literal) { + return a.value === b.value; + } + if ( + a.type === AST_NODE_TYPES.Identifier && + b.type === AST_NODE_TYPES.Identifier + ) { + return a.name === b.name; + } + if ( + a.type === AST_NODE_TYPES.MemberExpression && + b.type === AST_NODE_TYPES.MemberExpression + ) { + return ( + isNodeEqual(a.property, b.property) && isNodeEqual(a.object, b.object) + ); + } + return false; +} diff --git a/packages/eslint-plugin/src/util/isNull.ts b/packages/eslint-plugin/src/util/isNull.ts new file mode 100644 index 000000000000..a42757a025ab --- /dev/null +++ b/packages/eslint-plugin/src/util/isNull.ts @@ -0,0 +1,5 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; + +export function isNull(i: TSESTree.Node): boolean { + return i.type === AST_NODE_TYPES.Literal && i.value === null; +} diff --git a/packages/eslint-plugin/src/util/isUndefined.ts b/packages/eslint-plugin/src/util/isUndefined.ts new file mode 100644 index 000000000000..220dedd0dd61 --- /dev/null +++ b/packages/eslint-plugin/src/util/isUndefined.ts @@ -0,0 +1,5 @@ +import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils'; + +export function isUndefined(i: TSESTree.Node): boolean { + return i.type === AST_NODE_TYPES.Identifier && i.name === 'undefined'; +} diff --git a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts index 9ad3be9a3ce9..e13103b895b7 100644 --- a/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts +++ b/packages/eslint-plugin/tests/rules/prefer-nullish-coalescing.test.ts @@ -96,6 +96,14 @@ x ?? 'foo'; 'null != x ? y : x;', 'undefined != x ? y : x;', ` +declare const x: string; +x === null ? x : y; + `, + ` +declare const x: string | undefined; +x === null ? x : y; + `, + ` declare const x: string | null; x === undefined ? x : y; `, diff --git a/packages/eslint-plugin/tests/util/isNodeEqual.test.ts b/packages/eslint-plugin/tests/util/isNodeEqual.test.ts new file mode 100644 index 000000000000..76da9eabdb42 --- /dev/null +++ b/packages/eslint-plugin/tests/util/isNodeEqual.test.ts @@ -0,0 +1,106 @@ +import { TSESTree, TSESLint } from '@typescript-eslint/utils'; +import { getFixturesRootDir, RuleTester } from '../RuleTester'; +import { createRule, isNodeEqual } from '../../src/util'; + +const rule = createRule({ + name: 'no-useless-expression', + defaultOptions: [], + meta: { + type: 'suggestion', + fixable: 'code', + docs: { + description: 'Remove useless expressions.', + recommended: false, + }, + messages: { + removeExpression: 'Remove useless expression', + }, + schema: [], + }, + + create(context) { + const sourceCode = context.getSourceCode(); + + return { + LogicalExpression: (node: TSESTree.LogicalExpression): void => { + if ( + (node.operator === '??' || + node.operator === '||' || + node.operator === '&&') && + isNodeEqual(node.left, node.right) + ) { + context.report({ + node, + messageId: 'removeExpression', + fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { + return fixer.replaceText( + node, + sourceCode.text.slice(node.left.range[0], node.left.range[1]), + ); + }, + }); + } + }, + }; + }, +}); + +const rootPath = getFixturesRootDir(); +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +ruleTester.run('isNodeEqual', rule, { + valid: [ + { code: 'a || b' }, + { code: '!true || true' }, + { code: 'a() || a()' }, + { code: 'foo.bar || foo.bar.foo' }, + ], + invalid: [ + { + code: 'undefined || undefined', + errors: [{ messageId: 'removeExpression' }], + output: 'undefined', + }, + { + code: 'true && true', + errors: [{ messageId: 'removeExpression' }], + output: 'true', + }, + { + code: 'a || a', + errors: [{ messageId: 'removeExpression' }], + output: 'a', + }, + { + code: 'a && a', + errors: [{ messageId: 'removeExpression' }], + output: 'a', + }, + { + code: 'a ?? a', + errors: [{ messageId: 'removeExpression' }], + output: 'a', + }, + { + code: 'foo.bar || foo.bar', + errors: [{ messageId: 'removeExpression' }], + output: 'foo.bar', + }, + { + code: 'this.foo.bar || this.foo.bar', + errors: [{ messageId: 'removeExpression' }], + output: 'this.foo.bar', + }, + { + code: 'x.z[1][this[this.o]]["3"][a.b.c] || x.z[1][this[this.o]]["3"][a.b.c]', + errors: [{ messageId: 'removeExpression' }], + output: 'x.z[1][this[this.o]]["3"][a.b.c]', + }, + ], +});