Skip to content

Commit

Permalink
feat(eslint-plugin): [no-import-type-side-effects] add rule to warn a…
Browse files Browse the repository at this point in the history
…gainst runtime side effects with `verbatimModuleSyntax` (#6394)
  • Loading branch information
bradzacher committed Feb 1, 2023
1 parent add18e7 commit b14d3be
Show file tree
Hide file tree
Showing 10 changed files with 245 additions and 23 deletions.
6 changes: 6 additions & 0 deletions packages/eslint-plugin/docs/rules/consistent-type-imports.md
Expand Up @@ -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-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)
75 changes: 75 additions & 0 deletions packages/eslint-plugin/docs/rules/no-import-type-side-effects.md
@@ -0,0 +1,75 @@
---
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 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

This rule enforces that you use a top-level `type` qualifier for imports when it only imports specifiers with an inline `type` qualifier

<!--tabs-->

### ❌ 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)
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -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',
Expand All @@ -71,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',
Expand All @@ -88,7 +88,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',
Expand Down Expand Up @@ -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',
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/src/configs/strict.ts
Expand Up @@ -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',
Expand Down
24 changes: 6 additions & 18 deletions 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';

Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -751,7 +739,7 @@ export default util.createRule<Options, MessageIds>({
) {
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'
Expand Down Expand Up @@ -800,7 +788,7 @@ export default util.createRule<Options, MessageIds>({
// 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');
Expand Down Expand Up @@ -945,14 +933,14 @@ export default util.createRule<Options, MessageIds>({
// 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),
);
Expand All @@ -970,7 +958,7 @@ export default util.createRule<Options, MessageIds>({
// 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(
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Expand Up @@ -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';
Expand Down Expand Up @@ -180,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,
Expand Down
76 changes: 76 additions & 0 deletions packages/eslint-plugin/src/rules/no-import-type-side-effects.ts
@@ -0,0 +1,76 @@
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<Options, MessageIds>({
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[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'
) {
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;
},
});
},
};
},
});
@@ -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' }],
},
],
});
16 changes: 16 additions & 0 deletions packages/utils/src/ast-utils/predicates.ts
Expand Up @@ -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,
Expand All @@ -156,6 +170,7 @@ export {
isFunctionOrFunctionType,
isFunctionType,
isIdentifier,
isImportKeyword,
isLoop,
isLogicalOrOperator,
isNonNullAssertionPunctuator,
Expand All @@ -167,5 +182,6 @@ export {
isTSConstructorType,
isTSFunctionType,
isTypeAssertion,
isTypeKeyword,
isVariableDeclarator,
};
19 changes: 17 additions & 2 deletions packages/utils/src/ts-eslint/SourceCode.ts
Expand Up @@ -389,10 +389,25 @@ namespace SourceCode {
}

export type FilterPredicate = (token: TSESTree.Token) => boolean;
export type GetFilterPredicate<TFilter, TDefault> =
// 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, TDefault> =
TOptions extends { filter?: FilterPredicate }
? GetFilterPredicate<TOptions['filter'], TDefault>
: GetFilterPredicate<TOptions, TDefault>;

export type ReturnTypeFromOptions<T> = T extends { includeComments: true }
? TSESTree.Token
: Exclude<TSESTree.Token, TSESTree.Comment>;
? GetFilterPredicateFromOptions<T, TSESTree.Token>
: GetFilterPredicateFromOptions<
T,
Exclude<TSESTree.Token, TSESTree.Comment>
>;

export type CursorWithSkipOptions =
| number
Expand Down

0 comments on commit b14d3be

Please sign in to comment.