From fedea7afc6779ba926a8a0cb39880bf6c699a908 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 31 Jan 2023 15:11:56 +1030 Subject: [PATCH 1/7] feat(eslint-plugin): add rule to warn against runtime side effects with verbatimModuleSyntax --- .../docs/rules/consistent-type-imports.md | 6 ++ ...type-imports-with-verbatim-side-effects.md | 74 ++++++++++++++++++ packages/eslint-plugin/src/configs/all.ts | 4 +- packages/eslint-plugin/src/configs/strict.ts | 2 +- .../src/rules/consistent-type-imports.ts | 24 ++---- packages/eslint-plugin/src/rules/index.ts | 3 + ...type-imports-with-verbatim-side-effects.ts | 78 +++++++++++++++++++ ...imports-with-verbatim-side-effects.test.ts | 44 +++++++++++ packages/utils/src/ast-utils/predicates.ts | 16 ++++ packages/utils/src/ts-eslint/SourceCode.ts | 19 ++++- 10 files changed, 247 insertions(+), 23 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md create mode 100644 packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts create mode 100644 packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts diff --git a/packages/eslint-plugin/docs/rules/consistent-type-imports.md b/packages/eslint-plugin/docs/rules/consistent-type-imports.md index 045b7c2fa7d..600f4087912 100644 --- a/packages/eslint-plugin/docs/rules/consistent-type-imports.md +++ b/packages/eslint-plugin/docs/rules/consistent-type-imports.md @@ -95,3 +95,9 @@ If you are using [type-aware linting](https://typescript-eslint.io/linting/typed ## When Not To Use It - If you specifically want to use both import kinds for stylistic reasons, you can disable this rule. + +## Related To + +- [`no-type-imports-with-verbatim-side-effects`](./no-type-imports-with-verbatim-side-effects.md) +- [`import/consistent-type-specifier-style`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md) +- [`import/no-duplicates` with `{"prefer-inline": true}`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports) diff --git a/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md b/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md new file mode 100644 index 00000000000..2d8441853e8 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md @@ -0,0 +1,74 @@ +--- +description: 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-type-imports-with-verbatim-side-effects** for documentation. + +The [`--verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option causes TypeScript to do simple and predictable transpilation on import declarations, namely it completely removes import declarations with a top-level `type` qualifier, and it removes any import specifiers with an inline `type` qualifier. + +The latter behavior does have one potentially surprising effect in that in certain cases TS can leave behind a "side effect" import at runtime: + +```ts +import { type A, type B } from 'mod'; + +// is transpiled to + +import {} from 'mod'; +// which is the same as +import 'mod'; +``` + +For some rare cases, this may be desirable - but for many cases you will not want to leave behind an unnecessary side effect import. + +## Examples + +This rule enforces that you use a top-level `type` qualifier for imports when it only imports specifiers with an inline `type` qualifier + + + +### ❌ Incorrect + +```ts +import { type A } from 'mod'; +import { type A as AA } from 'mod'; +import { type A, type B } from 'mod'; +import { type A as AA, type B as BB } from 'mod'; +``` + +### ✅ Correct + +```ts +import type { A } from 'mod'; +import type { A as AA } from 'mod'; +import type { A, B } from 'mod'; +import type { A as AA, B as BB } from 'mod'; + +import T from 'mod'; +import type T from 'mod'; + +import * as T from 'mod'; +import type * as T from 'mod'; + +import { T } from 'mod'; +import type { T } from 'mod'; +import { T, U } from 'mod'; +import type { T, U } from 'mod'; +import { type T, U } from 'mod'; +import { T, type U } from 'mod'; + +import type T, { U } from 'mod'; +import T, { type U } from 'mod'; +``` + +## When Not To Use It + +- If you want to leave behind side effect imports, then you shouldn't use this rule. +- If you're not using TypeScript 5.0's `verbatimModuleSyntax` option, then you don't need this rule. + +## Related To + +- [`consistent-type-imports`](./consistent-type-imports.md) +- [`import/consistent-type-specifier-style`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md) +- [`import/no-duplicates` with `{"prefer-inline": true}`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 452035c4ebf..0297acdfc28 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -55,7 +55,6 @@ export = { 'no-dupe-class-members': 'off', '@typescript-eslint/no-dupe-class-members': 'error', '@typescript-eslint/no-duplicate-enum-values': 'error', - 'no-duplicate-imports': 'off', '@typescript-eslint/no-dynamic-delete': 'error', 'no-empty-function': 'off', '@typescript-eslint/no-empty-function': 'error', @@ -88,7 +87,6 @@ export = { '@typescript-eslint/no-non-null-asserted-nullish-coalescing': 'error', '@typescript-eslint/no-non-null-asserted-optional-chain': 'error', '@typescript-eslint/no-non-null-assertion': 'error', - '@typescript-eslint/parameter-properties': 'error', 'no-redeclare': 'off', '@typescript-eslint/no-redeclare': 'error', '@typescript-eslint/no-redundant-type-constituents': 'error', @@ -101,6 +99,7 @@ export = { 'no-throw-literal': 'off', '@typescript-eslint/no-throw-literal': 'error', '@typescript-eslint/no-type-alias': 'error', + '@typescript-eslint/no-type-imports-with-verbatim-side-effects': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', @@ -128,6 +127,7 @@ export = { '@typescript-eslint/object-curly-spacing': 'error', 'padding-line-between-statements': 'off', '@typescript-eslint/padding-line-between-statements': 'error', + '@typescript-eslint/parameter-properties': 'error', '@typescript-eslint/prefer-as-const': 'error', '@typescript-eslint/prefer-enum-initializers': 'error', '@typescript-eslint/prefer-for-of': 'error', diff --git a/packages/eslint-plugin/src/configs/strict.ts b/packages/eslint-plugin/src/configs/strict.ts index ccd44b85a0a..99b4e83b508 100644 --- a/packages/eslint-plugin/src/configs/strict.ts +++ b/packages/eslint-plugin/src/configs/strict.ts @@ -8,8 +8,8 @@ export = { '@typescript-eslint/array-type': 'warn', '@typescript-eslint/ban-tslint-comment': 'warn', '@typescript-eslint/class-literal-property-style': 'warn', - '@typescript-eslint/consistent-indexed-object-style': 'warn', '@typescript-eslint/consistent-generic-constructors': 'warn', + '@typescript-eslint/consistent-indexed-object-style': 'warn', '@typescript-eslint/consistent-type-assertions': 'warn', '@typescript-eslint/consistent-type-definitions': 'warn', 'dot-notation': 'off', diff --git a/packages/eslint-plugin/src/rules/consistent-type-imports.ts b/packages/eslint-plugin/src/rules/consistent-type-imports.ts index a812116f2f8..4c5cf771901 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-imports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-imports.ts @@ -1,5 +1,5 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES, AST_TOKEN_TYPES } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; import * as util from '../util'; @@ -32,18 +32,6 @@ interface ReportValueImport { inlineTypeSpecifiers: TSESTree.ImportSpecifier[]; } -function isImportToken( - token: TSESTree.Token, -): token is TSESTree.KeywordToken & { value: 'import' } { - return token.type === AST_TOKEN_TYPES.Keyword && token.value === 'import'; -} - -function isTypeToken( - token: TSESTree.Token, -): token is TSESTree.IdentifierToken & { value: 'type' } { - return token.type === AST_TOKEN_TYPES.Identifier && token.value === 'type'; -} - type MessageIds = | 'typeOverValue' | 'someImportsAreOnlyTypes' @@ -751,7 +739,7 @@ export default util.createRule({ ) { if (report.typeSpecifiers.length === node.specifiers.length) { const importToken = util.nullThrows( - sourceCode.getFirstToken(node, isImportToken), + sourceCode.getFirstToken(node, util.isImportKeyword), util.NullThrowsReasons.MissingToken('import', node.type), ); // import type Type from 'foo' @@ -800,7 +788,7 @@ export default util.createRule({ // import type Foo from 'foo' // ^^^^^ insert const importToken = util.nullThrows( - sourceCode.getFirstToken(node, isImportToken), + sourceCode.getFirstToken(node, util.isImportKeyword), util.NullThrowsReasons.MissingToken('import', node.type), ); yield fixer.insertTextAfter(importToken, ' type'); @@ -945,14 +933,14 @@ export default util.createRule({ // import type Foo from 'foo' // ^^^^ remove const importToken = util.nullThrows( - sourceCode.getFirstToken(node, isImportToken), + sourceCode.getFirstToken(node, util.isImportKeyword), util.NullThrowsReasons.MissingToken('import', node.type), ); const typeToken = util.nullThrows( sourceCode.getFirstTokenBetween( importToken, node.specifiers[0]?.local ?? node.source, - isTypeToken, + util.isTypeKeyword, ), util.NullThrowsReasons.MissingToken('type', node.type), ); @@ -970,7 +958,7 @@ export default util.createRule({ // import { type Foo } from 'foo' // ^^^^ remove const typeToken = util.nullThrows( - sourceCode.getFirstToken(node, isTypeToken), + sourceCode.getFirstToken(node, util.isTypeKeyword), util.NullThrowsReasons.MissingToken('type', node.type), ); const afterToken = util.nullThrows( diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index f7e51fdabd5..773012940d7 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -70,6 +70,7 @@ import noShadow from './no-shadow'; import noThisAlias from './no-this-alias'; import noThrowLiteral from './no-throw-literal'; import noTypeAlias from './no-type-alias'; +import noTypeImportsWithVerbatimSideEffects from './no-type-imports-with-verbatim-side-effects'; import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare'; import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; @@ -202,6 +203,8 @@ export default { 'no-this-alias': noThisAlias, 'no-throw-literal': noThrowLiteral, 'no-type-alias': noTypeAlias, + 'no-type-imports-with-verbatim-side-effects': + noTypeImportsWithVerbatimSideEffects, 'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare, 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-qualifier': noUnnecessaryQualifier, diff --git a/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts b/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts new file mode 100644 index 00000000000..a10731e46ef --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts @@ -0,0 +1,78 @@ +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import * as util from '../util'; + +type Options = []; +type MessageIds = 'useTopLevelQualifier'; + +export default util.createRule({ + name: 'no-type-imports-with-verbatim-side-effects', + meta: { + type: 'problem', + docs: { + description: + 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers', + recommended: false, + }, + fixable: 'code', + messages: { + useTopLevelQualifier: + 'TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime. Convert this to a top-level type qualifier to properly remove the entire import.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode(); + return { + ImportDeclaration(node): void { + if (node.importKind === 'type') { + return; + } + + const specifiers: TSESTree.ImportSpecifier[] = []; + for (const specifier of node.specifiers) { + if (specifier.type !== AST_NODE_TYPES.ImportSpecifier) { + return; + } + if (specifier.importKind !== 'type') { + return; + } + specifiers.push(specifier); + } + + context.report({ + node, + messageId: 'useTopLevelQualifier', + fix(fixer) { + const fixes: TSESLint.RuleFix[] = []; + for (const specifier of specifiers) { + const qualifier = util.nullThrows( + sourceCode.getFirstToken(specifier, util.isTypeKeyword), + util.NullThrowsReasons.MissingToken( + 'type keyword', + 'import specifier', + ), + ); + fixes.push( + fixer.removeRange([ + qualifier.range[0], + specifier.imported.range[0], + ]), + ); + } + + const importKeyword = util.nullThrows( + sourceCode.getFirstToken(node, util.isImportKeyword), + util.NullThrowsReasons.MissingToken('import keyword', 'import'), + ); + fixes.push(fixer.insertTextAfter(importKeyword, ' type')); + + return fixes; + }, + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts b/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts new file mode 100644 index 00000000000..68d876f2b85 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts @@ -0,0 +1,44 @@ +import rule from '../../src/rules/no-type-imports-with-verbatim-side-effects'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-type-imports-with-verbatim-side-effects', rule, { + valid: [ + "import T from 'mod';", + "import * as T from 'mod';", + "import { T } from 'mod';", + "import type { T } from 'mod';", + "import type { T, U } from 'mod';", + "import { type T, U } from 'mod';", + "import { T, type U } from 'mod';", + "import type T from 'mod';", + "import type T, { U } from 'mod';", + "import T, { type U } from 'mod';", + "import type * as T from 'mod';", + ], + invalid: [ + { + code: "import { type A } from 'mod';", + output: "import type { A } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A as AA } from 'mod';", + output: "import type { A as AA } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A, type B } from 'mod';", + output: "import type { A, B } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A as AA, type B as BB } from 'mod';", + output: "import type { A as AA, B as BB } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + ], +}); diff --git a/packages/utils/src/ast-utils/predicates.ts b/packages/utils/src/ast-utils/predicates.ts index 6a65cab7625..36e08ee5c42 100644 --- a/packages/utils/src/ast-utils/predicates.ts +++ b/packages/utils/src/ast-utils/predicates.ts @@ -139,6 +139,20 @@ const isAwaitKeyword = isTokenOfTypeWithConditions(AST_TOKEN_TYPES.Identifier, { value: 'await', }); +/** + * Checks if a possible token is the `type` keyword. + */ +const isTypeKeyword = isTokenOfTypeWithConditions(AST_TOKEN_TYPES.Identifier, { + value: 'type', +}); + +/** + * Checks if a possible token is the `import` keyword. + */ +const isImportKeyword = isTokenOfTypeWithConditions(AST_TOKEN_TYPES.Keyword, { + value: 'import', +}); + const isLoop = isNodeOfTypes([ AST_NODE_TYPES.DoWhileStatement, AST_NODE_TYPES.ForStatement, @@ -156,6 +170,7 @@ export { isFunctionOrFunctionType, isFunctionType, isIdentifier, + isImportKeyword, isLoop, isLogicalOrOperator, isNonNullAssertionPunctuator, @@ -167,5 +182,6 @@ export { isTSConstructorType, isTSFunctionType, isTypeAssertion, + isTypeKeyword, isVariableDeclarator, }; diff --git a/packages/utils/src/ts-eslint/SourceCode.ts b/packages/utils/src/ts-eslint/SourceCode.ts index 447c9debedb..a44cdee3676 100644 --- a/packages/utils/src/ts-eslint/SourceCode.ts +++ b/packages/utils/src/ts-eslint/SourceCode.ts @@ -389,10 +389,25 @@ namespace SourceCode { } export type FilterPredicate = (token: TSESTree.Token) => boolean; + export type GetFilterPredicate = + // https://github.com/prettier/prettier/issues/14275 + // prettier-ignore + TFilter extends (( + token: TSESTree.Token, + ) => token is infer U extends TSESTree.Token) + ? U + : TDefault; + export type GetFilterPredicateFromOptions = + TOptions extends { filter?: FilterPredicate } + ? GetFilterPredicate + : GetFilterPredicate; export type ReturnTypeFromOptions = T extends { includeComments: true } - ? TSESTree.Token - : Exclude; + ? GetFilterPredicateFromOptions + : GetFilterPredicateFromOptions< + T, + Exclude + >; export type CursorWithSkipOptions = | number From fb41743631a2ad8554027099544386c072906dd8 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Tue, 31 Jan 2023 15:43:17 +1030 Subject: [PATCH 2/7] lint --- packages/type-utils/tests/isTypeReadonly.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/type-utils/tests/isTypeReadonly.test.ts b/packages/type-utils/tests/isTypeReadonly.test.ts index 382091a2375..0b171dc12a2 100644 --- a/packages/type-utils/tests/isTypeReadonly.test.ts +++ b/packages/type-utils/tests/isTypeReadonly.test.ts @@ -4,8 +4,8 @@ import path from 'path'; import type * as ts from 'typescript'; import { - type ReadonlynessOptions, isTypeReadonly, + type ReadonlynessOptions, } from '../src/isTypeReadonly'; describe('isTypeReadonly', () => { From 997ae3e01d139386482b4c2b76ebe0cdaf119296 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Feb 2023 12:59:54 +1030 Subject: [PATCH 3/7] rename rule --- .../docs/rules/consistent-type-imports.md | 2 +- ...type-imports-with-verbatim-side-effects.md | 74 ------------------ packages/eslint-plugin/src/configs/all.ts | 2 +- packages/eslint-plugin/src/rules/index.ts | 5 +- ...type-imports-with-verbatim-side-effects.ts | 78 ------------------- ...imports-with-verbatim-side-effects.test.ts | 44 ----------- 6 files changed, 4 insertions(+), 201 deletions(-) delete mode 100644 packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md delete mode 100644 packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts delete mode 100644 packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts diff --git a/packages/eslint-plugin/docs/rules/consistent-type-imports.md b/packages/eslint-plugin/docs/rules/consistent-type-imports.md index 600f4087912..baa41eccda1 100644 --- a/packages/eslint-plugin/docs/rules/consistent-type-imports.md +++ b/packages/eslint-plugin/docs/rules/consistent-type-imports.md @@ -98,6 +98,6 @@ If you are using [type-aware linting](https://typescript-eslint.io/linting/typed ## Related To -- [`no-type-imports-with-verbatim-side-effects`](./no-type-imports-with-verbatim-side-effects.md) +- [`no-import-type-side-effects`](./no-import-type-side-effects.md) - [`import/consistent-type-specifier-style`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md) - [`import/no-duplicates` with `{"prefer-inline": true}`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports) diff --git a/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md b/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md deleted file mode 100644 index 2d8441853e8..00000000000 --- a/packages/eslint-plugin/docs/rules/no-type-imports-with-verbatim-side-effects.md +++ /dev/null @@ -1,74 +0,0 @@ ---- -description: 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers.' ---- - -> 🛑 This file is source code, not the primary documentation location! 🛑 -> -> See **https://typescript-eslint.io/rules/no-type-imports-with-verbatim-side-effects** for documentation. - -The [`--verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option causes TypeScript to do simple and predictable transpilation on import declarations, namely it completely removes import declarations with a top-level `type` qualifier, and it removes any import specifiers with an inline `type` qualifier. - -The latter behavior does have one potentially surprising effect in that in certain cases TS can leave behind a "side effect" import at runtime: - -```ts -import { type A, type B } from 'mod'; - -// is transpiled to - -import {} from 'mod'; -// which is the same as -import 'mod'; -``` - -For some rare cases, this may be desirable - but for many cases you will not want to leave behind an unnecessary side effect import. - -## Examples - -This rule enforces that you use a top-level `type` qualifier for imports when it only imports specifiers with an inline `type` qualifier - - - -### ❌ Incorrect - -```ts -import { type A } from 'mod'; -import { type A as AA } from 'mod'; -import { type A, type B } from 'mod'; -import { type A as AA, type B as BB } from 'mod'; -``` - -### ✅ Correct - -```ts -import type { A } from 'mod'; -import type { A as AA } from 'mod'; -import type { A, B } from 'mod'; -import type { A as AA, B as BB } from 'mod'; - -import T from 'mod'; -import type T from 'mod'; - -import * as T from 'mod'; -import type * as T from 'mod'; - -import { T } from 'mod'; -import type { T } from 'mod'; -import { T, U } from 'mod'; -import type { T, U } from 'mod'; -import { type T, U } from 'mod'; -import { T, type U } from 'mod'; - -import type T, { U } from 'mod'; -import T, { type U } from 'mod'; -``` - -## When Not To Use It - -- If you want to leave behind side effect imports, then you shouldn't use this rule. -- If you're not using TypeScript 5.0's `verbatimModuleSyntax` option, then you don't need this rule. - -## Related To - -- [`consistent-type-imports`](./consistent-type-imports.md) -- [`import/consistent-type-specifier-style`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md) -- [`import/no-duplicates` with `{"prefer-inline": true}`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports) diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index 0297acdfc28..eb3856f10c3 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -70,6 +70,7 @@ export = { '@typescript-eslint/no-for-in-array': 'error', 'no-implied-eval': 'off', '@typescript-eslint/no-implied-eval': 'error', + '@typescript-eslint/no-import-type-side-effects': 'error', '@typescript-eslint/no-inferrable-types': 'error', 'no-invalid-this': 'off', '@typescript-eslint/no-invalid-this': 'error', @@ -99,7 +100,6 @@ export = { 'no-throw-literal': 'off', '@typescript-eslint/no-throw-literal': 'error', '@typescript-eslint/no-type-alias': 'error', - '@typescript-eslint/no-type-imports-with-verbatim-side-effects': 'error', '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', '@typescript-eslint/no-unnecessary-condition': 'error', '@typescript-eslint/no-unnecessary-qualifier': 'error', diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 773012940d7..bbddfc8d470 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -48,6 +48,7 @@ import noFloatingPromises from './no-floating-promises'; import noForInArray from './no-for-in-array'; import noImplicitAnyCatch from './no-implicit-any-catch'; import noImpliedEval from './no-implied-eval'; +import noImportTypeSideEffects from './no-import-type-side-effects'; import noInferrableTypes from './no-inferrable-types'; import noInvalidThis from './no-invalid-this'; import noInvalidVoidType from './no-invalid-void-type'; @@ -70,7 +71,6 @@ import noShadow from './no-shadow'; import noThisAlias from './no-this-alias'; import noThrowLiteral from './no-throw-literal'; import noTypeAlias from './no-type-alias'; -import noTypeImportsWithVerbatimSideEffects from './no-type-imports-with-verbatim-side-effects'; import noUnnecessaryBooleanLiteralCompare from './no-unnecessary-boolean-literal-compare'; import noUnnecessaryCondition from './no-unnecessary-condition'; import noUnnecessaryQualifier from './no-unnecessary-qualifier'; @@ -181,6 +181,7 @@ export default { 'no-for-in-array': noForInArray, 'no-implicit-any-catch': noImplicitAnyCatch, 'no-implied-eval': noImpliedEval, + 'no-import-type-side-effects': noImportTypeSideEffects, 'no-inferrable-types': noInferrableTypes, 'no-invalid-this': noInvalidThis, 'no-invalid-void-type': noInvalidVoidType, @@ -203,8 +204,6 @@ export default { 'no-this-alias': noThisAlias, 'no-throw-literal': noThrowLiteral, 'no-type-alias': noTypeAlias, - 'no-type-imports-with-verbatim-side-effects': - noTypeImportsWithVerbatimSideEffects, 'no-unnecessary-boolean-literal-compare': noUnnecessaryBooleanLiteralCompare, 'no-unnecessary-condition': noUnnecessaryCondition, 'no-unnecessary-qualifier': noUnnecessaryQualifier, diff --git a/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts b/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts deleted file mode 100644 index a10731e46ef..00000000000 --- a/packages/eslint-plugin/src/rules/no-type-imports-with-verbatim-side-effects.ts +++ /dev/null @@ -1,78 +0,0 @@ -import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/utils'; - -import * as util from '../util'; - -type Options = []; -type MessageIds = 'useTopLevelQualifier'; - -export default util.createRule({ - name: 'no-type-imports-with-verbatim-side-effects', - meta: { - type: 'problem', - docs: { - description: - 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers', - recommended: false, - }, - fixable: 'code', - messages: { - useTopLevelQualifier: - 'TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime. Convert this to a top-level type qualifier to properly remove the entire import.', - }, - schema: [], - }, - defaultOptions: [], - create(context) { - const sourceCode = context.getSourceCode(); - return { - ImportDeclaration(node): void { - if (node.importKind === 'type') { - return; - } - - const specifiers: TSESTree.ImportSpecifier[] = []; - for (const specifier of node.specifiers) { - if (specifier.type !== AST_NODE_TYPES.ImportSpecifier) { - return; - } - if (specifier.importKind !== 'type') { - return; - } - specifiers.push(specifier); - } - - context.report({ - node, - messageId: 'useTopLevelQualifier', - fix(fixer) { - const fixes: TSESLint.RuleFix[] = []; - for (const specifier of specifiers) { - const qualifier = util.nullThrows( - sourceCode.getFirstToken(specifier, util.isTypeKeyword), - util.NullThrowsReasons.MissingToken( - 'type keyword', - 'import specifier', - ), - ); - fixes.push( - fixer.removeRange([ - qualifier.range[0], - specifier.imported.range[0], - ]), - ); - } - - const importKeyword = util.nullThrows( - sourceCode.getFirstToken(node, util.isImportKeyword), - util.NullThrowsReasons.MissingToken('import keyword', 'import'), - ); - fixes.push(fixer.insertTextAfter(importKeyword, ' type')); - - return fixes; - }, - }); - }, - }; - }, -}); diff --git a/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts b/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts deleted file mode 100644 index 68d876f2b85..00000000000 --- a/packages/eslint-plugin/tests/rules/no-type-imports-with-verbatim-side-effects.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import rule from '../../src/rules/no-type-imports-with-verbatim-side-effects'; -import { RuleTester } from '../RuleTester'; - -const ruleTester = new RuleTester({ - parser: '@typescript-eslint/parser', -}); - -ruleTester.run('no-type-imports-with-verbatim-side-effects', rule, { - valid: [ - "import T from 'mod';", - "import * as T from 'mod';", - "import { T } from 'mod';", - "import type { T } from 'mod';", - "import type { T, U } from 'mod';", - "import { type T, U } from 'mod';", - "import { T, type U } from 'mod';", - "import type T from 'mod';", - "import type T, { U } from 'mod';", - "import T, { type U } from 'mod';", - "import type * as T from 'mod';", - ], - invalid: [ - { - code: "import { type A } from 'mod';", - output: "import type { A } from 'mod';", - errors: [{ messageId: 'useTopLevelQualifier' }], - }, - { - code: "import { type A as AA } from 'mod';", - output: "import type { A as AA } from 'mod';", - errors: [{ messageId: 'useTopLevelQualifier' }], - }, - { - code: "import { type A, type B } from 'mod';", - output: "import type { A, B } from 'mod';", - errors: [{ messageId: 'useTopLevelQualifier' }], - }, - { - code: "import { type A as AA, type B as BB } from 'mod';", - output: "import type { A as AA, B as BB } from 'mod';", - errors: [{ messageId: 'useTopLevelQualifier' }], - }, - ], -}); From 903dd6a27076d67ce181b1c97ead0d82a8cb8594 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Feb 2023 13:00:04 +1030 Subject: [PATCH 4/7] rename rule --- .../docs/rules/no-import-type-side-effects.md | 74 ++++++++++++++++++ .../src/rules/no-import-type-side-effects.ts | 78 +++++++++++++++++++ .../rules/no-import-type-side-effects.test.ts | 44 +++++++++++ 3 files changed, 196 insertions(+) create mode 100644 packages/eslint-plugin/docs/rules/no-import-type-side-effects.md create mode 100644 packages/eslint-plugin/src/rules/no-import-type-side-effects.ts create mode 100644 packages/eslint-plugin/tests/rules/no-import-type-side-effects.test.ts diff --git a/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md b/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md new file mode 100644 index 00000000000..4cc35547e6f --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md @@ -0,0 +1,74 @@ +--- +description: 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers.' +--- + +> 🛑 This file is source code, not the primary documentation location! 🛑 +> +> See **https://typescript-eslint.io/rules/no-import-type-side-effects** for documentation. + +The [`--verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option causes TypeScript to do simple and predictable transpilation on import declarations, namely it completely removes import declarations with a top-level `type` qualifier, and it removes any import specifiers with an inline `type` qualifier. + +The latter behavior does have one potentially surprising effect in that in certain cases TS can leave behind a "side effect" import at runtime: + +```ts +import { type A, type B } from 'mod'; + +// is transpiled to + +import {} from 'mod'; +// which is the same as +import 'mod'; +``` + +For some rare cases, this may be desirable - but for many cases you will not want to leave behind an unnecessary side effect import. + +## Examples + +This rule enforces that you use a top-level `type` qualifier for imports when it only imports specifiers with an inline `type` qualifier + + + +### ❌ Incorrect + +```ts +import { type A } from 'mod'; +import { type A as AA } from 'mod'; +import { type A, type B } from 'mod'; +import { type A as AA, type B as BB } from 'mod'; +``` + +### ✅ Correct + +```ts +import type { A } from 'mod'; +import type { A as AA } from 'mod'; +import type { A, B } from 'mod'; +import type { A as AA, B as BB } from 'mod'; + +import T from 'mod'; +import type T from 'mod'; + +import * as T from 'mod'; +import type * as T from 'mod'; + +import { T } from 'mod'; +import type { T } from 'mod'; +import { T, U } from 'mod'; +import type { T, U } from 'mod'; +import { type T, U } from 'mod'; +import { T, type U } from 'mod'; + +import type T, { U } from 'mod'; +import T, { type U } from 'mod'; +``` + +## When Not To Use It + +- If you want to leave behind side effect imports, then you shouldn't use this rule. +- If you're not using TypeScript 5.0's `verbatimModuleSyntax` option, then you don't need this rule. + +## Related To + +- [`consistent-type-imports`](./consistent-type-imports.md) +- [`import/consistent-type-specifier-style`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/consistent-type-specifier-style.md) +- [`import/no-duplicates` with `{"prefer-inline": true}`](https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-duplicates.md#inline-type-imports) diff --git a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts new file mode 100644 index 00000000000..ee2a4d3291e --- /dev/null +++ b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts @@ -0,0 +1,78 @@ +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/utils'; + +import * as util from '../util'; + +type Options = []; +type MessageIds = 'useTopLevelQualifier'; + +export default util.createRule({ + name: 'no-import-type-side-effects', + meta: { + type: 'problem', + docs: { + description: + 'Enforce the use of top-level import type qualifier when an import only has specifiers with inline type qualifiers', + recommended: false, + }, + fixable: 'code', + messages: { + useTopLevelQualifier: + 'TypeScript will only remove the inline type specifiers which will leave behind a side effect import at runtime. Convert this to a top-level type qualifier to properly remove the entire import.', + }, + schema: [], + }, + defaultOptions: [], + create(context) { + const sourceCode = context.getSourceCode(); + return { + ImportDeclaration(node): void { + if (node.importKind === 'type') { + return; + } + + const specifiers: TSESTree.ImportSpecifier[] = []; + for (const specifier of node.specifiers) { + if (specifier.type !== AST_NODE_TYPES.ImportSpecifier) { + return; + } + if (specifier.importKind !== 'type') { + return; + } + specifiers.push(specifier); + } + + context.report({ + node, + messageId: 'useTopLevelQualifier', + fix(fixer) { + const fixes: TSESLint.RuleFix[] = []; + for (const specifier of specifiers) { + const qualifier = util.nullThrows( + sourceCode.getFirstToken(specifier, util.isTypeKeyword), + util.NullThrowsReasons.MissingToken( + 'type keyword', + 'import specifier', + ), + ); + fixes.push( + fixer.removeRange([ + qualifier.range[0], + specifier.imported.range[0], + ]), + ); + } + + const importKeyword = util.nullThrows( + sourceCode.getFirstToken(node, util.isImportKeyword), + util.NullThrowsReasons.MissingToken('import keyword', 'import'), + ); + fixes.push(fixer.insertTextAfter(importKeyword, ' type')); + + return fixes; + }, + }); + }, + }; + }, +}); diff --git a/packages/eslint-plugin/tests/rules/no-import-type-side-effects.test.ts b/packages/eslint-plugin/tests/rules/no-import-type-side-effects.test.ts new file mode 100644 index 00000000000..9dade06a943 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/no-import-type-side-effects.test.ts @@ -0,0 +1,44 @@ +import rule from '../../src/rules/no-import-type-side-effects'; +import { RuleTester } from '../RuleTester'; + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', +}); + +ruleTester.run('no-import-type-side-effects', rule, { + valid: [ + "import T from 'mod';", + "import * as T from 'mod';", + "import { T } from 'mod';", + "import type { T } from 'mod';", + "import type { T, U } from 'mod';", + "import { type T, U } from 'mod';", + "import { T, type U } from 'mod';", + "import type T from 'mod';", + "import type T, { U } from 'mod';", + "import T, { type U } from 'mod';", + "import type * as T from 'mod';", + ], + invalid: [ + { + code: "import { type A } from 'mod';", + output: "import type { A } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A as AA } from 'mod';", + output: "import type { A as AA } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A, type B } from 'mod';", + output: "import type { A, B } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + { + code: "import { type A as AA, type B as BB } from 'mod';", + output: "import type { A as AA, B as BB } from 'mod';", + errors: [{ messageId: 'useTopLevelQualifier' }], + }, + ], +}); From 3162d562f6423d0a5f9c58aebbef55192734fa28 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Feb 2023 10:03:58 +1030 Subject: [PATCH 5/7] Apply suggestions from code review Co-authored-by: Josh Goldberg (cherry picked from commit 3848375986d7f43232e60568f29f1088a520bbab) --- .../docs/rules/no-import-type-side-effects.md | 5 +++-- .../src/rules/no-import-type-side-effects.ts | 17 +++++++---------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md b/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md index 4cc35547e6f..35b8f2c5282 100644 --- a/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md +++ b/packages/eslint-plugin/docs/rules/no-import-type-side-effects.md @@ -6,7 +6,8 @@ description: 'Enforce the use of top-level import type qualifier when an import > > See **https://typescript-eslint.io/rules/no-import-type-side-effects** for documentation. -The [`--verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option causes TypeScript to do simple and predictable transpilation on import declarations, namely it completely removes import declarations with a top-level `type` qualifier, and it removes any import specifiers with an inline `type` qualifier. +The [`--verbatimModuleSyntax`](https://www.typescriptlang.org/tsconfig#verbatimModuleSyntax) compiler option causes TypeScript to do simple and predictable transpilation on import declarations. +Namely, it completely removes import declarations with a top-level `type` qualifier, and it removes any import specifiers with an inline `type` qualifier. The latter behavior does have one potentially surprising effect in that in certain cases TS can leave behind a "side effect" import at runtime: @@ -20,7 +21,7 @@ import {} from 'mod'; import 'mod'; ``` -For some rare cases, this may be desirable - but for many cases you will not want to leave behind an unnecessary side effect import. +For the rare case of needing to import for side effects, this may be desirable - but for most cases you will not want to leave behind an unnecessary side effect import. ## Examples diff --git a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts index ee2a4d3291e..f087ffe565f 100644 --- a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts +++ b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts @@ -26,19 +26,16 @@ export default util.createRule({ create(context) { const sourceCode = context.getSourceCode(); return { - ImportDeclaration(node): void { - if (node.importKind === 'type') { - return; - } + 'ImportDeclaration[importKind!="type"]'( + node: TSESTree.ImportDeclaration, + ): void { const specifiers: TSESTree.ImportSpecifier[] = []; for (const specifier of node.specifiers) { - if (specifier.type !== AST_NODE_TYPES.ImportSpecifier) { - return; - } - if (specifier.importKind !== 'type') { - return; - } + if ( + specifier.type !== AST_NODE_TYPES.ImportSpecifier || + specifier.importKind !== 'type' + ) { specifiers.push(specifier); } From af024eea6a9791a39ce2ef563e0352c53cd7a58b Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Feb 2023 13:07:15 +1030 Subject: [PATCH 6/7] bad merge --- .../src/rules/no-import-type-side-effects.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts index f087ffe565f..9b0d08d6ec4 100644 --- a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts +++ b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts @@ -29,14 +29,14 @@ export default util.createRule({ 'ImportDeclaration[importKind!="type"]'( node: TSESTree.ImportDeclaration, ): void { - const specifiers: TSESTree.ImportSpecifier[] = []; for (const specifier of node.specifiers) { if ( - specifier.type !== AST_NODE_TYPES.ImportSpecifier || - specifier.importKind !== 'type' + specifier.type === AST_NODE_TYPES.ImportSpecifier && + specifier.importKind === 'type' ) { - specifiers.push(specifier); + specifiers.push(specifier); + } } context.report({ From da7b02ff20b7d35de01fad18005ea7048c56e0bc Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Wed, 1 Feb 2023 14:54:56 +1030 Subject: [PATCH 7/7] fix bad change --- .../eslint-plugin/src/rules/no-import-type-side-effects.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts index 9b0d08d6ec4..ce80a654afe 100644 --- a/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts +++ b/packages/eslint-plugin/src/rules/no-import-type-side-effects.ts @@ -32,11 +32,12 @@ export default util.createRule({ const specifiers: TSESTree.ImportSpecifier[] = []; for (const specifier of node.specifiers) { if ( - specifier.type === AST_NODE_TYPES.ImportSpecifier && - specifier.importKind === 'type' + specifier.type !== AST_NODE_TYPES.ImportSpecifier || + specifier.importKind !== 'type' ) { - specifiers.push(specifier); + return; } + specifiers.push(specifier); } context.report({