diff --git a/packages/eslint-plugin/src/rules/consistent-type-imports.ts b/packages/eslint-plugin/src/rules/consistent-type-imports.ts index 6badf04d4dd6..c56f71ea0074 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-imports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-imports.ts @@ -7,11 +7,13 @@ import { import * as util from '../util'; type Prefer = 'type-imports' | 'no-type-imports'; +type FixStyle = 'separate-type-imports' | 'inline-type-imports'; type Options = [ { prefer?: Prefer; disallowTypeAnnotations?: boolean; + fixStyle?: FixStyle; }, ]; @@ -50,7 +52,8 @@ type MessageIds = | 'valueOverType' | 'noImportTypeAnnotations' | 'someImportsInDecoMeta' - | 'aImportInDecoMeta'; + | 'aImportInDecoMeta' + | 'inlineTypes'; export default util.createRule({ name: 'consistent-type-imports', meta: { @@ -71,6 +74,7 @@ export default util.createRule({ 'Type import {{typeImports}} is used by decorator metadata.', valueOverType: 'Use an `import` instead of an `import type`.', noImportTypeAnnotations: '`import()` type annotations are forbidden.', + inlineTypes: 'Type imports can be inlined.', }, schema: [ { @@ -82,6 +86,9 @@ export default util.createRule({ disallowTypeAnnotations: { type: 'boolean', }, + fixStyle: { + enum: ['separate-type-imports', 'inline-type-imports'], + }, }, additionalProperties: false, }, @@ -93,12 +100,14 @@ export default util.createRule({ { prefer: 'type-imports', disallowTypeAnnotations: true, + fixStyle: 'separate-type-imports', }, ], create(context, [option]) { const prefer = option.prefer ?? 'type-imports'; const disallowTypeAnnotations = option.disallowTypeAnnotations !== false; + const fixStyle = option.fixStyle ?? 'separate-type-imports'; const sourceCode = context.getSourceCode(); const sourceImportsMap: { [key: string]: SourceImports } = {}; @@ -109,13 +118,14 @@ export default util.createRule({ // prefer type imports ImportDeclaration(node): void { const source = node.source.value; + // sourceImports is the object containing all the specifics for a particular import source, type or value const sourceImports = sourceImportsMap[source] ?? (sourceImportsMap[source] = { source, - reportValueImports: [], - typeOnlyNamedImport: null, - valueOnlyNamedImport: null, + reportValueImports: [], // if there is a mismatch where type importKind but value specifiers + typeOnlyNamedImport: null, // if only type imports + valueOnlyNamedImport: null, // if only value imports }); if (node.importKind === 'type') { if ( @@ -125,6 +135,7 @@ export default util.createRule({ specifier.type === AST_NODE_TYPES.ImportSpecifier, ) ) { + // definitely import type { TypeX } sourceImports.typeOnlyNamedImport = node; } } else { @@ -241,17 +252,31 @@ export default util.createRule({ unusedSpecifiers, inlineTypeSpecifiers, }); + } else if ( + fixStyle === 'inline-type-imports' && + node.importKind === 'type' && + sourceImports.valueOnlyNamedImport + ) { + sourceImports.reportValueImports.push({ + node, + typeSpecifiers, + valueSpecifiers, + unusedSpecifiers, + inlineTypeSpecifiers, + }); } }, 'Program:exit'(): void { for (const sourceImports of Object.values(sourceImportsMap)) { if (sourceImports.reportValueImports.length === 0) { + // nothing to fix. value specifiers and type specifiers are correctly written continue; } for (const report of sourceImports.reportValueImports) { if ( report.valueSpecifiers.length === 0 && - report.unusedSpecifiers.length === 0 + report.unusedSpecifiers.length === 0 && + report.node.importKind !== 'type' ) { // import is all type-only, convert the entire import to `import type` context.report({ @@ -265,15 +290,16 @@ export default util.createRule({ ); }, }); - } else { + } else if (fixStyle === 'separate-type-imports') { const isTypeImport = report.node.importKind === 'type'; - // we have a mixed type/value import, so we need to split them out into multiple exports + // we have a mixed type/value import or just value imports, so we need to split them out into multiple imports if separate-type-imports is configured const importNames = ( isTypeImport - ? report.valueSpecifiers + ? report.valueSpecifiers // import type { A } from 'roo'; // WHERE A is used in value position : report.typeSpecifiers - ).map(specifier => `"${specifier.local.name}"`); + ) // import { A, B } from 'roo'; // WHERE A is used in type position and B is in value position + .map(specifier => `"${specifier.local.name}"`); const message = ((): { messageId: MessageIds; @@ -297,12 +323,12 @@ export default util.createRule({ if (isTypeImport) { return { messageId: 'someImportsInDecoMeta', - data: { typeImports }, + data: { typeImports }, // typeImports are all the value specifiers that are in the type position }; } else { return { messageId: 'someImportsAreOnlyTypes', - data: { typeImports }, + data: { typeImports }, // typeImports are all the type specifiers in the value position }; } } @@ -313,12 +339,14 @@ export default util.createRule({ ...message, *fix(fixer) { if (isTypeImport) { + // take all the valueSpecifiers and put them on a new line yield* fixToValueImportDeclaration( fixer, report, sourceImports, ); } else { + // take all the typeSpecifiers and put them on a new line yield* fixToTypeImportDeclaration( fixer, report, @@ -327,6 +355,27 @@ export default util.createRule({ } }, }); + } else if ( + fixStyle === 'inline-type-imports' && + sourceImports.valueOnlyNamedImport + ) { + // grab type imports and add to existing import if present. Unsure what imports we will fix so no imports in message + // import ValueImport, { type AType } from 'foo' + // ^^^^ add + // import ValueImport from 'foo' + // import type { type AType } from 'foo' + // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ remove and add to value import above -> import ValueImport, { type AType } from 'foo' + context.report({ + node: report.node, + messageId: 'inlineTypes', + *fix(fixer) { + yield* fixInlineTypeImportDeclaration( + fixer, + report, + sourceImports, + ); + }, + }); } } } @@ -399,12 +448,12 @@ export default util.createRule({ } /** - * Returns information for fixing named specifiers. + * Returns information for fixing named specifiers, type or value */ function getFixesNamedSpecifiers( fixer: TSESLint.RuleFixer, node: TSESTree.ImportDeclaration, - typeNamedSpecifiers: TSESTree.ImportSpecifier[], + subsetNamedSpecifiers: TSESTree.ImportSpecifier[], allNamedSpecifiers: TSESTree.ImportSpecifier[], ): { typeNamedSpecifiersText: string; @@ -418,16 +467,17 @@ export default util.createRule({ } const typeNamedSpecifiersTexts: string[] = []; const removeTypeNamedSpecifiers: TSESLint.RuleFix[] = []; - if (typeNamedSpecifiers.length === allNamedSpecifiers.length) { + if (subsetNamedSpecifiers.length === allNamedSpecifiers.length) { // import Foo, {Type1, Type2} from 'foo' // import DefType, {Type1, Type2} from 'foo' const openingBraceToken = util.nullThrows( sourceCode.getTokenBefore( - typeNamedSpecifiers[0], + subsetNamedSpecifiers[0], util.isOpeningBraceToken, ), util.NullThrowsReasons.MissingToken('{', node.type), ); + // TODO: why require default import as well? What about import type { type1, type2 }? const commaToken = util.nullThrows( sourceCode.getTokenBefore(openingBraceToken, util.isCommaToken), util.NullThrowsReasons.MissingToken(',', node.type), @@ -454,20 +504,20 @@ export default util.createRule({ ), ); } else { - const typeNamedSpecifierGroups: TSESTree.ImportSpecifier[][] = []; + const namedSpecifierGroups: TSESTree.ImportSpecifier[][] = []; let group: TSESTree.ImportSpecifier[] = []; for (const namedSpecifier of allNamedSpecifiers) { - if (typeNamedSpecifiers.includes(namedSpecifier)) { + if (subsetNamedSpecifiers.includes(namedSpecifier)) { group.push(namedSpecifier); } else if (group.length) { - typeNamedSpecifierGroups.push(group); + namedSpecifierGroups.push(group); group = []; } } if (group.length) { - typeNamedSpecifierGroups.push(group); + namedSpecifierGroups.push(group); } - for (const namedSpecifiers of typeNamedSpecifierGroups) { + for (const namedSpecifiers of namedSpecifierGroups) { const { removeRange, textRange } = getNamedSpecifierRanges( namedSpecifiers, allNamedSpecifiers, @@ -544,7 +594,104 @@ export default util.createRule({ if (!util.isCommaToken(before) && !util.isOpeningBraceToken(before)) { insertText = `,${insertText}`; } - return fixer.insertTextBefore(closingBraceToken, `${insertText}`); + return fixer.insertTextBefore(closingBraceToken, insertText); + } + + /** + * insert type keyword to named import node. + * e.g. + * import { Already, type Type1, type Type2 } from 'foo' + * ^^^^ insert + * or + * import { Already, type Type1, type Type2 } from 'foo' + * ^^^^^^^^^^ insert + */ + function fixInsertTypeKeywordInNamedSpecifierList( + fixer: TSESLint.RuleFixer, + target: TSESTree.ImportDeclaration, + typeSpecifiers: TSESTree.ImportSpecifier[], + ): TSESLint.RuleFix[] { + const rules = []; + const existingSpecifiers = target.specifiers.map(spec => spec.local.name); + for (const spec of typeSpecifiers) { + const insertText = sourceCode.text.slice(...spec.range); + if (existingSpecifiers.includes(spec.local.name)) { + rules.push(fixer.replaceTextRange(spec.range, `type ${insertText}`)); + } else { + const closingBraceToken = util.nullThrows( + sourceCode.getFirstTokenBetween( + sourceCode.getFirstToken(target)!, + target.source, + util.isClosingBraceToken, + ), + util.NullThrowsReasons.MissingToken('}', target.type), + ); + const before = sourceCode.getTokenBefore(closingBraceToken)!; + if (!util.isCommaToken(before) && !util.isOpeningBraceToken(before)) { + rules.push(fixer.insertTextAfter(before, `, type ${insertText}`)); + } else { + rules.push(fixer.insertTextAfter(before, `type ${insertText}`)); + } + } + } + + return rules; + } + + function* fixInlineTypeImportDeclaration( + fixer: TSESLint.RuleFixer, + report: ReportValueImport, + sourceImports: SourceImports, + ): IterableIterator { + const { node } = report; + if (sourceImports.valueOnlyNamedImport) { + // cannot mix default and named import types. Only care about named specifiers + const { namedSpecifiers } = classifySpecifier(node); + const typeNamedSpecifiers = namedSpecifiers.filter(specifier => + report.typeSpecifiers.includes(specifier), + ); + + if (node.importKind === 'type') { + // move import named type specifiers to its value import + const insertTypeNamedSpecifiers = + fixInsertTypeKeywordInNamedSpecifierList( + fixer, + sourceImports.valueOnlyNamedImport, + typeNamedSpecifiers, + ); + + for (const i of insertTypeNamedSpecifiers) { + yield i; + } + + // we are here if there are only named specifiers. Remove b/c now moved to valueOnlyImport + yield fixer.remove(node); + + // remove line + space leftover + const lastToken = sourceCode.getLastToken(node); + if (lastToken) { + const nextToken = sourceCode.getTokenAfter(lastToken); + if (nextToken) { + yield fixer.replaceTextRange( + [lastToken.range[1], nextToken.range[0]], + '', + ); + } + } + } else { + // add `type` before typeNameSpecifiers. + const insertTypeNamedSpecifiers = + fixInsertTypeKeywordInNamedSpecifierList( + fixer, + sourceImports.valueOnlyNamedImport, + typeNamedSpecifiers, + ); + + for (const i of insertTypeNamedSpecifiers) { + yield i; + } + } + } } function* fixToTypeImportDeclaration( @@ -597,6 +744,7 @@ export default util.createRule({ const afterFixes: TSESLint.RuleFix[] = []; if (typeNamedSpecifiers.length) { if (sourceImports.typeOnlyNamedImport) { + // TODO: is this ever the case? Seems like this isn't called when the source import is import type ... const insertTypeNamedSpecifiers = fixInsertNamedSpecifiersInNamedSpecifierList( fixer, @@ -606,6 +754,7 @@ export default util.createRule({ if (sourceImports.typeOnlyNamedImport.range[1] <= node.range[0]) { yield insertTypeNamedSpecifiers; } else { + // TODO: delay inserting b/c.... afterFixes.push(insertTypeNamedSpecifiers); } } else { @@ -797,6 +946,9 @@ export default util.createRule({ } } + // we have some valueSpecifiers intermixed in types that need to be put on their own line + // import type { Type1, A } from 'foo' + // import type { A } from 'foo' const valueNamedSpecifiers = namedSpecifiers.filter(specifier => report.valueSpecifiers.includes(specifier), ); @@ -810,6 +962,7 @@ export default util.createRule({ const afterFixes: TSESLint.RuleFix[] = []; if (valueNamedSpecifiers.length) { if (sourceImports.valueOnlyNamedImport) { + // TODO: The only use we have of this is for `isTypeImport` block. Do we need this? const insertTypeNamedSpecifiers = fixInsertNamedSpecifiersInNamedSpecifierList( fixer, @@ -822,6 +975,8 @@ export default util.createRule({ afterFixes.push(insertTypeNamedSpecifiers); } } else { + // some are types. + // Add new value import and later remove those value specifiers from import type yield fixer.insertTextBefore( node, `import {${ @@ -865,6 +1020,8 @@ export default util.createRule({ fixer: TSESLint.RuleFixer, node: TSESTree.ImportSpecifier, ): IterableIterator { + // import { type Foo } from 'foo' + // ^^^^ remove const typeToken = util.nullThrows( sourceCode.getFirstToken(node, isTypeToken), util.NullThrowsReasons.MissingToken('type', node.type), diff --git a/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts index ec78a1afd508..b50c3e50e30c 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-imports.test.ts @@ -118,11 +118,53 @@ ruleTester.run('consistent-type-imports', rule, { `, options: [{ prefer: 'no-type-imports' }], }, + ` + import { type A } from 'foo'; + type T = A; + `, ` import { type A, B } from 'foo'; type T = A; const b = B; `, + ` + import { type A, type B } from 'foo'; + type T = A; + type Z = B; + `, + ` + import { B } from 'foo'; + import { type A } from 'foo'; + type T = A; + const b = B; + `, + { + code: ` + import { B, type A } from 'foo'; + type T = A; + const b = B; + `, + options: [{ fixStyle: 'inline-type-imports' }], + }, + { + code: ` + import { B } from 'foo'; + import type A from 'baz'; + type T = A; + const b = B; + `, + options: [{ fixStyle: 'inline-type-imports' }], + }, + { + code: ` + import { B, type C } from 'foo'; + import type A from 'baz'; + type T = A; + type Z = C; + const b = B; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + }, // exports ` import Type from 'foo'; @@ -512,6 +554,24 @@ export type Y = { }, ], }, + { + code: ` + import Foo from 'foo'; + let foo: Foo; + `, + output: ` + import type Foo from 'foo'; + let foo: Foo; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, { code: ` import { A, B } from 'foo'; @@ -1833,5 +1893,233 @@ const b = B; }, ], }, + + // inline-type-imports + { + code: ` + import { A, B } from 'foo'; + let foo: A; + let bar: B; + `, + output: ` + import type { A, B } from 'foo'; + let foo: A; + let bar: B; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'typeOverValue', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { A, B } from 'foo'; + let foo: A; + B(); + `, + output: ` + import { type A, B } from 'foo'; + let foo: A; + B(); + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { A, B } from 'foo'; + type T = A; + B(); + `, + output: ` + import { type A, B } from 'foo'; + type T = A; + B(); + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { /* comment */ A, B } from 'foo'; + type T = A; + `, + output: ` + import { /* comment */ type A, B } from 'foo'; + type T = A; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 9, + }, + ], + }, + { + code: ` + import { B, /* comment */ A } from 'foo'; + type T = A; + `, + output: ` + import { B, /* comment */ type A } from 'foo'; + type T = A; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 9, + }, + ], + }, + { + code: ` +import { A, B, C } from 'foo'; +import type { D } from 'deez'; +const foo: A = B(); +let bar: C; +let baz: D; + `, + output: ` +import { type A, B, type C } from 'foo'; +import type { D } from 'deez'; +const foo: A = B(); +let bar: C; +let baz: D; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 1, + }, + ], + }, + { + code: ` +import { A, B, type C } from 'foo'; +import type { D } from 'deez'; +const foo: A = B(); +let bar: C; +let baz: D; + `, + output: ` +import { type A, B, type C } from 'foo'; +import type { D } from 'deez'; +const foo: A = B(); +let bar: C; +let baz: D; + `, + options: [{ prefer: 'type-imports', fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 2, + column: 1, + }, + ], + }, + { + code: ` +import { B } from 'foo'; +import type { A } from 'foo'; +const foo: A = B(); + `, + output: ` +import { B, type A } from 'foo'; +const foo: A = B(); + `, + options: [{ fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +import { B } from 'foo'; +import type { A, C } from 'foo'; +const foo: A = B(); +let bar: C; + `, + output: ` +import { B, type A, type C } from 'foo'; +const foo: A = B(); +let bar: C; + `, + options: [{ fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +import { B, type A } from 'foo'; +import type { C } from 'foo'; +const foo: A = B(); +let bar: C; + `, + output: ` +import { B, type A, type C } from 'foo'; +const foo: A = B(); +let bar: C; + `, + options: [{ fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +import { B } from 'foo'; +import type { C as A } from 'foo'; +let bar: A; +let baz = B(); + `, + output: ` +import { B, type C as A } from 'foo'; +let bar: A; +let baz = B(); + `, + options: [{ fixStyle: 'inline-type-imports' }], + errors: [ + { + messageId: 'inlineTypes', + line: 3, + column: 1, + }, + ], + }, ], });