From ecbc29fad26a55ba77385e029e10010c92184f2f Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Mon, 29 Nov 2021 16:13:47 -0800 Subject: [PATCH] feat(eslint-plugin): [consistent-type-exports] support TS4.5 inline export specifiers --- package.json | 4 +- .../docs/rules/consistent-type-exports.md | 49 +++++- .../src/rules/consistent-type-exports.ts | 161 ++++++++++++------ .../rules/consistent-type-exports.test.ts | 148 +++++++++++++++- yarn.lock | 10 +- 5 files changed, 309 insertions(+), 63 deletions(-) diff --git a/package.json b/package.json index fe8d5e25da31..7c532099e9bb 100644 --- a/package.json +++ b/package.json @@ -76,7 +76,7 @@ "@types/marked": "^3.0.2", "@types/ncp": "^2.0.5", "@types/node": "^16.11.4", - "@types/prettier": "^2.3.2", + "@types/prettier": "^2.4.2", "@types/rimraf": "^3.0.2", "@types/semver": "^7.3.9", "@types/tmp": "^0.2.2", @@ -102,7 +102,7 @@ "markdownlint-cli": "^0.29.0", "ncp": "^2.0.0", "node-fetch": "^3.0.0", - "prettier": "2.4.1", + "prettier": "^2.5.0", "pretty-format": "^27.3.1", "rimraf": "^3.0.2", "tmp": "^0.2.1", diff --git a/packages/eslint-plugin/docs/rules/consistent-type-exports.md b/packages/eslint-plugin/docs/rules/consistent-type-exports.md index 71303d3659e8..3c02e38d29c9 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 98e6bd355e4c..7ad52679226d 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 301d70e54dc3..561660a7f456 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, + }, + ], + }, ], }); diff --git a/yarn.lock b/yarn.lock index cf83b08f8e63..889f67f81324 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3770,7 +3770,7 @@ resolved "https://registry.yarnpkg.com/@types/parse5/-/parse5-5.0.3.tgz#e7b5aebbac150f8b5fdd4a46e7f0bd8e65e19109" integrity sha512-kUNnecmtkunAoQ3CnjmMkzNU/gtxG8guhi+Fk2U/kOpIKjIMKnXGp4IJCgQJrXSgMsWYimYG4TGjz/UzbGEBTw== -"@types/prettier@*", "@types/prettier@^2.1.5", "@types/prettier@^2.3.2": +"@types/prettier@*", "@types/prettier@^2.1.5", "@types/prettier@^2.4.2": version "2.4.2" resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.4.2.tgz#4c62fae93eb479660c3bd93f9d24d561597a8281" integrity sha512-ekoj4qOQYp7CvjX8ZDBgN86w3MqQhLE1hczEJbEIjgFEumDy+na/4AJAbLXfgEWFNB2pKadM5rPFtuSGMWK7xA== @@ -11706,10 +11706,10 @@ prepend-http@^2.0.0: resolved "https://registry.yarnpkg.com/prepend-http/-/prepend-http-2.0.0.tgz#e92434bfa5ea8c19f41cdfd401d741a3c819d897" integrity sha1-6SQ0v6XqjBn0HN/UAddBo8gZ2Jc= -prettier@*, prettier@2.4.1: - version "2.4.1" - resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.4.1.tgz#671e11c89c14a4cfc876ce564106c4a6726c9f5c" - integrity sha512-9fbDAXSBcc6Bs1mZrDYb3XKzDLm4EXXL9sC1LqKP5rZkT6KRr/rf9amVUcODVXgguK/isJz0d0hP72WeaKWsvA== +prettier@*, prettier@^2.5.0: + version "2.5.0" + resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.5.0.tgz#a6370e2d4594e093270419d9cc47f7670488f893" + integrity sha512-FM/zAKgWTxj40rH03VxzIPdXmj39SwSjwG0heUcNFwI+EMZJnY93yAiKXM3dObIKAM5TA88werc8T/EwhB45eg== pretty-error@^4.0.0: version "4.0.0"