From be4d976215614cc032730ae596d2f6e47df67730 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 6 Dec 2021 10:04:51 -0800 Subject: [PATCH] feat(eslint-plugin): [consistent-type-exports] support TS4.5 inline export specifiers (#4236) --- .../docs/rules/consistent-type-exports.md | 49 +++++- .../src/rules/consistent-type-exports.ts | 161 ++++++++++++------ .../rules/consistent-type-exports.test.ts | 148 +++++++++++++++- 3 files changed, 302 insertions(+), 56 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/consistent-type-exports.md b/packages/eslint-plugin/docs/rules/consistent-type-exports.md index 71303d3659e..3c02e38d29c 100644 --- a/packages/eslint-plugin/docs/rules/consistent-type-exports.md +++ b/packages/eslint-plugin/docs/rules/consistent-type-exports.md @@ -11,6 +11,51 @@ This rule aims to standardize the use of type exports style across a codebase. Given a class `Button`, and an interface `ButtonProps`, examples of code: +## Options + +```ts +interface Options { + fixMixedExportsWithInlineTypeSpecifier?: boolean; +} + +const defaultOptions: Options = { + fixMixedExportsWithInlineTypeSpecifier: false, +}; +``` + +### `fixMixedExportsWithInlineTypeSpecifier` + +When this is set to true, the rule will autofix "mixed" export cases using TS 4.5's "inline type specifier". +If you are using a TypeScript version less than 4.5, then you will not be able to use this option. + +For example the following code: + +```ts +const x = 1; +type T = number; + +export { x, T }; +``` + +With `{fixMixedExportsWithInlineTypeSpecifier: true}` will be fixed to: + +```ts +const x = 1; +type T = number; + +export { x, type T }; +``` + +With `{fixMixedExportsWithInlineTypeSpecifier: false}` will be fixed to: + +```ts +const x = 1; +type T = number; + +export type { T }; +export { x }; +``` + ### ❌ Incorrect @@ -23,7 +68,9 @@ export type { ButtonProps } from 'some-library'; ### ✅ Correct ```ts -export { Button, ButtonProps } from 'some-library'; +export { Button } from 'some-library'; +export type { ButtonProps } from 'some-library'; +export { Button, type ButtonProps } from 'some-library'; ``` ## When Not To Use It diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 98e6bd355e4..7ad52679226 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -7,7 +7,11 @@ import { import { SymbolFlags } from 'typescript'; import * as util from '../util'; -type Options = []; +type Options = [ + { + fixMixedExportsWithInlineTypeSpecifier: boolean; + }, +]; interface SourceExports { source: string; @@ -18,8 +22,9 @@ interface SourceExports { interface ReportValueExport { node: TSESTree.ExportNamedDeclaration; - typeSpecifiers: TSESTree.ExportSpecifier[]; + typeBasedSpecifiers: TSESTree.ExportSpecifier[]; valueSpecifiers: TSESTree.ExportSpecifier[]; + inlineTypeSpecifiers: TSESTree.ExportSpecifier[]; } type MessageIds = @@ -29,7 +34,6 @@ type MessageIds = export default util.createRule({ name: 'consistent-type-exports', - defaultOptions: [], meta: { type: 'suggestion', docs: { @@ -46,11 +50,26 @@ export default util.createRule({ multipleExportsAreTypes: 'Type exports {{exportNames}} are not values and should be exported using `export type`.', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + fixMixedExportsWithInlineTypeSpecifier: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], fixable: 'code', }, + defaultOptions: [ + { + fixMixedExportsWithInlineTypeSpecifier: false, + }, + ], - create(context) { + create(context, [{ fixMixedExportsWithInlineTypeSpecifier }]) { const sourceCode = context.getSourceCode(); const sourceExportsMap: { [key: string]: SourceExports } = {}; const parserServices = util.getParserServices(context); @@ -69,27 +88,33 @@ export default util.createRule({ // Cache the first encountered exports for the package. We will need to come // back to these later when fixing the problems. if (node.exportKind === 'type') { - if (!sourceExports.typeOnlyNamedExport) { + if (sourceExports.typeOnlyNamedExport == null) { // The export is a type export sourceExports.typeOnlyNamedExport = node; } - } else if (!sourceExports.valueOnlyNamedExport) { + } else if (sourceExports.valueOnlyNamedExport == null) { // The export is a value export sourceExports.valueOnlyNamedExport = node; } // Next for the current export, we will separate type/value specifiers. - const typeSpecifiers: TSESTree.ExportSpecifier[] = []; + const typeBasedSpecifiers: TSESTree.ExportSpecifier[] = []; + const inlineTypeSpecifiers: TSESTree.ExportSpecifier[] = []; const valueSpecifiers: TSESTree.ExportSpecifier[] = []; // Note: it is valid to export values as types. We will avoid reporting errors // when this is encountered. if (node.exportKind !== 'type') { for (const specifier of node.specifiers) { + if (specifier.exportKind === 'type') { + inlineTypeSpecifiers.push(specifier); + continue; + } + const isTypeBased = isSpecifierTypeBased(parserServices, specifier); if (isTypeBased === true) { - typeSpecifiers.push(specifier); + typeBasedSpecifiers.push(specifier); } else if (isTypeBased === false) { // When isTypeBased is undefined, we should avoid reporting them. valueSpecifiers.push(specifier); @@ -98,13 +123,14 @@ export default util.createRule({ } if ( - (node.exportKind === 'value' && typeSpecifiers.length) || + (node.exportKind === 'value' && typeBasedSpecifiers.length) || (node.exportKind === 'type' && valueSpecifiers.length) ) { sourceExports.reportValueExports.push({ node, - typeSpecifiers, + typeBasedSpecifiers, valueSpecifiers, + inlineTypeSpecifiers, }); } }, @@ -117,8 +143,8 @@ export default util.createRule({ } for (const report of sourceExports.reportValueExports) { - if (!report.valueSpecifiers.length) { - // Export is all type-only; convert the entire export to `export type`. + if (report.valueSpecifiers.length === 0) { + // Export is all type-only with no type specifiers; convert the entire export to `export type`. context.report({ node: report.node, messageId: 'typeOverValue', @@ -126,35 +152,44 @@ export default util.createRule({ yield* fixExportInsertType(fixer, sourceCode, report.node); }, }); - } else { - // We have both type and value violations. - const allExportNames = report.typeSpecifiers.map( - specifier => `${specifier.local.name}`, - ); - - if (allExportNames.length === 1) { - const exportNames = allExportNames[0]; - - context.report({ - node: report.node, - messageId: 'singleExportIsType', - data: { exportNames }, - *fix(fixer) { + continue; + } + + // We have both type and value violations. + const allExportNames = report.typeBasedSpecifiers.map( + specifier => `${specifier.local.name}`, + ); + + if (allExportNames.length === 1) { + const exportNames = allExportNames[0]; + + context.report({ + node: report.node, + messageId: 'singleExportIsType', + data: { exportNames }, + *fix(fixer) { + if (fixMixedExportsWithInlineTypeSpecifier) { + yield* fixAddTypeSpecifierToNamedExports(fixer, report); + } else { yield* fixSeparateNamedExports(fixer, sourceCode, report); - }, - }); - } else { - const exportNames = util.formatWordList(allExportNames); - - context.report({ - node: report.node, - messageId: 'multipleExportsAreTypes', - data: { exportNames }, - *fix(fixer) { + } + }, + }); + } else { + const exportNames = util.formatWordList(allExportNames); + + context.report({ + node: report.node, + messageId: 'multipleExportsAreTypes', + data: { exportNames }, + *fix(fixer) { + if (fixMixedExportsWithInlineTypeSpecifier) { + yield* fixAddTypeSpecifierToNamedExports(fixer, report); + } else { yield* fixSeparateNamedExports(fixer, sourceCode, report); - }, - }); - } + } + }, + }); } } } @@ -205,6 +240,23 @@ function* fixExportInsertType( ); yield fixer.insertTextAfter(exportToken, ' type'); + + for (const specifier of node.specifiers) { + if (specifier.exportKind === 'type') { + const kindToken = util.nullThrows( + sourceCode.getFirstToken(specifier), + util.NullThrowsReasons.MissingToken('export', specifier.type), + ); + const firstTokenAfter = util.nullThrows( + sourceCode.getTokenAfter(kindToken, { + includeComments: true, + }), + 'Missing token following the export kind.', + ); + + yield fixer.removeRange([kindToken.range[0], firstTokenAfter.range[0]]); + } + } } /** @@ -217,11 +269,11 @@ function* fixSeparateNamedExports( sourceCode: Readonly, report: ReportValueExport, ): IterableIterator { - const { node, typeSpecifiers, valueSpecifiers } = report; + const { node, typeBasedSpecifiers, inlineTypeSpecifiers, valueSpecifiers } = + report; + const typeSpecifiers = typeBasedSpecifiers.concat(inlineTypeSpecifiers); const source = getSourceFromExport(node); - const separateTypes = node.exportKind !== 'type'; - const specifiersToSeparate = separateTypes ? typeSpecifiers : valueSpecifiers; - const specifierNames = specifiersToSeparate.map(getSpecifierText).join(', '); + const specifierNames = typeSpecifiers.map(getSpecifierText).join(', '); const exportToken = util.nullThrows( sourceCode.getFirstToken(node), @@ -229,9 +281,7 @@ function* fixSeparateNamedExports( ); // Filter the bad exports from the current line. - const filteredSpecifierNames = ( - separateTypes ? valueSpecifiers : typeSpecifiers - ) + const filteredSpecifierNames = valueSpecifiers .map(getSpecifierText) .join(', '); const openToken = util.nullThrows( @@ -252,12 +302,23 @@ function* fixSeparateNamedExports( // Insert the bad exports into a new export line above. yield fixer.insertTextBefore( exportToken, - `export ${separateTypes ? 'type ' : ''}{ ${specifierNames} }${ - source ? ` from '${source}'` : '' - };\n`, + `export type { ${specifierNames} }${source ? ` from '${source}'` : ''};\n`, ); } +function* fixAddTypeSpecifierToNamedExports( + fixer: TSESLint.RuleFixer, + report: ReportValueExport, +): IterableIterator { + if (report.node.exportKind === 'type') { + return; + } + + for (const specifier of report.typeBasedSpecifiers) { + yield fixer.insertTextBefore(specifier, 'type '); + } +} + /** * Returns the source of the export, or undefined if the named export has no source. */ diff --git a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts index 301d70e54dc..561660a7f45 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -1,5 +1,5 @@ import rule from '../../src/rules/consistent-type-exports'; -import { RuleTester, getFixturesRootDir } from '../RuleTester'; +import { RuleTester, getFixturesRootDir, noFormat } from '../RuleTester'; const rootDir = getFixturesRootDir(); @@ -106,8 +106,7 @@ export { CatchScope, } from '@typescript-eslint/scope-manager'; `, - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - output: ` + output: noFormat` export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; `, @@ -158,8 +157,7 @@ export { CatchScope as CScope, } from '@typescript-eslint/scope-manager'; `, - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - output: ` + output: noFormat` export type { Definition as Foo } from '@typescript-eslint/scope-manager'; export { BlockScope as BScope, CatchScope as CScope } from '@typescript-eslint/scope-manager'; `, @@ -259,5 +257,145 @@ export type { TypeNS }; }, ], }, + { + code: ` +type T = 1; +export { type T, T }; + `, + output: ` +type T = 1; +export type { T, T }; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 3, + column: 1, + }, + ], + }, + { + code: noFormat` +type T = 1; +export { type/* */T, type /* */T, T }; + `, + output: noFormat` +type T = 1; +export type { /* */T, /* */T, T }; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +type T = 1; +const x = 1; +export { type T, T, x }; + `, + output: ` +type T = 1; +const x = 1; +export type { T, T }; +export { x }; + `, + errors: [ + { + messageId: 'singleExportIsType', + line: 4, + column: 1, + }, + ], + }, + { + code: ` +type T = 1; +const x = 1; +export { T, x }; + `, + output: ` +type T = 1; +const x = 1; +export { type T, x }; + `, + options: [{ fixMixedExportsWithInlineTypeSpecifier: true }], + errors: [ + { + messageId: 'singleExportIsType', + line: 4, + column: 1, + }, + ], + }, + { + code: ` +type T = 1; +export { type T, T }; + `, + output: ` +type T = 1; +export type { T, T }; + `, + options: [{ fixMixedExportsWithInlineTypeSpecifier: true }], + errors: [ + { + messageId: 'typeOverValue', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +export { + AnalyzeOptions, + Definition as Foo, + type BlockScope as BScope, + CatchScope as CScope, +} from '@typescript-eslint/scope-manager'; + `, + output: noFormat` +export type { AnalyzeOptions, Definition as Foo, BlockScope as BScope } from '@typescript-eslint/scope-manager'; +export { CatchScope as CScope } from '@typescript-eslint/scope-manager'; + `, + options: [{ fixMixedExportsWithInlineTypeSpecifier: false }], + errors: [ + { + messageId: 'multipleExportsAreTypes', + line: 2, + column: 1, + }, + ], + }, + { + code: ` +export { + AnalyzeOptions, + Definition as Foo, + type BlockScope as BScope, + CatchScope as CScope, +} from '@typescript-eslint/scope-manager'; + `, + output: noFormat` +export { + type AnalyzeOptions, + type Definition as Foo, + type BlockScope as BScope, + CatchScope as CScope, +} from '@typescript-eslint/scope-manager'; + `, + options: [{ fixMixedExportsWithInlineTypeSpecifier: true }], + errors: [ + { + messageId: 'multipleExportsAreTypes', + line: 2, + column: 1, + }, + ], + }, ], });