From 17b7fb04b82dd87d331f6f852479b27f7ccb2859 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Dec 2021 02:29:16 -0500 Subject: [PATCH 01/10] feat(eslint-plugin): add `no-redundant-type-constituents` rule --- .cspell.json | 5 +- .../rules/no-redundant-type-constituents.ts | 323 ++++++++++++ packages/eslint-plugin/src/util/misc.ts | 21 + .../no-redundant-type-constituents.test.ts | 461 ++++++++++++++++++ packages/type-utils/src/predicates.ts | 19 + 5 files changed, 827 insertions(+), 2 deletions(-) create mode 100644 packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts create mode 100644 packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts diff --git a/.cspell.json b/.cspell.json index 898e1093251..4a8c0ad226b 100644 --- a/.cspell.json +++ b/.cspell.json @@ -71,8 +71,8 @@ "IIFE", "IIFEs", "linebreaks", - "markdownlint", "lzstring", + "markdownlint", "necroing", "nocheck", "nullish", @@ -101,14 +101,15 @@ "transpiled", "transpiles", "transpiling", - "tsvfs", "tsconfigs", "tsutils", + "tsvfs", "typedef", "typedefs", "unfixable", "unoptimized", "unprefixed", + "upsert", "Zacher" ], "overrides": [ diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts new file mode 100644 index 00000000000..1ae9bce111e --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -0,0 +1,323 @@ +import { + TSESTree, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import * as tsutils from 'tsutils'; +import * as ts from 'typescript'; +import * as util from '../util'; + +const literalToPrimitiveTypeFlags = { + [ts.TypeFlags.BigIntLiteral]: ts.TypeFlags.BigInt, + [ts.TypeFlags.BooleanLiteral]: ts.TypeFlags.Boolean, + [ts.TypeFlags.NumberLiteral]: ts.TypeFlags.Number, + [ts.TypeFlags.StringLiteral]: ts.TypeFlags.String, + [ts.TypeFlags.TemplateLiteral]: ts.TypeFlags.String, +} as const; + +const literalTypeFlags = [ + ts.TypeFlags.BigIntLiteral, + ts.TypeFlags.BooleanLiteral, + ts.TypeFlags.NumberLiteral, + ts.TypeFlags.StringLiteral, + ts.TypeFlags.TemplateLiteral, +] as const; + +const primitiveTypeFlags = [ + ts.TypeFlags.BigInt, + ts.TypeFlags.Boolean, + ts.TypeFlags.Number, + ts.TypeFlags.String, +] as const; + +const primitiveTypeFlagNames = { + [ts.TypeFlags.BigInt]: 'bigint', + [ts.TypeFlags.Boolean]: 'boolean', + [ts.TypeFlags.Number]: 'number', + [ts.TypeFlags.String]: 'string', +} as const; + +type PrimitiveTypeFlag = typeof primitiveTypeFlags[number]; + +interface TypeNodeWithValue { + literalValue: unknown; + typeNode: TSESTree.TypeNode; +} + +function addToMapGroup( + map: Map, + key: Key, + value: Value, +): void { + const existing = map.get(key); + + if (existing) { + existing.push(value); + } else { + map.set(key, [value]); + } +} + +function describeLiteralType(type: ts.Type): unknown { + return type.isStringLiteral() + ? JSON.stringify(type.value) + : type.isLiteral() + ? type.value + : util.isTypeTemplateLiteralType(type) + ? 'template literal type' + : util.isTypeBigIntLiteralType(type) + ? `${type.value.negative ? '-' : ''}${type.value.base10Value}n` + : tsutils.isBooleanLiteralType(type, true) + ? 'true' + : tsutils.isBooleanLiteralType(type, false) + ? 'false' + : 'literal type'; +} + +function isNodeInsideReturnType(node: TSESTree.TSUnionType): boolean { + return !!( + node.parent?.type === AST_NODE_TYPES.TSTypeAnnotation && + node.parent.parent && + util.isFunctionType(node.parent.parent) + ); +} + +/** + * @remarks TypeScript stores boolean types as the union false | true, always. + */ +function unionTypePartsUnlessBoolean(type: ts.Type): ts.Type[] { + return type.isUnion() && + type.types.length === 2 && + tsutils.isBooleanLiteralType(type.types[0], false) && + tsutils.isBooleanLiteralType(type.types[1], true) + ? [type] + : tsutils.unionTypeParts(type); +} + +export default util.createRule({ + name: 'no-redundant-type-constituents', + meta: { + docs: { + description: + 'Disallow members of unions and intersections that do nothing or override type information', + recommended: 'error', + requiresTypeChecking: true, + }, + messages: { + literalOverridden: `{{literal}} is overridden by {{primitive}} in this union type.`, + primitiveOverridden: `{{primitive}} is overridden by the literal {{literal}} in this intersection type.`, + overridden: `'never' is overridden by other types in this {{container}} type.`, + overrides: `'{{typeName}}' overrides all other types in this {{container}} type.`, + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create(context) { + return { + TSIntersectionType(node): void { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const seenLiteralTypes = new Map(); + const seenPrimitiveTypes = new Map< + PrimitiveTypeFlag, + TSESTree.TypeNode[] + >(); + + function checkIntersectionBottomAndTopTypes( + nodeType: ts.Type, + typeNode: TSESTree.TypeNode, + ): boolean { + for (const [typeName, messageId, check] of [ + ['any', 'overrides', util.isTypeAnyType], + ['never', 'overrides', util.isTypeNeverType], + ['unknown', 'overridden', util.isTypeUnknownType], + ] as const) { + if (check(nodeType)) { + context.report({ + data: { + container: 'intersection', + typeName, + }, + messageId, + node: typeNode, + }); + return true; + } + } + + return false; + } + + for (const typeNode of node.types) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(typeNode); + const nodeType = checker.getTypeAtLocation(tsNode); + const typeParts = tsutils.unionTypeParts(nodeType); + + for (const typePart of typeParts) { + if (checkIntersectionBottomAndTopTypes(typePart, typeNode)) { + continue; + } + + for (const literalTypeFlag of literalTypeFlags) { + if (typePart.flags === literalTypeFlag) { + addToMapGroup( + seenLiteralTypes, + literalToPrimitiveTypeFlags[literalTypeFlag], + describeLiteralType(typePart), + ); + break; + } + } + + for (const primitiveTypeFlag of primitiveTypeFlags) { + if (typePart.flags === primitiveTypeFlag) { + addToMapGroup(seenPrimitiveTypes, primitiveTypeFlag, typeNode); + } + } + } + } + + // For each primitive type of all the seen primitive types, + // if there was a literal type seen that overrides it, + // report each of the primitive type's type nodes + for (const [primitiveTypeFlag, typeNodes] of seenPrimitiveTypes) { + const matchedLiteralTypes = seenLiteralTypes.get(primitiveTypeFlag); + if (matchedLiteralTypes) { + for (const typeNode of typeNodes) { + context.report({ + data: { + literal: matchedLiteralTypes.join(' | '), + primitive: primitiveTypeFlagNames[primitiveTypeFlag], + }, + messageId: 'primitiveOverridden', + node: typeNode, + }); + } + } + } + }, + TSUnionType(node): void { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const seenLiteralTypes = new Map< + PrimitiveTypeFlag, + TypeNodeWithValue[] + >(); + const seenPrimitiveTypes = new Set(); + + function checkUnionBottomAndTopTypes( + nodeType: ts.Type, + typeNode: TSESTree.TypeNode, + ): boolean { + for (const [typeName, check] of [ + ['any', util.isTypeAnyType], + ['unknown', util.isTypeUnknownType], + ] as const) { + if (check(nodeType)) { + context.report({ + data: { + container: 'union', + typeName, + }, + messageId: 'overrides', + node: typeNode, + }); + return true; + } + } + + if (util.isTypeNeverType(nodeType) && !isNodeInsideReturnType(node)) { + context.report({ + data: { + container: 'union', + typeName: 'never', + }, + messageId: 'overridden', + node: typeNode, + }); + return true; + } + + return false; + } + + for (const typeNode of node.types) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(typeNode); + const nodeType = checker.getTypeAtLocation(tsNode); + const typeParts = unionTypePartsUnlessBoolean(nodeType); + + for (const typePart of typeParts) { + if (checkUnionBottomAndTopTypes(typePart, typeNode)) { + continue; + } + + for (const literalTypeFlag of literalTypeFlags) { + if (typePart.flags === literalTypeFlag) { + addToMapGroup( + seenLiteralTypes, + literalToPrimitiveTypeFlags[literalTypeFlag], + { + literalValue: describeLiteralType(typePart), + typeNode, + }, + ); + break; + } + } + + for (const primitiveTypeFlag of primitiveTypeFlags) { + if (tsutils.isTypeFlagSet(nodeType, primitiveTypeFlag)) { + seenPrimitiveTypes.add(primitiveTypeFlag); + } + } + } + } + + interface TypeFlagWithText { + literalValue: unknown; + primitiveTypeFlag: PrimitiveTypeFlag; + } + + const overriddenTypeNodes = new Map< + TSESTree.TypeNode, + TypeFlagWithText[] + >(); + + // For each primitive type of all the seen literal types, + // if there was a primitive type seen that overrides it, + // upsert the literal text and primitive type under the backing type node + for (const [primitiveTypeFlag, typeNodesWithText] of seenLiteralTypes) { + if (seenPrimitiveTypes.has(primitiveTypeFlag)) { + for (const { literalValue, typeNode } of typeNodesWithText) { + addToMapGroup(overriddenTypeNodes, typeNode, { + literalValue, + primitiveTypeFlag, + }); + } + } + } + + // For each type node that had at least one overridden literal, + // group those literals by their primitive type, + // then report each primitive type with all its literals + for (const [typeNode, typeFlagsWithText] of overriddenTypeNodes) { + const grouped = util.arrayGroupByToMap( + typeFlagsWithText, + pair => pair.primitiveTypeFlag, + ); + + for (const [primitiveTypeFlag, pairs] of grouped) { + context.report({ + data: { + literal: pairs.map(pair => pair.literalValue).join(' | '), + primitive: primitiveTypeFlagNames[primitiveTypeFlag], + }, + messageId: 'literalOverridden', + node: typeNode, + }); + } + } + }, + }; + }, +}); diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index 4c6b71d2a4d..b4cec978506 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -23,6 +23,26 @@ function upperCaseFirst(str: string): string { return str[0].toUpperCase() + str.slice(1); } +function arrayGroupByToMap( + array: T[], + getKey: (item: T) => Key, +): Map { + const groups = new Map(); + + for (const item of array) { + const key = getKey(item); + const existing = groups.get(key); + + if (existing) { + existing.push(item); + } else { + groups.set(key, [item]); + } + } + + return groups; +} + /** Return true if both parameters are equal. */ type Equal = (a: T, b: T) => boolean; @@ -152,6 +172,7 @@ function formatWordList(words: string[]): string { } export { + arrayGroupByToMap, arraysAreEqual, Equal, ExcludeKeys, diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts new file mode 100644 index 00000000000..20bb68bbdd5 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -0,0 +1,461 @@ +import rule from '../../src/rules/no-redundant-type-constituents'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2021, + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-redundant-type-constituents', rule, { + valid: [ + 'type _ = any;', + 'type _ = never;', + 'type _ = () => never;', + 'type _ = () => never | string;', + 'type _ = () => string | never;', + 'type _ = { (): string | never };', + 'type _ = { new (): string | never };', + 'type _ = unknown;', + 'type _ = bigint;', + 'type _ = 1n | 2n;', + 'type _ = boolean;', + 'type _ = false | true;', + 'type _ = number;', + 'type _ = 1 | 2;', + 'type _ = 1 | false;', + 'type _ = string;', + "type _ = 'a' | 'b';", + 'type _ = bigint | null;', + 'type _ = boolean | null;', + 'type _ = number | null;', + 'type _ = string | null;', + 'type _ = bigint & null;', + 'type _ = boolean & null;', + 'type _ = number & null;', + 'type _ = string & null;', + ], + + invalid: [ + { + code: 'type _ = number | any;', + errors: [ + { + column: 19, + data: { + container: 'union', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = any | number;', + errors: [ + { + column: 10, + data: { + container: 'union', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = number | never;', + errors: [ + { + column: 19, + data: { + container: 'union', + typeName: 'never', + }, + messageId: 'overridden', + }, + ], + }, + { + code: 'type _ = never | number;', + errors: [ + { + column: 10, + data: { + container: 'union', + typeName: 'never', + }, + messageId: 'overridden', + }, + ], + }, + { + code: 'type _ = number | unknown;', + errors: [ + { + column: 19, + data: { + container: 'union', + typeName: 'unknown', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = unknown | number;', + errors: [ + { + column: 10, + data: { + container: 'union', + typeName: 'unknown', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = number | 0;', + errors: [ + { + column: 19, + data: { + literal: '0', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = number | (0 | 1);', + errors: [ + { + column: 20, + data: { + literal: '0 | 1', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (0 | 0) | number;', + errors: [ + { + column: 11, + data: { + literal: '0', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (0 | (0 | 0)) | number;', + errors: [ + { + column: 11, + data: { + literal: '0', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (0 | 1) | number;', + errors: [ + { + column: 11, + data: { + literal: '0 | 1', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (0 | (0 | 1)) | number;', + errors: [ + { + column: 11, + data: { + literal: '0 | 1', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (0 | (1 | 2)) | number;', + errors: [ + { + column: 11, + data: { + literal: '0 | 1 | 2', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: "type _ = (2 | 'other' | 3) | number;", + errors: [ + { + column: 11, + data: { + literal: '2 | 3', + primitive: 'number', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: "type _ = '' | string;", + errors: [ + { + column: 10, + data: { + literal: '""', + primitive: 'string', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = `a${number}c` | string;', + errors: [ + { + column: 10, + data: { + literal: 'template literal type', + primitive: 'string', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = `${number}` | string;', + errors: [ + { + column: 10, + data: { + literal: 'template literal type', + primitive: 'string', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = 0n | bigint;', + errors: [ + { + column: 10, + data: { + literal: '0n', + primitive: 'bigint', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = -1n | bigint;', + errors: [ + { + column: 10, + data: { + literal: '-1n', + primitive: 'bigint', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = (-1n | 1n) | bigint;', + errors: [ + { + column: 11, + data: { + literal: '1n | -1n', + primitive: 'bigint', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = false | boolean;', + errors: [ + { + column: 10, + data: { + literal: 'false', + primitive: 'boolean', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = true | boolean;', + errors: [ + { + column: 10, + data: { + literal: 'true', + primitive: 'boolean', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type _ = number & any;', + errors: [ + { + column: 19, + data: { + container: 'intersection', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = any & number;', + errors: [ + { + column: 10, + data: { + container: 'intersection', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = number & never;', + errors: [ + { + column: 19, + data: { + container: 'intersection', + typeName: 'never', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = never & number;', + errors: [ + { + column: 10, + data: { + container: 'intersection', + typeName: 'never', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type _ = number & unknown;', + errors: [ + { + column: 19, + data: { + container: 'intersection', + typeName: 'unknown', + }, + messageId: 'overridden', + }, + ], + }, + { + code: 'type _ = unknown & number;', + errors: [ + { + column: 10, + data: { + container: 'intersection', + typeName: 'unknown', + }, + messageId: 'overridden', + }, + ], + }, + { + code: 'type _ = number & 0;', + errors: [ + { + column: 10, + data: { + literal: '0', + primitive: 'number', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: "type _ = '' & string;", + errors: [ + { + column: 15, + data: { + literal: '""', + primitive: 'string', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: 'type _ = 0n & bigint;', + errors: [ + { + column: 15, + data: { + literal: '0n', + primitive: 'bigint', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: 'type _ = -1n & bigint;', + errors: [ + { + column: 16, + data: { + literal: '-1n', + primitive: 'bigint', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + ], +}); diff --git a/packages/type-utils/src/predicates.ts b/packages/type-utils/src/predicates.ts index 16afbb25ea8..dd7b35735b8 100644 --- a/packages/type-utils/src/predicates.ts +++ b/packages/type-utils/src/predicates.ts @@ -47,6 +47,13 @@ export function isTypeArrayTypeOrUnionOfArrayTypes( return true; } +/** + * @returns true if the type is `never` + */ +export function isTypeNeverType(type: ts.Type): boolean { + return isTypeFlagSet(type, ts.TypeFlags.Never); +} + /** * @returns true if the type is `unknown` */ @@ -150,3 +157,15 @@ export function typeIsOrHasBaseType( return false; } + +export function isTypeBigIntLiteralType( + type: ts.Type, +): type is ts.BigIntLiteralType { + return isTypeFlagSet(type, ts.TypeFlags.BigIntLiteral); +} + +export function isTypeTemplateLiteralType( + type: ts.Type, +): type is ts.TemplateLiteralType { + return isTypeFlagSet(type, ts.TypeFlags.TemplateLiteral); +} From 31b70d3e7e1df59c7a03bfe140296f5f8784587e Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Dec 2021 17:32:36 -0500 Subject: [PATCH 02/10] chore: docs and configs --- .../rules/no-redundant-type-constituents.md | 84 +++++++++++++++++++ packages/eslint-plugin/src/configs/all.ts | 1 + packages/eslint-plugin/src/rules/index.ts | 2 + 3 files changed, 87 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md diff --git a/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md b/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md new file mode 100644 index 00000000000..b887e5be586 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md @@ -0,0 +1,84 @@ +# Disallow members of unions and intersections that do nothing or override type information (`no-redundant-type-constituents`) + +Some types can override some other types ("constituents") in a union or intersection and/or be overridden by some other types. + +## Rule Details + +TypeScript's set theory of types includes cases where a constituent type might be useless in the parent union or intersection. + +Within `|` unions: + +- `any` and `unknown` "override" all other union members +- `never` is dropped from unions in any position except when in a return type position +- primitive types such as `string` "override" any of their literal types such as `""` + +Within `&` intersections: + +- `any` and `never` "override" all other intersection members +- `unknown` is dropped from intersections +- literal types "override" any primitive types in an intersection +- literal types such as `""` "override" any of their primitive types such as `string` + +Examples of code for this rule: + + + +### ❌ Incorrect + +```ts +type UnionAny = any | 'foo'; +type UnionUnknown = unknown | 'foo'; +type UnionNever = never | 'foo'; + +type UnionBooleanLiteral = boolean | false; +type UnionNumberLiteral = number | 1; +type UnionStringLiteral = string | 'foo'; + +type IntersectionAny = any & 'foo'; +type IntersectionUnknown = string & unknown; +type IntersectionNever = string | never; + +type IntersectionBooleanLiteral = boolean & false; +type IntersectionNumberLiteral = number & 1; +type IntersectionStringLiteral = string & 'foo'; +``` + +### ✅ Correct + +```ts +type UnionAny = any; +type UnionUnknown = unknown; +type UnionNever = never; + +type UnionBooleanLiteral = boolean; +type UnionNumberLiteral = number; +type UnionStringLiteral = string; + +type IntersectionAny = any; +type IntersectionUnknown = string; +type IntersectionNever = string; + +type IntersectionBooleanLiteral = false; +type IntersectionNumberLiteral = 1; +type IntersectionStringLiteral = 'foo'; + +type ReturnUnionNever = () => string | never; +``` + +## Limitations + +This rule plays it safe and only works with bottom types, top types, and comparing literal types to primitive types. +It also does not provide an auto-fixer just yet. + +## Further Reading + +- [Union Types](https://www.typescriptlang.org/docs/handbook/2/everyday-types.html#union-types) +- [Intersection Types](https://www.typescriptlang.org/docs/handbook/2/objects.html#intersection-types) +- [Bottom Types](https://en.wikipedia.org/wiki/Bottom_type) +- [Top Types](https://en.wikipedia.org/wiki/Top_type) + +## Attributes + +- [ ] ✅ Recommended +- [ ] 🔧 Fixable +- [x] 💭 Requires type information diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 538be53ce58..c052ce4eb37 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -88,6 +88,7 @@ export = { '@typescript-eslint/no-parameter-properties': 'error', 'no-redeclare': 'off', '@typescript-eslint/no-redeclare': 'error', + '@typescript-eslint/no-redundant-type-constituents': 'error', '@typescript-eslint/no-require-imports': 'error', 'no-restricted-imports': 'off', '@typescript-eslint/no-restricted-imports': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 83e76f0a23d..96251c3c22f 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -60,6 +60,7 @@ import noNonNullAssertedOptionalChain from './no-non-null-asserted-optional-chai import noNonNullAssertion from './no-non-null-assertion'; import noParameterProperties from './no-parameter-properties'; import noRedeclare from './no-redeclare'; +import noRedundantTypeConstituents from './no-redundant-type-constituents'; import noRequireImports from './no-require-imports'; import noRestrictedImports from './no-restricted-imports'; import noShadow from './no-shadow'; @@ -183,6 +184,7 @@ export default { 'no-non-null-assertion': noNonNullAssertion, 'no-parameter-properties': noParameterProperties, 'no-redeclare': noRedeclare, + 'no-redundant-type-constituents': noRedundantTypeConstituents, 'no-require-imports': noRequireImports, 'no-restricted-imports': noRestrictedImports, 'no-shadow': noShadow, From 0916559f9df3c7adadd388c924b1bf1423f2dbc6 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Dec 2021 17:45:18 -0500 Subject: [PATCH 03/10] chore: no, not recommended just yet --- .../eslint-plugin/src/rules/no-redundant-type-constituents.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 1ae9bce111e..096172c201a 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -99,7 +99,7 @@ export default util.createRule({ docs: { description: 'Disallow members of unions and intersections that do nothing or override type information', - recommended: 'error', + recommended: false, requiresTypeChecking: true, }, messages: { From 59c2a2149447f10dfdbaeab4a95e224200a51e9c Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Fri, 31 Dec 2021 17:46:42 -0500 Subject: [PATCH 04/10] chore: no, not recommended; also fix docs --- packages/eslint-plugin/README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index 47012b81b8f..f58f2363258 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -136,6 +136,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-non-null-asserted-optional-chain`](./docs/rules/no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | :white_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :white_check_mark: | | | | [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | +| [`@typescript-eslint/no-no-redundant-type-constituents.md`](./docs/rules/no-redundant-type-constituents.md) | Disallow members of unions and intersections that do nothing or override type information | | | :thought_balloon: | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :white_check_mark: | | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | From 7ba346715e51b87f83ecf37a8126ebeaa01aa47b Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 2 Jan 2022 14:47:08 -0500 Subject: [PATCH 05/10] chore: lazy type checking --- packages/eslint-plugin/README.md | 2 +- .../rules/no-redundant-type-constituents.ts | 212 ++++++++++--- .../no-redundant-type-constituents.test.ts | 285 ++++++++++++++---- 3 files changed, 389 insertions(+), 110 deletions(-) diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index f58f2363258..d10d9f07990 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -136,7 +136,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/no-non-null-asserted-optional-chain`](./docs/rules/no-non-null-asserted-optional-chain.md) | Disallows using a non-null assertion after an optional chain expression | :white_check_mark: | | | | [`@typescript-eslint/no-non-null-assertion`](./docs/rules/no-non-null-assertion.md) | Disallows non-null assertions using the `!` postfix operator | :white_check_mark: | | | | [`@typescript-eslint/no-parameter-properties`](./docs/rules/no-parameter-properties.md) | Disallow the use of parameter properties in class constructors | | | | -| [`@typescript-eslint/no-no-redundant-type-constituents.md`](./docs/rules/no-redundant-type-constituents.md) | Disallow members of unions and intersections that do nothing or override type information | | | :thought_balloon: | +| [`@typescript-eslint/no-redundant-type-constituents`](./docs/rules/no-redundant-type-constituents.md) | Disallow members of unions and intersections that do nothing or override type information | | | :thought_balloon: | | [`@typescript-eslint/no-require-imports`](./docs/rules/no-require-imports.md) | Disallows invocation of `require()` | | | | | [`@typescript-eslint/no-this-alias`](./docs/rules/no-this-alias.md) | Disallow aliasing `this` | :white_check_mark: | | | | [`@typescript-eslint/no-type-alias`](./docs/rules/no-type-alias.md) | Disallow the use of type aliases | | | | diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 096172c201a..b8298e4d68e 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -36,8 +36,28 @@ const primitiveTypeFlagNames = { [ts.TypeFlags.String]: 'string', } as const; +const primitiveTypeFlagTypes = { + bigint: ts.TypeFlags.BigIntLiteral, + boolean: ts.TypeFlags.BooleanLiteral, + number: ts.TypeFlags.NumberLiteral, + string: ts.TypeFlags.StringLiteral, +} as const; + +const keywordNodeTypesToTsTypes = new Map([ + [TSESTree.AST_NODE_TYPES.TSAnyKeyword, ts.TypeFlags.Any], + [TSESTree.AST_NODE_TYPES.TSBigIntKeyword, ts.TypeFlags.BigInt], + [TSESTree.AST_NODE_TYPES.TSBooleanKeyword, ts.TypeFlags.Boolean], + [TSESTree.AST_NODE_TYPES.TSNumberKeyword, ts.TypeFlags.Number], + [TSESTree.AST_NODE_TYPES.TSStringKeyword, ts.TypeFlags.String], +]); + type PrimitiveTypeFlag = typeof primitiveTypeFlags[number]; +interface TypeFlagsWithName { + typeFlags: ts.TypeFlags; + typeName: string; +} + interface TypeNodeWithValue { literalValue: unknown; typeNode: TSESTree.TypeNode; @@ -57,20 +77,79 @@ function addToMapGroup( } } -function describeLiteralType(type: ts.Type): unknown { - return type.isStringLiteral() - ? JSON.stringify(type.value) - : type.isLiteral() - ? type.value - : util.isTypeTemplateLiteralType(type) - ? 'template literal type' - : util.isTypeBigIntLiteralType(type) - ? `${type.value.negative ? '-' : ''}${type.value.base10Value}n` - : tsutils.isBooleanLiteralType(type, true) - ? 'true' - : tsutils.isBooleanLiteralType(type, false) - ? 'false' - : 'literal type'; +function describeLiteralType(type: ts.Type): string { + if (type.isLiteral()) { + return type.value.toString(); + } + + if (util.isTypeAnyType(type)) { + return 'any'; + } + + if (util.isTypeNeverType(type)) { + return 'never'; + } + + if (util.isTypeUnknownType(type)) { + return 'unknown'; + } + + if (type.isStringLiteral()) { + return JSON.stringify(type.value); + } + + if (util.isTypeTemplateLiteralType(type)) { + return 'template literal type'; + } + + if (util.isTypeBigIntLiteralType(type)) { + return `${type.value.negative ? '-' : ''}${type.value.base10Value}n`; + } + + if (tsutils.isBooleanLiteralType(type, true)) { + return 'true'; + } + + if (tsutils.isBooleanLiteralType(type, false)) { + return 'false'; + } + + return 'literal type'; +} + +function describeLiteralTypeNode(typeNode: TSESTree.TypeNode): string { + switch (typeNode.type) { + case AST_NODE_TYPES.TSAnyKeyword: + return 'any'; + case AST_NODE_TYPES.TSBooleanKeyword: + return 'boolean'; + case AST_NODE_TYPES.TSNeverKeyword: + return 'never'; + case AST_NODE_TYPES.TSNumberKeyword: + return 'number'; + case AST_NODE_TYPES.TSStringKeyword: + return 'string'; + case AST_NODE_TYPES.TSUnknownKeyword: + return 'unknown'; + case AST_NODE_TYPES.TSLiteralType: + switch (typeNode.literal.type) { + case TSESTree.AST_NODE_TYPES.Literal: + switch (typeof typeNode.literal.value) { + case 'bigint': + return `${typeNode.literal.value < 0 ? '-' : ''}${ + typeNode.literal.value + }n`; + case 'string': + return JSON.stringify(typeNode.literal.value); + default: + return `${typeNode.literal.value}`; + } + case TSESTree.AST_NODE_TYPES.TemplateLiteral: + return 'template literal type'; + } + } + + return 'literal type'; } function isNodeInsideReturnType(node: TSESTree.TSUnionType): boolean { @@ -104,8 +183,8 @@ export default util.createRule({ }, messages: { literalOverridden: `{{literal}} is overridden by {{primitive}} in this union type.`, - primitiveOverridden: `{{primitive}} is overridden by the literal {{literal}} in this intersection type.`, - overridden: `'never' is overridden by other types in this {{container}} type.`, + primitiveOverridden: `{{primitive}} is overridden by the {{literal}} in this intersection type.`, + overridden: `'{{typeName}}' is overridden by other types in this {{container}} type.`, overrides: `'{{typeName}}' overrides all other types in this {{container}} type.`, }, schema: [], @@ -113,10 +192,54 @@ export default util.createRule({ }, defaultOptions: [], create(context) { + const parserServices = util.getParserServices(context); + + function getTypeNodeTypePartFlags( + typeNode: TSESTree.TypeNode, + ): TypeFlagsWithName[] { + const keywordTypeFlags = keywordNodeTypesToTsTypes.get(typeNode.type); + if (keywordTypeFlags) { + return [ + { + typeFlags: keywordTypeFlags, + typeName: describeLiteralTypeNode(typeNode), + }, + ]; + } + + if ( + typeNode.type === AST_NODE_TYPES.TSLiteralType && + typeNode.literal.type === AST_NODE_TYPES.Literal + ) { + return [ + { + typeFlags: + primitiveTypeFlagTypes[ + typeof typeNode.literal + .value as keyof typeof primitiveTypeFlagTypes + ], + typeName: describeLiteralTypeNode(typeNode), + }, + ]; + } + + if (typeNode.type === AST_NODE_TYPES.TSUnionType) { + return typeNode.types.flatMap(getTypeNodeTypePartFlags); + } + + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(typeNode); + const checker = parserServices.program.getTypeChecker(); + const nodeType = checker.getTypeAtLocation(tsNode); + const typeParts = unionTypePartsUnlessBoolean(nodeType); + + return typeParts.map(typePart => ({ + typeFlags: typePart.flags, + typeName: describeLiteralType(typePart), + })); + } + return { TSIntersectionType(node): void { - const parserServices = util.getParserServices(context); - const checker = parserServices.program.getTypeChecker(); const seenLiteralTypes = new Map(); const seenPrimitiveTypes = new Map< PrimitiveTypeFlag, @@ -124,15 +247,15 @@ export default util.createRule({ >(); function checkIntersectionBottomAndTopTypes( - nodeType: ts.Type, + { typeFlags, typeName }: TypeFlagsWithName, typeNode: TSESTree.TypeNode, ): boolean { - for (const [typeName, messageId, check] of [ - ['any', 'overrides', util.isTypeAnyType], - ['never', 'overrides', util.isTypeNeverType], - ['unknown', 'overridden', util.isTypeUnknownType], + for (const [messageId, checkFlag] of [ + ['overrides', ts.TypeFlags.Any], + ['overrides', ts.TypeFlags.Never], + ['overridden', ts.TypeFlags.Unknown], ] as const) { - if (check(nodeType)) { + if (typeFlags === checkFlag) { context.report({ data: { container: 'intersection', @@ -149,28 +272,26 @@ export default util.createRule({ } for (const typeNode of node.types) { - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(typeNode); - const nodeType = checker.getTypeAtLocation(tsNode); - const typeParts = tsutils.unionTypeParts(nodeType); + const typePartFlags = getTypeNodeTypePartFlags(typeNode); - for (const typePart of typeParts) { + for (const typePart of typePartFlags) { if (checkIntersectionBottomAndTopTypes(typePart, typeNode)) { continue; } for (const literalTypeFlag of literalTypeFlags) { - if (typePart.flags === literalTypeFlag) { + if (typePart.typeFlags === literalTypeFlag) { addToMapGroup( seenLiteralTypes, literalToPrimitiveTypeFlags[literalTypeFlag], - describeLiteralType(typePart), + typePart.typeName, ); break; } } for (const primitiveTypeFlag of primitiveTypeFlags) { - if (typePart.flags === primitiveTypeFlag) { + if (typePart.typeFlags === primitiveTypeFlag) { addToMapGroup(seenPrimitiveTypes, primitiveTypeFlag, typeNode); } } @@ -197,8 +318,6 @@ export default util.createRule({ } }, TSUnionType(node): void { - const parserServices = util.getParserServices(context); - const checker = parserServices.program.getTypeChecker(); const seenLiteralTypes = new Map< PrimitiveTypeFlag, TypeNodeWithValue[] @@ -206,14 +325,14 @@ export default util.createRule({ const seenPrimitiveTypes = new Set(); function checkUnionBottomAndTopTypes( - nodeType: ts.Type, + { typeFlags, typeName }: TypeFlagsWithName, typeNode: TSESTree.TypeNode, ): boolean { - for (const [typeName, check] of [ - ['any', util.isTypeAnyType], - ['unknown', util.isTypeUnknownType], + for (const checkFlag of [ + ts.TypeFlags.Any, + ts.TypeFlags.Unknown, ] as const) { - if (check(nodeType)) { + if (typeFlags === checkFlag) { context.report({ data: { container: 'union', @@ -226,7 +345,10 @@ export default util.createRule({ } } - if (util.isTypeNeverType(nodeType) && !isNodeInsideReturnType(node)) { + if ( + typeFlags === ts.TypeFlags.Never && + !isNodeInsideReturnType(node) + ) { context.report({ data: { container: 'union', @@ -242,22 +364,20 @@ export default util.createRule({ } for (const typeNode of node.types) { - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(typeNode); - const nodeType = checker.getTypeAtLocation(tsNode); - const typeParts = unionTypePartsUnlessBoolean(nodeType); + const typePartFlags = getTypeNodeTypePartFlags(typeNode); - for (const typePart of typeParts) { + for (const typePart of typePartFlags) { if (checkUnionBottomAndTopTypes(typePart, typeNode)) { continue; } for (const literalTypeFlag of literalTypeFlags) { - if (typePart.flags === literalTypeFlag) { + if (typePart.typeFlags === literalTypeFlag) { addToMapGroup( seenLiteralTypes, literalToPrimitiveTypeFlags[literalTypeFlag], { - literalValue: describeLiteralType(typePart), + literalValue: typePart.typeName, typeNode, }, ); @@ -266,7 +386,7 @@ export default util.createRule({ } for (const primitiveTypeFlag of primitiveTypeFlags) { - if (tsutils.isTypeFlagSet(nodeType, primitiveTypeFlag)) { + if ((typePart.typeFlags & primitiveTypeFlag) !== 0) { seenPrimitiveTypes.add(primitiveTypeFlag); } } diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 20bb68bbdd5..0c87278a7d1 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -13,36 +13,128 @@ const ruleTester = new RuleTester({ ruleTester.run('no-redundant-type-constituents', rule, { valid: [ - 'type _ = any;', - 'type _ = never;', - 'type _ = () => never;', - 'type _ = () => never | string;', - 'type _ = () => string | never;', - 'type _ = { (): string | never };', - 'type _ = { new (): string | never };', - 'type _ = unknown;', - 'type _ = bigint;', - 'type _ = 1n | 2n;', - 'type _ = boolean;', - 'type _ = false | true;', - 'type _ = number;', - 'type _ = 1 | 2;', - 'type _ = 1 | false;', - 'type _ = string;', - "type _ = 'a' | 'b';", - 'type _ = bigint | null;', - 'type _ = boolean | null;', - 'type _ = number | null;', - 'type _ = string | null;', - 'type _ = bigint & null;', - 'type _ = boolean & null;', - 'type _ = number & null;', - 'type _ = string & null;', + 'type T = any;', + 'type T = never;', + 'type T = () => never;', + 'type T = () => never | string;', + ` + type B = never; + type T = () => B | string; + `, + ` + type B = string; + type T = () => B | never; + `, + 'type T = () => string | never;', + 'type T = { (): string | never };', + ` + type B = string; + type T = { (): B | never }; + `, + 'type T = { new (): string | never };', + ` + type B = never; + type T = { new (): string | B }; + `, + 'type T = unknown;', + ` + type B = unknown; + type T = B; + `, + 'type T = bigint;', + ` + type B = bigint; + type T = B; + `, + 'type T = 1n | 2n;', + ` + type B = 1n; + type T = B | 2n; + `, + 'type T = boolean;', + ` + type B = boolean; + type T = B; + `, + 'type T = false | true;', + ` + type B = false; + type T = B | true; + `, + 'type T = number;', + ` + type B = number; + type T = B; + `, + 'type T = 1 | 2;', + ` + type B = 1; + type T = B | 2; + `, + 'type T = 1 | false;', + ` + type B = 1; + type T = B | false; + `, + 'type T = string;', + ` + type B = string; + type T = B; + `, + "type T = 'a' | 'b';", + ` + type B = 'b'; + type T = 'a' | B; + `, + ` + type B = 'a'; + type T = B | 'b'; + `, + 'type T = bigint | null;', + ` + type B = bigint; + type T = B | null; + `, + 'type T = boolean | null;', + ` + type B = boolean; + type T = B | null; + `, + 'type T = number | null;', + ` + type B = number; + type T = B | null; + `, + 'type T = string | null;', + ` + type B = string; + type T = B | null; + `, + 'type T = bigint & null;', + ` + type B = bigint; + type T = B & null; + `, + 'type T = boolean & null;', + ` + type B = boolean; + type T = B & null; + `, + 'type T = number & null;', + ` + type B = number; + type T = B & null; + `, + 'type T = string & null;', + ` + type B = string; + type T = B & null; + `, ], invalid: [ { - code: 'type _ = number | any;', + code: 'type T = number | any;', errors: [ { column: 19, @@ -55,7 +147,23 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = any | number;', + code: ` + type B = number; + type T = B | any; + `, + errors: [ + { + column: 22, + data: { + container: 'union', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type T = any | number;', errors: [ { column: 10, @@ -68,7 +176,23 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number | never;', + code: ` + type B = any; + type T = B | number; + `, + errors: [ + { + column: 18, + data: { + container: 'union', + typeName: 'any', + }, + messageId: 'overrides', + }, + ], + }, + { + code: 'type T = number | never;', errors: [ { column: 19, @@ -81,7 +205,23 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = never | number;', + code: ` + type B = number; + type T = B | never; + `, + errors: [ + { + column: 22, + data: { + container: 'union', + typeName: 'never', + }, + messageId: 'overridden', + }, + ], + }, + { + code: 'type T = never | number;', errors: [ { column: 10, @@ -94,7 +234,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number | unknown;', + code: 'type T = number | unknown;', errors: [ { column: 19, @@ -107,7 +247,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = unknown | number;', + code: 'type T = unknown | number;', errors: [ { column: 10, @@ -120,7 +260,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number | 0;', + code: 'type T = number | 0;', errors: [ { column: 19, @@ -133,7 +273,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number | (0 | 1);', + code: 'type T = number | (0 | 1);', errors: [ { column: 20, @@ -146,12 +286,12 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (0 | 0) | number;', + code: 'type T = (0 | 0) | number;', errors: [ { column: 11, data: { - literal: '0', + literal: '0 | 0', primitive: 'number', }, messageId: 'literalOverridden', @@ -159,12 +299,15 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (0 | (0 | 0)) | number;', + code: ` + type B = 0 | 1; + type T = (2 | B) | number; + `, errors: [ { - column: 11, + column: 19, data: { - literal: '0', + literal: '2 | 0 | 1', primitive: 'number', }, messageId: 'literalOverridden', @@ -172,12 +315,12 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (0 | 1) | number;', + code: 'type T = (0 | (1 | 2)) | number;', errors: [ { column: 11, data: { - literal: '0 | 1', + literal: '0 | 1 | 2', primitive: 'number', }, messageId: 'literalOverridden', @@ -185,7 +328,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (0 | (0 | 1)) | number;', + code: 'type T = (0 | 1) | number;', errors: [ { column: 11, @@ -198,12 +341,12 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (0 | (1 | 2)) | number;', + code: 'type T = (0 | (0 | 1)) | number;', errors: [ { column: 11, data: { - literal: '0 | 1 | 2', + literal: '0 | 0 | 1', primitive: 'number', }, messageId: 'literalOverridden', @@ -211,7 +354,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: "type _ = (2 | 'other' | 3) | number;", + code: "type T = (2 | 'other' | 3) | number;", errors: [ { column: 11, @@ -224,7 +367,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: "type _ = '' | string;", + code: "type T = '' | string;", errors: [ { column: 10, @@ -237,7 +380,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = `a${number}c` | string;', + code: 'type T = `a${number}c` | string;', errors: [ { column: 10, @@ -250,7 +393,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = `${number}` | string;', + code: 'type T = `${number}` | string;', errors: [ { column: 10, @@ -263,7 +406,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = 0n | bigint;', + code: 'type T = 0n | bigint;', errors: [ { column: 10, @@ -276,7 +419,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = -1n | bigint;', + code: 'type T = -1n | bigint;', errors: [ { column: 10, @@ -289,12 +432,12 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = (-1n | 1n) | bigint;', + code: 'type T = (-1n | 1n) | bigint;', errors: [ { column: 11, data: { - literal: '1n | -1n', + literal: '-1n | 1n', primitive: 'bigint', }, messageId: 'literalOverridden', @@ -302,7 +445,23 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = false | boolean;', + code: ` + type B = boolean; + type T = B | false; + `, + errors: [ + { + column: 22, + data: { + literal: 'false', + primitive: 'boolean', + }, + messageId: 'literalOverridden', + }, + ], + }, + { + code: 'type T = false | boolean;', errors: [ { column: 10, @@ -315,7 +474,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = true | boolean;', + code: 'type T = true | boolean;', errors: [ { column: 10, @@ -328,7 +487,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number & any;', + code: 'type T = number & any;', errors: [ { column: 19, @@ -341,7 +500,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = any & number;', + code: 'type T = any & number;', errors: [ { column: 10, @@ -354,7 +513,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number & never;', + code: 'type T = number & never;', errors: [ { column: 19, @@ -367,7 +526,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = never & number;', + code: 'type T = never & number;', errors: [ { column: 10, @@ -380,7 +539,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number & unknown;', + code: 'type T = number & unknown;', errors: [ { column: 19, @@ -393,7 +552,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = unknown & number;', + code: 'type T = unknown & number;', errors: [ { column: 10, @@ -406,7 +565,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = number & 0;', + code: 'type T = number & 0;', errors: [ { column: 10, @@ -419,7 +578,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: "type _ = '' & string;", + code: "type T = '' & string;", errors: [ { column: 15, @@ -432,7 +591,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = 0n & bigint;', + code: 'type T = 0n & bigint;', errors: [ { column: 15, @@ -445,7 +604,7 @@ ruleTester.run('no-redundant-type-constituents', rule, { ], }, { - code: 'type _ = -1n & bigint;', + code: 'type T = -1n & bigint;', errors: [ { column: 16, From 22ed9d5644b02842ee15f0bde2b60d9e90c71026 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 2 Jan 2022 15:42:45 -0500 Subject: [PATCH 06/10] chore: fill in missing edge cases --- .../rules/no-redundant-type-constituents.ts | 8 +- .../no-redundant-type-constituents.test.ts | 126 ++++++++++++++++++ 2 files changed, 130 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index b8298e4d68e..634775d8fb3 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -78,6 +78,10 @@ function addToMapGroup( } function describeLiteralType(type: ts.Type): string { + if (type.isStringLiteral()) { + return JSON.stringify(type.value); + } + if (type.isLiteral()) { return type.value.toString(); } @@ -94,10 +98,6 @@ function describeLiteralType(type: ts.Type): string { return 'unknown'; } - if (type.isStringLiteral()) { - return JSON.stringify(type.value); - } - if (util.isTypeTemplateLiteralType(type)) { return 'template literal type'; } diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 0c87278a7d1..f6fb288865d 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -61,6 +61,10 @@ ruleTester.run('no-redundant-type-constituents', rule, { type B = false; type T = B | true; `, + ` + type B = true; + type T = B | false; + `, 'type T = number;', ` type B = number; @@ -379,6 +383,22 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type B = 'b'; + type T = B | string; + `, + errors: [ + { + column: 18, + data: { + literal: '"b"', + primitive: 'string', + }, + messageId: 'literalOverridden', + }, + ], + }, { code: 'type T = `a${number}c` | string;', errors: [ @@ -392,6 +412,22 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type B = \`a\${number}c\`; + type T = B | string; + `, + errors: [ + { + column: 18, + data: { + literal: 'template literal type', + primitive: 'string', + }, + messageId: 'literalOverridden', + }, + ], + }, { code: 'type T = `${number}` | string;', errors: [ @@ -486,6 +522,64 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: 'type T = false & boolean;', + errors: [ + { + column: 18, + data: { + literal: 'false', + primitive: 'boolean', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: ` + type B = false; + type T = B & boolean; + `, + errors: [ + { + column: 22, + data: { + literal: 'false', + primitive: 'boolean', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: ` + type B = true; + type T = B & boolean; + `, + errors: [ + { + column: 22, + data: { + literal: 'true', + primitive: 'boolean', + }, + messageId: 'primitiveOverridden', + }, + ], + }, + { + code: 'type T = true & boolean;', + errors: [ + { + column: 17, + data: { + literal: 'true', + primitive: 'boolean', + }, + messageId: 'primitiveOverridden', + }, + ], + }, { code: 'type T = number & any;', errors: [ @@ -525,6 +619,22 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type B = never; + type T = B & number; + `, + errors: [ + { + column: 18, + data: { + container: 'intersection', + typeName: 'never', + }, + messageId: 'overrides', + }, + ], + }, { code: 'type T = never & number;', errors: [ @@ -590,6 +700,22 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type B = 0n; + type T = B & bigint; + `, + errors: [ + { + column: 22, + data: { + literal: '0n', + primitive: 'bigint', + }, + messageId: 'primitiveOverridden', + }, + ], + }, { code: 'type T = 0n & bigint;', errors: [ From 9742e45edadd242cb1eddb8ea7803294f101afd8 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 23 Jan 2022 17:56:09 -0500 Subject: [PATCH 07/10] chore: add type cache --- .../rules/no-redundant-type-constituents.ts | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 634775d8fb3..48a7c02499f 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -1,7 +1,4 @@ -import { - TSESTree, - AST_NODE_TYPES, -} from '@typescript-eslint/experimental-utils'; +import { TSESTree, AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as tsutils from 'tsutils'; import * as ts from 'typescript'; import * as util from '../util'; @@ -193,6 +190,7 @@ export default util.createRule({ defaultOptions: [], create(context) { const parserServices = util.getParserServices(context); + const typesCache = new Map(); function getTypeNodeTypePartFlags( typeNode: TSESTree.TypeNode, @@ -238,8 +236,21 @@ export default util.createRule({ })); } + function getTypeNodeTypePartFlagsCached( + typeNode: TSESTree.TypeNode, + ): TypeFlagsWithName[] { + const existing = typesCache.get(typeNode); + if (existing) { + return existing; + } + + const created = getTypeNodeTypePartFlags(typeNode); + typesCache.set(typeNode, created); + return created; + } + return { - TSIntersectionType(node): void { + 'TSIntersectionType:exit'(node: TSESTree.TSIntersectionType): void { const seenLiteralTypes = new Map(); const seenPrimitiveTypes = new Map< PrimitiveTypeFlag, @@ -272,7 +283,7 @@ export default util.createRule({ } for (const typeNode of node.types) { - const typePartFlags = getTypeNodeTypePartFlags(typeNode); + const typePartFlags = getTypeNodeTypePartFlagsCached(typeNode); for (const typePart of typePartFlags) { if (checkIntersectionBottomAndTopTypes(typePart, typeNode)) { @@ -317,7 +328,7 @@ export default util.createRule({ } } }, - TSUnionType(node): void { + 'TSUnionType:exit'(node: TSESTree.TSUnionType): void { const seenLiteralTypes = new Map< PrimitiveTypeFlag, TypeNodeWithValue[] @@ -364,7 +375,7 @@ export default util.createRule({ } for (const typeNode of node.types) { - const typePartFlags = getTypeNodeTypePartFlags(typeNode); + const typePartFlags = getTypeNodeTypePartFlagsCached(typeNode); for (const typePart of typePartFlags) { if (checkUnionBottomAndTopTypes(typePart, typeNode)) { From 3c1c5247035112191d5a489256a3aa903c392147 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Feb 2022 17:35:20 -0500 Subject: [PATCH 08/10] chore: more test coverage; I'd missed a couple of keywords --- .../rules/no-redundant-type-constituents.ts | 2 + .../no-redundant-type-constituents.test.ts | 37 +++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 48a7c02499f..4a75cf2214e 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -44,6 +44,8 @@ const keywordNodeTypesToTsTypes = new Map([ [TSESTree.AST_NODE_TYPES.TSAnyKeyword, ts.TypeFlags.Any], [TSESTree.AST_NODE_TYPES.TSBigIntKeyword, ts.TypeFlags.BigInt], [TSESTree.AST_NODE_TYPES.TSBooleanKeyword, ts.TypeFlags.Boolean], + [TSESTree.AST_NODE_TYPES.TSNeverKeyword, ts.TypeFlags.Never], + [TSESTree.AST_NODE_TYPES.TSUnknownKeyword, ts.TypeFlags.Unknown], [TSESTree.AST_NODE_TYPES.TSNumberKeyword, ts.TypeFlags.Number], [TSESTree.AST_NODE_TYPES.TSStringKeyword, ts.TypeFlags.String], ]); diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index f6fb288865d..68baac588ff 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -13,8 +13,19 @@ const ruleTester = new RuleTester({ ruleTester.run('no-redundant-type-constituents', rule, { valid: [ - 'type T = any;', - 'type T = never;', + ` + type T = any; + type U = T; + `, + ` + type T = never; + type U = T; + `, + ` + type T = 1 | 2; + type U = T | 3; + type V = U; + `, 'type T = () => never;', 'type T = () => never | string;', ` @@ -36,7 +47,6 @@ ruleTester.run('no-redundant-type-constituents', rule, { type B = never; type T = { new (): string | B }; `, - 'type T = unknown;', ` type B = unknown; type T = B; @@ -134,6 +144,11 @@ ruleTester.run('no-redundant-type-constituents', rule, { type B = string; type T = B & null; `, + 'type T = `${string}` & null;', + ` + type B = \`\${string}\`; + type T = B & null; + `, ], invalid: [ @@ -224,6 +239,22 @@ ruleTester.run('no-redundant-type-constituents', rule, { }, ], }, + { + code: ` + type B = never; + type T = B | number; + `, + errors: [ + { + column: 18, + data: { + container: 'union', + typeName: 'never', + }, + messageId: 'overridden', + }, + ], + }, { code: 'type T = never | number;', errors: [ From aa2eb9eb728cfd14661ba1bc601c47e25a1fc747 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 2 Feb 2022 17:39:12 -0500 Subject: [PATCH 09/10] fix: functions too --- .../src/rules/no-redundant-type-constituents.ts | 3 ++- .../tests/rules/no-redundant-type-constituents.test.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts index 4a75cf2214e..1ee8c3c4959 100644 --- a/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts +++ b/packages/eslint-plugin/src/rules/no-redundant-type-constituents.ts @@ -155,7 +155,8 @@ function isNodeInsideReturnType(node: TSESTree.TSUnionType): boolean { return !!( node.parent?.type === AST_NODE_TYPES.TSTypeAnnotation && node.parent.parent && - util.isFunctionType(node.parent.parent) + (util.isFunctionType(node.parent.parent) || + util.isFunction(node.parent.parent)) ); } diff --git a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts index 68baac588ff..4994459278f 100644 --- a/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts +++ b/packages/eslint-plugin/tests/rules/no-redundant-type-constituents.test.ts @@ -38,6 +38,16 @@ ruleTester.run('no-redundant-type-constituents', rule, { `, 'type T = () => string | never;', 'type T = { (): string | never };', + ` + function _(): string | never { + return ''; + } + `, + ` + const _ = (): string | never => { + return ''; + }; + `, ` type B = string; type T = { (): B | never }; From ecd559a760a17e41ae5522f7e52439cc0a57f121 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Wed, 23 Feb 2022 15:33:56 -0500 Subject: [PATCH 10/10] docs: corrected docs file formatting --- .../docs/rules/no-redundant-type-constituents.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md b/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md index b887e5be586..e427c671d3f 100644 --- a/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md +++ b/packages/eslint-plugin/docs/rules/no-redundant-type-constituents.md @@ -1,9 +1,10 @@ -# Disallow members of unions and intersections that do nothing or override type information (`no-redundant-type-constituents`) +# `no-redundant-type-constituents` -Some types can override some other types ("constituents") in a union or intersection and/or be overridden by some other types. +Disallow members of unions and intersections that do nothing or override type information. ## Rule Details +Some types can override some other types ("constituents") in a union or intersection and/or be overridden by some other types. TypeScript's set theory of types includes cases where a constituent type might be useless in the parent union or intersection. Within `|` unions: