From 4760ea5f81ac5d9d062a5a9646012594471eea72 Mon Sep 17 00:00:00 2001 From: dzearing Date: Thu, 23 Sep 2021 12:51:25 -0700 Subject: [PATCH 01/16] initial seed of rule. --- .../src/rules/consistent-type-exports.ts | 864 ++++++++++++++++++ packages/eslint-plugin/src/rules/index.ts | 2 + 2 files changed, 866 insertions(+) create mode 100644 packages/eslint-plugin/src/rules/consistent-type-exports.ts diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts new file mode 100644 index 00000000000..9b68525bfe4 --- /dev/null +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -0,0 +1,864 @@ +import { + // TSESLint, + TSESTree, + // AST_TOKEN_TYPES, + AST_NODE_TYPES, +} from '@typescript-eslint/experimental-utils'; +import * as util from '../util'; + +type Prefer = 'type-exports' | 'no-type-exports'; + +type Options = [ + { + prefer?: Prefer; + }, +]; + +interface SourceExports { + source: string; + reportValueExports: ReportValueExport[]; + typeOnlyNamedExport: TSESTree.ExportNamedDeclaration | null; + valueOnlyNamedExport: TSESTree.ExportNamedDeclaration | null; +} + +interface ReportValueExport { + node: TSESTree.ExportNamedDeclaration; + typeSpecifiers: TSESTree.ExportSpecifier[]; // It has at least one element. + valueSpecifiers: TSESTree.ExportSpecifier[]; +} + +// function isExportToken( +// token: TSESTree.Token, +// ): token is TSESTree.KeywordToken & { value: 'export' } { +// return token.type === AST_TOKEN_TYPES.Keyword && token.value === 'export'; +// } + +// function isTypeToken( +// token: TSESTree.Token, +// ): token is TSESTree.IdentifierToken & { value: 'type' } { +// return token.type === AST_TOKEN_TYPES.Identifier && token.value === 'type'; +// } + +type MessageIds = + | 'typeOverValue' + | 'someExportsAreOnlyTypes' + | 'aExportIsOnlyTypes' + | 'valueOverType' + | 'someExportsInDecoMeta' + | 'aExportInDecoMeta'; + +export default util.createRule({ + name: 'consistent-type-exports', + meta: { + type: 'suggestion', + docs: { + description: 'Enforces consistent usage of type exports', + category: 'Stylistic Issues', + recommended: false, + }, + messages: { + typeOverValue: + 'All exports in the declaration are only used as types. Use `export type`.', + someExportsAreOnlyTypes: + 'Exports {{typeExports}} are only used as types.', + aExportIsOnlyTypes: 'Export {{typeExports}} is only used as types.', + someExportsInDecoMeta: + 'Type exports {{typeExports}} are used by decorator metadata.', + aExportInDecoMeta: + 'Type export {{typeExports}} is used by decorator metadata.', + valueOverType: 'Use an `export` instead of an `export type`.', + }, + schema: [ + { + type: 'object', + properties: { + prefer: { + enum: ['type-exports', 'no-type-exports'], + }, + }, + additionalProperties: false, + }, + ], + fixable: 'code', + }, + + defaultOptions: [ + { + prefer: 'type-exports', + }, + ], + + create(context, [option]) { + const prefer = option.prefer ?? 'type-exports'; + // const sourceCode = context.getSourceCode(); + const sourceExportsMap: { [key: string]: SourceExports } = {}; + + return { + ...(prefer === 'type-exports' + ? { + ExportNamedDeclaration(node): void { + // Coerce the node.source.value to a string via asserting. + if ( + node.source?.type !== AST_NODE_TYPES.Literal || + typeof node.source?.value !== 'string' + ) { + return; + } + + const source = node.source.value; + const sourceExports = (sourceExportsMap[source] = + sourceExportsMap[source] || { + source, + reportValueExports: [], + typeOnlyNamedExport: null, + valueOnlyNamedExport: null, + }); + + // 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) { + // The export is a type export + sourceExports.typeOnlyNamedExport = node; + } + } else if (!sourceExports.valueOnlyNamedExport) { + // The export is a value export + sourceExports.valueOnlyNamedExport = node; + } + + // Next for the current import, we will separate type/value specifiers. + const typeSpecifiers: TSESTree.ExportSpecifier[] = []; + const valueSpecifiers: TSESTree.ExportSpecifier[] = []; + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + + // Helper for identifying specifiers. + function isSpecifierTypeBased( + specifier: TSESTree.ExportSpecifier, + ): boolean | undefined { + const exportName = specifier.local.name; + const originalNode = + parserServices.esTreeNodeToTSNodeMap.get(specifier); + const specifierType = checker.getTypeAtLocation(originalNode); + + console.log('declaration', exportName, specifierType); + const children = + specifierType.getSymbol()?.declarations?.[0]?.getChildren() ?? + []; + let exportType: string | undefined; + + // Iterate through children of the declaration until we find the type before the symbolic name + for (const child of children) { + console.log('child', exportName, child.getText()); + if (child.getText() === exportName) { + break; + } + exportType = child.getText(); + } + + console.log(`Found specifier ${exportName} ${exportType}`); + + return exportType === 'interface' || exportType === 'type'; + } + + for (const specifier of node.specifiers) { + console.log('specifier', specifier.local.name); + const isTypeBased = isSpecifierTypeBased(specifier); + + if (isTypeBased === true) { + typeSpecifiers.push(specifier); + } else if (isTypeBased === false) { + // undefined means we don't know. + valueSpecifiers.push(specifier); + } + } + + if ( + (node.exportKind === 'value' && typeSpecifiers.length) || + (node.exportKind === 'type' && valueSpecifiers.length) + ) { + sourceExports.reportValueExports.push({ + node, + typeSpecifiers, + valueSpecifiers, + }); + } + }, + + 'Program:exit'(): void { + for (const sourceExports of Object.values(sourceExportsMap)) { + console.log( + 'sourceExports', + sourceExports.source, + sourceExports, + ); + + // If this export has no issues, move on. + if (sourceExports.reportValueExports.length === 0) { + continue; + } + + for (const report of sourceExports.reportValueExports) { + console.log('REPORT', report); + + if (!report.valueSpecifiers.length) { + // export is all type-only; convert the entire export to `export type` + context.report({ + node: report.node, + messageId: 'typeOverValue', + // *fix(fixer) { + // yield* fixToTypeExport(fixer, report, sourceExports); + // }, + }); + } else if (!report.typeSpecifiers.length) { + // we only have value violations. + } else { + // We have both type and value violations. + const isTypeExport = (report.node.exportKind = 'type'); + const exportNames = ( + isTypeExport + ? report.valueSpecifiers + : report.typeSpecifiers + ).map(specifier => `${specifier.local.name}`); + + const message = ((): { + messageId: MessageIds; + data: Record; + } => { + if (exportNames.length === 1) { + const typeExports = exportNames[0]; + if (isTypeExport) { + return { + messageId: 'aExportInDecoMeta', + data: { typeExports }, + }; + } else { + return { + messageId: 'aExportIsOnlyTypes', + data: { typeExports }, + }; + } + } else { + const typeExports = [ + exportNames.slice(0, -1).join(', '), + exportNames.slice(-1)[0], + ].join(' and '); + + if (isTypeExport) { + return { + messageId: 'someExportsInDecoMeta', + data: { typeExports }, + }; + } else { + return { + messageId: 'someExportsAreOnlyTypes', + data: { typeExports }, + }; + } + } + })(); + + context.report({ + node: report.node, + ...message, + // *fix(fixer) { + // if (isTypeExport) { + // yield* fixToValueImportInDecoMeta( + // fixer, + // report, + // sourceImports, + // ); + // } else { + // yield* fixToTypeImport(fixer, report, sourceImports); + // } + // }, + }); + } + } + } + }, + + // 'Program:exit'(): void { + // for (const sourceImports of Object.values(sourceExportsMap)) { + // if (sourceImports.reportValueImports.length === 0) { + // continue; + // } + // for (const report of sourceImports.reportValueImports) { + // if ( + // report.valueSpecifiers.length === 0 && + // report.unusedSpecifiers.length === 0 + // ) { + // // import is all type-only, convert the entire import to `import type` + // context.report({ + // node: report.node, + // messageId: 'typeOverValue', + // *fix(fixer) { + // yield* fixToTypeImport(fixer, report, sourceImports); + // }, + // }); + // } else { + // const isTypeImport = report.node.importKind === 'type'; + + // // we have a mixed type/value import, so we need to split them out into multiple imports + // const importNames = ( + // isTypeImport + // ? report.valueSpecifiers + // : report.typeSpecifiers + // ).map(specifier => `"${specifier.local.name}"`); + + // const message = ((): { + // messageId: MessageIds; + // data: Record; + // } => { + // if (importNames.length === 1) { + // const typeImports = importNames[0]; + // if (isTypeImport) { + // return { + // messageId: 'aImportInDecoMeta', + // data: { typeImports }, + // }; + // } else { + // return { + // messageId: 'aImportIsOnlyTypes', + // data: { typeImports }, + // }; + // } + // } else { + // const typeImports = [ + // importNames.slice(0, -1).join(', '), + // importNames.slice(-1)[0], + // ].join(' and '); + // if (isTypeImport) { + // return { + // messageId: 'someImportsInDecoMeta', + // data: { typeImports }, + // }; + // } else { + // return { + // messageId: 'someImportsAreOnlyTypes', + // data: { typeImports }, + // }; + // } + // } + // })(); + + // context.report({ + // node: report.node, + // ...message, + // *fix(fixer) { + // if (isTypeImport) { + // yield* fixToValueImportInDecoMeta( + // fixer, + // report, + // sourceImports, + // ); + // } else { + // yield* fixToTypeImport(fixer, report, sourceImports); + // } + // }, + // }); + // } + // } + // } + // }, + } + : { + // prefer no type exports + // 'ExportDeclaration[importKind = "type"]'( + // node: TSESTree.ExportDeclaration, + // ): void { + // context.report({ + // node, + // messageId: 'valueOverType', + // fix(fixer) { + // return fixToValueExport(fixer, node); + // }, + // }); + // }, + }), + }; + + // function classifySpecifier(node: TSESTree.ImportDeclaration): { + // defaultSpecifier: TSESTree.ImportDefaultSpecifier | null; + // namespaceSpecifier: TSESTree.ImportNamespaceSpecifier | null; + // namedSpecifiers: TSESTree.ImportSpecifier[]; + // } { + // const defaultSpecifier: TSESTree.ImportDefaultSpecifier | null = + // node.specifiers[0].type === AST_NODE_TYPES.ImportDefaultSpecifier + // ? node.specifiers[0] + // : null; + // const namespaceSpecifier: TSESTree.ImportNamespaceSpecifier | null = + // node.specifiers.find( + // (specifier): specifier is TSESTree.ImportNamespaceSpecifier => + // specifier.type === AST_NODE_TYPES.ImportNamespaceSpecifier, + // ) ?? null; + // const namedSpecifiers: TSESTree.ImportSpecifier[] = + // node.specifiers.filter( + // (specifier): specifier is TSESTree.ImportSpecifier => + // specifier.type === AST_NODE_TYPES.ImportSpecifier, + // ); + // return { + // defaultSpecifier, + // namespaceSpecifier, + // namedSpecifiers, + // }; + // } + + /** + * Returns information for fixing named specifiers. + */ + // function getFixesNamedSpecifiers( + // fixer: TSESLint.RuleFixer, + // node: TSESTree.ImportDeclaration, + // typeNamedSpecifiers: TSESTree.ImportSpecifier[], + // allNamedSpecifiers: TSESTree.ImportSpecifier[], + // ): { + // typeNamedSpecifiersText: string; + // removeTypeNamedSpecifiers: TSESLint.RuleFix[]; + // } { + // if (allNamedSpecifiers.length === 0) { + // return { + // typeNamedSpecifiersText: '', + // removeTypeNamedSpecifiers: [], + // }; + // } + // const typeNamedSpecifiersTexts: string[] = []; + // const removeTypeNamedSpecifiers: TSESLint.RuleFix[] = []; + // if (typeNamedSpecifiers.length === allNamedSpecifiers.length) { + // // e.g. + // // import Foo, {Type1, Type2} from 'foo' + // // import DefType, {Type1, Type2} from 'foo' + // const openingBraceToken = util.nullThrows( + // sourceCode.getTokenBefore( + // typeNamedSpecifiers[0], + // util.isOpeningBraceToken, + // ), + // util.NullThrowsReasons.MissingToken('{', node.type), + // ); + // const commaToken = util.nullThrows( + // sourceCode.getTokenBefore(openingBraceToken, util.isCommaToken), + // util.NullThrowsReasons.MissingToken(',', node.type), + // ); + // const closingBraceToken = util.nullThrows( + // sourceCode.getFirstTokenBetween( + // openingBraceToken, + // node.source, + // util.isClosingBraceToken, + // ), + // util.NullThrowsReasons.MissingToken('}', node.type), + // ); + + // // import DefType, {...} from 'foo' + // // ^^^^^^^ remove + // removeTypeNamedSpecifiers.push( + // fixer.removeRange([commaToken.range[0], closingBraceToken.range[1]]), + // ); + + // typeNamedSpecifiersTexts.push( + // sourceCode.text.slice( + // openingBraceToken.range[1], + // closingBraceToken.range[0], + // ), + // ); + // } else { + // const typeNamedSpecifierGroups: TSESTree.ImportSpecifier[][] = []; + // let group: TSESTree.ImportSpecifier[] = []; + // for (const namedSpecifier of allNamedSpecifiers) { + // if (typeNamedSpecifiers.includes(namedSpecifier)) { + // group.push(namedSpecifier); + // } else if (group.length) { + // typeNamedSpecifierGroups.push(group); + // group = []; + // } + // } + // if (group.length) { + // typeNamedSpecifierGroups.push(group); + // } + // for (const namedSpecifiers of typeNamedSpecifierGroups) { + // const { removeRange, textRange } = getNamedSpecifierRanges( + // namedSpecifiers, + // allNamedSpecifiers, + // ); + // removeTypeNamedSpecifiers.push(fixer.removeRange(removeRange)); + + // typeNamedSpecifiersTexts.push(sourceCode.text.slice(...textRange)); + // } + // } + // return { + // typeNamedSpecifiersText: typeNamedSpecifiersTexts.join(','), + // removeTypeNamedSpecifiers, + // }; + // } + + /** + * Returns ranges for fixing named specifier. + */ + // function getNamedSpecifierRanges( + // namedSpecifierGroup: TSESTree.ImportSpecifier[], + // allNamedSpecifiers: TSESTree.ImportSpecifier[], + // ): { + // textRange: TSESTree.Range; + // removeRange: TSESTree.Range; + // } { + // const first = namedSpecifierGroup[0]; + // const last = namedSpecifierGroup[namedSpecifierGroup.length - 1]; + // const removeRange: TSESTree.Range = [first.range[0], last.range[1]]; + // const textRange: TSESTree.Range = [...removeRange]; + // const before = sourceCode.getTokenBefore(first)!; + // textRange[0] = before.range[1]; + // if (util.isCommaToken(before)) { + // removeRange[0] = before.range[0]; + // } else { + // removeRange[0] = before.range[1]; + // } + + // const isFirst = allNamedSpecifiers[0] === first; + // const isLast = allNamedSpecifiers[allNamedSpecifiers.length - 1] === last; + // const after = sourceCode.getTokenAfter(last)!; + // textRange[1] = after.range[0]; + // if (isFirst || isLast) { + // if (util.isCommaToken(after)) { + // removeRange[1] = after.range[1]; + // } + // } + + // return { + // textRange, + // removeRange, + // }; + // } + + /** + * insert specifiers to named export node. + * e.g. + * import type { Already, Type1, Type2 } from 'foo' + * ^^^^^^^^^^^^^ insert + */ + // function insertToNamedExport( + // fixer: TSESLint.RuleFixer, + // target: TSESTree.ImportDeclaration, + // insertText: string, + // ): TSESLint.RuleFix { + // 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)) { + // insertText = ',' + insertText; + // } + // return fixer.insertTextBefore(closingBraceToken, insertText); + // } + + // function* fixToTypeExport(): IterableIterator { + // fixer: TSESLint.RuleFixer, + // report: ReportValueExport, + // sourceExports: SourceExports, + // const { node } = report; + // const { defaultSpecifier, namespaceSpecifier, namedSpecifiers } = + // classifySpecifier(node); + // if (namespaceSpecifier && !defaultSpecifier) { + // // e.g. + // // import * as types from 'foo' + // yield* fixToTypeExportByInsertType(fixer, node, false); + // return; + // } else if (defaultSpecifier) { + // if ( + // report.typeSpecifiers.includes(defaultSpecifier) && + // namedSpecifiers.length === 0 && + // !namespaceSpecifier + // ) { + // // e.g. + // // import Type from 'foo' + // yield* fixToTypeExportByInsertType(fixer, node, true); + // return; + // } + // } else { + // if ( + // namedSpecifiers.every(specifier => + // report.typeSpecifiers.includes(specifier), + // ) && + // !namespaceSpecifier + // ) { + // // e.g. + // // import {Type1, Type2} from 'foo' + // yield* fixToTypeExportByInsertType(fixer, node, false); + // return; + // } + // } + // const typeNamedSpecifiers = namedSpecifiers.filter(specifier => + // report.typeSpecifiers.includes(specifier), + // ); + // const fixesNamedSpecifiers = getFixesNamedSpecifiers( + // fixer, + // node, + // typeNamedSpecifiers, + // namedSpecifiers, + // ); + // const afterFixes: TSESLint.RuleFix[] = []; + // if (typeNamedSpecifiers.length) { + // if (sourceExports.typeOnlyNamedImport) { + // const insertTypeNamedSpecifiers = insertToNamedImport( + // fixer, + // sourceExports.typeOnlyNamedImport, + // fixesNamedSpecifiers.typeNamedSpecifiersText, + // ); + // if (sourceExports.typeOnlyNamedImport.range[1] <= node.range[0]) { + // yield insertTypeNamedSpecifiers; + // } else { + // afterFixes.push(insertTypeNamedSpecifiers); + // } + // } else { + // yield fixer.insertTextBefore( + // node, + // `import type {${ + // fixesNamedSpecifiers.typeNamedSpecifiersText + // }} from ${sourceCode.getText(node.source)};\n`, + // ); + // } + // } + // const fixesRemoveTypeNamespaceSpecifier: TSESLint.RuleFix[] = []; + // if ( + // namespaceSpecifier && + // report.typeSpecifiers.includes(namespaceSpecifier) + // ) { + // // e.g. + // // import Foo, * as Type from 'foo' + // // import DefType, * as Type from 'foo' + // // import DefType, * as Type from 'foo' + // const commaToken = util.nullThrows( + // sourceCode.getTokenBefore(namespaceSpecifier, util.isCommaToken), + // util.NullThrowsReasons.MissingToken(',', node.type), + // ); + // // import Def, * as Ns from 'foo' + // // ^^^^^^^^^ remove + // fixesRemoveTypeNamespaceSpecifier.push( + // fixer.removeRange([commaToken.range[0], namespaceSpecifier.range[1]]), + // ); + // // import type * as Ns from 'foo' + // // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ insert + // yield fixer.insertTextBefore( + // node, + // `import type ${sourceCode.getText( + // namespaceSpecifier, + // )} from ${sourceCode.getText(node.source)};\n`, + // ); + // } + // if ( + // defaultSpecifier && + // report.typeSpecifiers.includes(defaultSpecifier) + // ) { + // if (report.typeSpecifiers.length === node.specifiers.length) { + // const importToken = util.nullThrows( + // sourceCode.getFirstToken(node, isExportToken), + // util.NullThrowsReasons.MissingToken('import', node.type), + // ); + // // import type Type from 'foo' + // // ^^^^ insert + // yield fixer.insertTextAfter(importToken, ' type'); + // } else { + // const commaToken = util.nullThrows( + // sourceCode.getTokenAfter(defaultSpecifier, util.isCommaToken), + // util.NullThrowsReasons.MissingToken(',', defaultSpecifier.type), + // ); + // // import Type , {...} from 'foo' + // // ^^^^^ pick + // const defaultText = sourceCode.text + // .slice(defaultSpecifier.range[0], commaToken.range[0]) + // .trim(); + // yield fixer.insertTextBefore( + // node, + // `import type ${defaultText} from ${sourceCode.getText( + // node.source, + // )};\n`, + // ); + // const afterToken = util.nullThrows( + // sourceCode.getTokenAfter(commaToken, { includeComments: true }), + // util.NullThrowsReasons.MissingToken('any token', node.type), + // ); + // // import Type , {...} from 'foo' + // // ^^^^^^^ remove + // yield fixer.removeRange([ + // defaultSpecifier.range[0], + // afterToken.range[0], + // ]); + // } + // } + // yield* fixesNamedSpecifiers.removeTypeNamedSpecifiers; + // yield* fixesRemoveTypeNamespaceSpecifier; + // yield* afterFixes; + }, + + /** + * Fixer which inserts "type" into typed exports. + */ + // function* fixToTypeExportByInsertType( + // fixer: TSESLint.RuleFixer, + // node: TSESTree.ExportNamedDeclaration, + // isDefaultImport: boolean, + // ): IterableIterator { + // // export type Foo from 'foo' + // // ^^^^^ insert + // const importToken = util.nullThrows( + // sourceCode.getFirstToken(node, isExportToken), + // util.NullThrowsReasons.MissingToken('import', node.type), + // ); + // yield fixer.insertTextAfter(importToken, ' type'); + + // if (isDefaultImport) { + // // Has default import + // const openingBraceToken = sourceCode.getFirstTokenBetween( + // importToken, + // node.source, + // util.isOpeningBraceToken, + // ); + // if (openingBraceToken) { + // // Only braces. e.g. import Foo, {} from 'foo' + // const commaToken = util.nullThrows( + // sourceCode.getTokenBefore(openingBraceToken, util.isCommaToken), + // util.NullThrowsReasons.MissingToken(',', node.type), + // ); + // const closingBraceToken = util.nullThrows( + // sourceCode.getFirstTokenBetween( + // openingBraceToken, + // node.source, + // util.isClosingBraceToken, + // ), + // util.NullThrowsReasons.MissingToken('}', node.type), + // ); + + // // import type Foo, {} from 'foo' + // // ^^ remove + // yield fixer.removeRange([ + // commaToken.range[0], + // closingBraceToken.range[1], + // ]); + // const specifiersText = sourceCode.text.slice( + // commaToken.range[1], + // closingBraceToken.range[1], + // ); + // if (node.specifiers.length > 1) { + // // import type Foo from 'foo' + // // import type {...} from 'foo' // <- insert + // yield fixer.insertTextAfter( + // node, + // `\nimport type${specifiersText} from ${sourceCode.getText( + // node.source, + // )};`, + // ); + // } + // } + // } + // } + + // function* fixToValueExportInDecoMeta( + // fixer: TSESLint.RuleFixer, + // report: ReportValueExport, + // sourceImports: SourceImports, + // ): IterableIterator { + // const { node } = report; + + // const { defaultSpecifier, namespaceSpecifier, namedSpecifiers } = + // classifySpecifier(node); + + // if (namespaceSpecifier) { + // // e.g. + // // import type * as types from 'foo' + // yield* fixToValueExport(fixer, node); + // return; + // } else if (defaultSpecifier) { + // if ( + // report.valueSpecifiers.includes(defaultSpecifier) && + // namedSpecifiers.length === 0 + // ) { + // // e.g. + // // import type Type from 'foo' + // yield* fixToValueExport(fixer, node); + // return; + // } + // } else { + // if ( + // namedSpecifiers.every(specifier => + // report.valueSpecifiers.includes(specifier), + // ) + // ) { + // // e.g. + // // import type {Type1, Type2} from 'foo' + // yield* fixToValueExport(fixer, node); + // return; + // } + // } + + // const valueNamedSpecifiers = namedSpecifiers.filter(specifier => + // report.valueSpecifiers.includes(specifier), + // ); + + // const fixesNamedSpecifiers = getFixesNamedSpecifiers( + // fixer, + // node, + // valueNamedSpecifiers, + // namedSpecifiers, + // ); + // const afterFixes: TSESLint.RuleFix[] = []; + // if (valueNamedSpecifiers.length) { + // if (sourceImports.valueOnlyNamedImport) { + // const insertTypeNamedSpecifiers = insertToNamedImport( + // fixer, + // sourceImports.valueOnlyNamedImport, + // fixesNamedSpecifiers.typeNamedSpecifiersText, + // ); + // if (sourceImports.valueOnlyNamedImport.range[1] <= node.range[0]) { + // yield insertTypeNamedSpecifiers; + // } else { + // afterFixes.push(insertTypeNamedSpecifiers); + // } + // } else { + // yield fixer.insertTextBefore( + // node, + // `import {${ + // fixesNamedSpecifiers.typeNamedSpecifiersText + // }} from ${sourceCode.getText(node.source)};\n`, + // ); + // } + // } + + // yield* fixesNamedSpecifiers.removeTypeNamedSpecifiers; + + // yield* afterFixes; + // } + + /** + * Remove "type" from an export. + */ + // function* fixToValueExport( + // fixer: TSESLint.RuleFixer, + // node: TSESTree.ExportNamedDeclaration, + // ): IterableIterator { + // // export type { Foo } from 'foo' + // // ^^^^ remove + // const exportToken = util.nullThrows( + // sourceCode.getFirstToken(node, isExportToken), + // util.NullThrowsReasons.MissingToken('export', node.type), + // ); + + // const typeToken = util.nullThrows( + // sourceCode.getFirstTokenBetween( + // exportToken, + // node.specifiers[0]?.local ?? node.source, + // isTypeToken, + // ), + // util.NullThrowsReasons.MissingToken('type', node.type), + // ); + + // const afterToken = util.nullThrows( + // sourceCode.getTokenAfter(typeToken, { includeComments: true }), + // util.NullThrowsReasons.MissingToken('any token', node.type), + // ); + // yield fixer.removeRange([typeToken.range[0], afterToken.range[0]]); + // } + // }, +}); diff --git a/packages/eslint-plugin/src/rules/index.ts b/packages/eslint-plugin/src/rules/index.ts index 3ab88b478e2..7bc8652ec30 100644 --- a/packages/eslint-plugin/src/rules/index.ts +++ b/packages/eslint-plugin/src/rules/index.ts @@ -11,6 +11,7 @@ import commaSpacing from './comma-spacing'; import consistentIndexedObjectStyle from './consistent-indexed-object-style'; import consistentTypeAssertions from './consistent-type-assertions'; import consistentTypeDefinitions from './consistent-type-definitions'; +import consistentTypeExports from './consistent-type-exports'; import consistentTypeImports from './consistent-type-imports'; import defaultParamLast from './default-param-last'; import dotNotation from './dot-notation'; @@ -134,6 +135,7 @@ export default { 'consistent-indexed-object-style': consistentIndexedObjectStyle, 'consistent-type-assertions': consistentTypeAssertions, 'consistent-type-definitions': consistentTypeDefinitions, + 'consistent-type-exports': consistentTypeExports, 'consistent-type-imports': consistentTypeImports, 'default-param-last': defaultParamLast, 'dot-notation': dotNotation, From 815efaec49ccf98b30f1c32e37c9de3bc6dd2547 Mon Sep 17 00:00:00 2001 From: dzearing Date: Fri, 24 Sep 2021 22:59:15 -0700 Subject: [PATCH 02/16] Getting closer! --- .../src/rules/consistent-type-exports.ts | 763 +++--------------- 1 file changed, 90 insertions(+), 673 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 9b68525bfe4..e1e5a178b23 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -1,9 +1,11 @@ +/* eslint-disable no-console */ import { - // TSESLint, TSESTree, - // AST_TOKEN_TYPES, + ParserServices, AST_NODE_TYPES, + TSESLint, } from '@typescript-eslint/experimental-utils'; +import { SymbolFlags } from 'typescript'; import * as util from '../util'; type Prefer = 'type-exports' | 'no-type-exports'; @@ -27,25 +29,13 @@ interface ReportValueExport { valueSpecifiers: TSESTree.ExportSpecifier[]; } -// function isExportToken( -// token: TSESTree.Token, -// ): token is TSESTree.KeywordToken & { value: 'export' } { -// return token.type === AST_TOKEN_TYPES.Keyword && token.value === 'export'; -// } - -// function isTypeToken( -// token: TSESTree.Token, -// ): token is TSESTree.IdentifierToken & { value: 'type' } { -// return token.type === AST_TOKEN_TYPES.Identifier && token.value === 'type'; -// } - type MessageIds = | 'typeOverValue' - | 'someExportsAreOnlyTypes' - | 'aExportIsOnlyTypes' | 'valueOverType' - | 'someExportsInDecoMeta' - | 'aExportInDecoMeta'; + | 'singleExportIsType' + | 'multipleExportsAreTypes' + | 'singleExportisValue' + | 'multipleExportsAreValues'; export default util.createRule({ name: 'consistent-type-exports', @@ -59,14 +49,14 @@ export default util.createRule({ messages: { typeOverValue: 'All exports in the declaration are only used as types. Use `export type`.', - someExportsAreOnlyTypes: - 'Exports {{typeExports}} are only used as types.', - aExportIsOnlyTypes: 'Export {{typeExports}} is only used as types.', - someExportsInDecoMeta: - 'Type exports {{typeExports}} are used by decorator metadata.', - aExportInDecoMeta: - 'Type export {{typeExports}} is used by decorator metadata.', valueOverType: 'Use an `export` instead of an `export type`.', + singleExportIsType: 'Export {{typeExports}} is only used as types.', + multipleExportsAreTypes: + 'Exports {{typeExports}} should be exported using `export type`.', + singleExportisValue: + 'Type export {{typeExports}} is not a type and should be exported using `export`.', + multipleExportsAreValues: + 'Exports {{typeExports}} are not types and should be exported using `export`.', }, schema: [ { @@ -89,6 +79,7 @@ export default util.createRule({ ], create(context, [option]) { + const sourceCode = context.getSourceCode(); const prefer = option.prefer ?? 'type-exports'; // const sourceCode = context.getSourceCode(); const sourceExportsMap: { [key: string]: SourceExports } = {}; @@ -96,7 +87,9 @@ export default util.createRule({ return { ...(prefer === 'type-exports' ? { - ExportNamedDeclaration(node): void { + ExportNamedDeclaration( + node: TSESTree.ExportNamedDeclaration, + ): void { // Coerce the node.source.value to a string via asserting. if ( node.source?.type !== AST_NODE_TYPES.Literal || @@ -113,6 +106,7 @@ export default util.createRule({ typeOnlyNamedExport: null, valueOnlyNamedExport: null, }); + const parserServices = util.getParserServices(context); // Cache the first encountered exports for the package. We will need to come // back to these later when fixing the problems. @@ -129,41 +123,14 @@ export default util.createRule({ // Next for the current import, we will separate type/value specifiers. const typeSpecifiers: TSESTree.ExportSpecifier[] = []; const valueSpecifiers: TSESTree.ExportSpecifier[] = []; - const parserServices = util.getParserServices(context); - const checker = parserServices.program.getTypeChecker(); - - // Helper for identifying specifiers. - function isSpecifierTypeBased( - specifier: TSESTree.ExportSpecifier, - ): boolean | undefined { - const exportName = specifier.local.name; - const originalNode = - parserServices.esTreeNodeToTSNodeMap.get(specifier); - const specifierType = checker.getTypeAtLocation(originalNode); - - console.log('declaration', exportName, specifierType); - const children = - specifierType.getSymbol()?.declarations?.[0]?.getChildren() ?? - []; - let exportType: string | undefined; - - // Iterate through children of the declaration until we find the type before the symbolic name - for (const child of children) { - console.log('child', exportName, child.getText()); - if (child.getText() === exportName) { - break; - } - exportType = child.getText(); - } - - console.log(`Found specifier ${exportName} ${exportType}`); - - return exportType === 'interface' || exportType === 'type'; - } for (const specifier of node.specifiers) { - console.log('specifier', specifier.local.name); - const isTypeBased = isSpecifierTypeBased(specifier); + const isTypeBased = isSpecifierTypeBased( + parserServices, + specifier, + ); + + console.log('specifier', specifier.local.name, isTypeBased); if (isTypeBased === true) { typeSpecifiers.push(specifier); @@ -187,31 +154,27 @@ export default util.createRule({ 'Program:exit'(): void { for (const sourceExports of Object.values(sourceExportsMap)) { - console.log( - 'sourceExports', - sourceExports.source, - sourceExports, - ); - // If this export has no issues, move on. if (sourceExports.reportValueExports.length === 0) { continue; } for (const report of sourceExports.reportValueExports) { - console.log('REPORT', report); - if (!report.valueSpecifiers.length) { // export is all type-only; convert the entire export to `export type` context.report({ node: report.node, messageId: 'typeOverValue', - // *fix(fixer) { - // yield* fixToTypeExport(fixer, report, sourceExports); - // }, + *fix(fixer) { + yield* fixToTypeExportByInsertType( + fixer, + sourceCode, + report.node, + ); + }, }); } else if (!report.typeSpecifiers.length) { - // we only have value violations. + // we only have value violations; remove the `type` from the export } else { // We have both type and value violations. const isTypeExport = (report.node.exportKind = 'type'); @@ -227,34 +190,24 @@ export default util.createRule({ } => { if (exportNames.length === 1) { const typeExports = exportNames[0]; - if (isTypeExport) { - return { - messageId: 'aExportInDecoMeta', - data: { typeExports }, - }; - } else { - return { - messageId: 'aExportIsOnlyTypes', - data: { typeExports }, - }; - } + return { + messageId: isTypeExport + ? 'singleExportisValue' + : 'singleExportIsType', + data: { typeExports }, + }; } else { const typeExports = [ exportNames.slice(0, -1).join(', '), exportNames.slice(-1)[0], ].join(' and '); - if (isTypeExport) { - return { - messageId: 'someExportsInDecoMeta', - data: { typeExports }, - }; - } else { - return { - messageId: 'someExportsAreOnlyTypes', - data: { typeExports }, - }; - } + return { + messageId: isTypeExport + ? 'multipleExportsAreValues' + : 'multipleExportsAreTypes', + data: { typeExports }, + }; } })(); @@ -277,588 +230,52 @@ export default util.createRule({ } } }, - - // 'Program:exit'(): void { - // for (const sourceImports of Object.values(sourceExportsMap)) { - // if (sourceImports.reportValueImports.length === 0) { - // continue; - // } - // for (const report of sourceImports.reportValueImports) { - // if ( - // report.valueSpecifiers.length === 0 && - // report.unusedSpecifiers.length === 0 - // ) { - // // import is all type-only, convert the entire import to `import type` - // context.report({ - // node: report.node, - // messageId: 'typeOverValue', - // *fix(fixer) { - // yield* fixToTypeImport(fixer, report, sourceImports); - // }, - // }); - // } else { - // const isTypeImport = report.node.importKind === 'type'; - - // // we have a mixed type/value import, so we need to split them out into multiple imports - // const importNames = ( - // isTypeImport - // ? report.valueSpecifiers - // : report.typeSpecifiers - // ).map(specifier => `"${specifier.local.name}"`); - - // const message = ((): { - // messageId: MessageIds; - // data: Record; - // } => { - // if (importNames.length === 1) { - // const typeImports = importNames[0]; - // if (isTypeImport) { - // return { - // messageId: 'aImportInDecoMeta', - // data: { typeImports }, - // }; - // } else { - // return { - // messageId: 'aImportIsOnlyTypes', - // data: { typeImports }, - // }; - // } - // } else { - // const typeImports = [ - // importNames.slice(0, -1).join(', '), - // importNames.slice(-1)[0], - // ].join(' and '); - // if (isTypeImport) { - // return { - // messageId: 'someImportsInDecoMeta', - // data: { typeImports }, - // }; - // } else { - // return { - // messageId: 'someImportsAreOnlyTypes', - // data: { typeImports }, - // }; - // } - // } - // })(); - - // context.report({ - // node: report.node, - // ...message, - // *fix(fixer) { - // if (isTypeImport) { - // yield* fixToValueImportInDecoMeta( - // fixer, - // report, - // sourceImports, - // ); - // } else { - // yield* fixToTypeImport(fixer, report, sourceImports); - // } - // }, - // }); - // } - // } - // } - // }, } - : { - // prefer no type exports - // 'ExportDeclaration[importKind = "type"]'( - // node: TSESTree.ExportDeclaration, - // ): void { - // context.report({ - // node, - // messageId: 'valueOverType', - // fix(fixer) { - // return fixToValueExport(fixer, node); - // }, - // }); - // }, - }), + : {}), }; - - // function classifySpecifier(node: TSESTree.ImportDeclaration): { - // defaultSpecifier: TSESTree.ImportDefaultSpecifier | null; - // namespaceSpecifier: TSESTree.ImportNamespaceSpecifier | null; - // namedSpecifiers: TSESTree.ImportSpecifier[]; - // } { - // const defaultSpecifier: TSESTree.ImportDefaultSpecifier | null = - // node.specifiers[0].type === AST_NODE_TYPES.ImportDefaultSpecifier - // ? node.specifiers[0] - // : null; - // const namespaceSpecifier: TSESTree.ImportNamespaceSpecifier | null = - // node.specifiers.find( - // (specifier): specifier is TSESTree.ImportNamespaceSpecifier => - // specifier.type === AST_NODE_TYPES.ImportNamespaceSpecifier, - // ) ?? null; - // const namedSpecifiers: TSESTree.ImportSpecifier[] = - // node.specifiers.filter( - // (specifier): specifier is TSESTree.ImportSpecifier => - // specifier.type === AST_NODE_TYPES.ImportSpecifier, - // ); - // return { - // defaultSpecifier, - // namespaceSpecifier, - // namedSpecifiers, - // }; - // } - - /** - * Returns information for fixing named specifiers. - */ - // function getFixesNamedSpecifiers( - // fixer: TSESLint.RuleFixer, - // node: TSESTree.ImportDeclaration, - // typeNamedSpecifiers: TSESTree.ImportSpecifier[], - // allNamedSpecifiers: TSESTree.ImportSpecifier[], - // ): { - // typeNamedSpecifiersText: string; - // removeTypeNamedSpecifiers: TSESLint.RuleFix[]; - // } { - // if (allNamedSpecifiers.length === 0) { - // return { - // typeNamedSpecifiersText: '', - // removeTypeNamedSpecifiers: [], - // }; - // } - // const typeNamedSpecifiersTexts: string[] = []; - // const removeTypeNamedSpecifiers: TSESLint.RuleFix[] = []; - // if (typeNamedSpecifiers.length === allNamedSpecifiers.length) { - // // e.g. - // // import Foo, {Type1, Type2} from 'foo' - // // import DefType, {Type1, Type2} from 'foo' - // const openingBraceToken = util.nullThrows( - // sourceCode.getTokenBefore( - // typeNamedSpecifiers[0], - // util.isOpeningBraceToken, - // ), - // util.NullThrowsReasons.MissingToken('{', node.type), - // ); - // const commaToken = util.nullThrows( - // sourceCode.getTokenBefore(openingBraceToken, util.isCommaToken), - // util.NullThrowsReasons.MissingToken(',', node.type), - // ); - // const closingBraceToken = util.nullThrows( - // sourceCode.getFirstTokenBetween( - // openingBraceToken, - // node.source, - // util.isClosingBraceToken, - // ), - // util.NullThrowsReasons.MissingToken('}', node.type), - // ); - - // // import DefType, {...} from 'foo' - // // ^^^^^^^ remove - // removeTypeNamedSpecifiers.push( - // fixer.removeRange([commaToken.range[0], closingBraceToken.range[1]]), - // ); - - // typeNamedSpecifiersTexts.push( - // sourceCode.text.slice( - // openingBraceToken.range[1], - // closingBraceToken.range[0], - // ), - // ); - // } else { - // const typeNamedSpecifierGroups: TSESTree.ImportSpecifier[][] = []; - // let group: TSESTree.ImportSpecifier[] = []; - // for (const namedSpecifier of allNamedSpecifiers) { - // if (typeNamedSpecifiers.includes(namedSpecifier)) { - // group.push(namedSpecifier); - // } else if (group.length) { - // typeNamedSpecifierGroups.push(group); - // group = []; - // } - // } - // if (group.length) { - // typeNamedSpecifierGroups.push(group); - // } - // for (const namedSpecifiers of typeNamedSpecifierGroups) { - // const { removeRange, textRange } = getNamedSpecifierRanges( - // namedSpecifiers, - // allNamedSpecifiers, - // ); - // removeTypeNamedSpecifiers.push(fixer.removeRange(removeRange)); - - // typeNamedSpecifiersTexts.push(sourceCode.text.slice(...textRange)); - // } - // } - // return { - // typeNamedSpecifiersText: typeNamedSpecifiersTexts.join(','), - // removeTypeNamedSpecifiers, - // }; - // } - - /** - * Returns ranges for fixing named specifier. - */ - // function getNamedSpecifierRanges( - // namedSpecifierGroup: TSESTree.ImportSpecifier[], - // allNamedSpecifiers: TSESTree.ImportSpecifier[], - // ): { - // textRange: TSESTree.Range; - // removeRange: TSESTree.Range; - // } { - // const first = namedSpecifierGroup[0]; - // const last = namedSpecifierGroup[namedSpecifierGroup.length - 1]; - // const removeRange: TSESTree.Range = [first.range[0], last.range[1]]; - // const textRange: TSESTree.Range = [...removeRange]; - // const before = sourceCode.getTokenBefore(first)!; - // textRange[0] = before.range[1]; - // if (util.isCommaToken(before)) { - // removeRange[0] = before.range[0]; - // } else { - // removeRange[0] = before.range[1]; - // } - - // const isFirst = allNamedSpecifiers[0] === first; - // const isLast = allNamedSpecifiers[allNamedSpecifiers.length - 1] === last; - // const after = sourceCode.getTokenAfter(last)!; - // textRange[1] = after.range[0]; - // if (isFirst || isLast) { - // if (util.isCommaToken(after)) { - // removeRange[1] = after.range[1]; - // } - // } - - // return { - // textRange, - // removeRange, - // }; - // } - - /** - * insert specifiers to named export node. - * e.g. - * import type { Already, Type1, Type2 } from 'foo' - * ^^^^^^^^^^^^^ insert - */ - // function insertToNamedExport( - // fixer: TSESLint.RuleFixer, - // target: TSESTree.ImportDeclaration, - // insertText: string, - // ): TSESLint.RuleFix { - // 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)) { - // insertText = ',' + insertText; - // } - // return fixer.insertTextBefore(closingBraceToken, insertText); - // } - - // function* fixToTypeExport(): IterableIterator { - // fixer: TSESLint.RuleFixer, - // report: ReportValueExport, - // sourceExports: SourceExports, - // const { node } = report; - // const { defaultSpecifier, namespaceSpecifier, namedSpecifiers } = - // classifySpecifier(node); - // if (namespaceSpecifier && !defaultSpecifier) { - // // e.g. - // // import * as types from 'foo' - // yield* fixToTypeExportByInsertType(fixer, node, false); - // return; - // } else if (defaultSpecifier) { - // if ( - // report.typeSpecifiers.includes(defaultSpecifier) && - // namedSpecifiers.length === 0 && - // !namespaceSpecifier - // ) { - // // e.g. - // // import Type from 'foo' - // yield* fixToTypeExportByInsertType(fixer, node, true); - // return; - // } - // } else { - // if ( - // namedSpecifiers.every(specifier => - // report.typeSpecifiers.includes(specifier), - // ) && - // !namespaceSpecifier - // ) { - // // e.g. - // // import {Type1, Type2} from 'foo' - // yield* fixToTypeExportByInsertType(fixer, node, false); - // return; - // } - // } - // const typeNamedSpecifiers = namedSpecifiers.filter(specifier => - // report.typeSpecifiers.includes(specifier), - // ); - // const fixesNamedSpecifiers = getFixesNamedSpecifiers( - // fixer, - // node, - // typeNamedSpecifiers, - // namedSpecifiers, - // ); - // const afterFixes: TSESLint.RuleFix[] = []; - // if (typeNamedSpecifiers.length) { - // if (sourceExports.typeOnlyNamedImport) { - // const insertTypeNamedSpecifiers = insertToNamedImport( - // fixer, - // sourceExports.typeOnlyNamedImport, - // fixesNamedSpecifiers.typeNamedSpecifiersText, - // ); - // if (sourceExports.typeOnlyNamedImport.range[1] <= node.range[0]) { - // yield insertTypeNamedSpecifiers; - // } else { - // afterFixes.push(insertTypeNamedSpecifiers); - // } - // } else { - // yield fixer.insertTextBefore( - // node, - // `import type {${ - // fixesNamedSpecifiers.typeNamedSpecifiersText - // }} from ${sourceCode.getText(node.source)};\n`, - // ); - // } - // } - // const fixesRemoveTypeNamespaceSpecifier: TSESLint.RuleFix[] = []; - // if ( - // namespaceSpecifier && - // report.typeSpecifiers.includes(namespaceSpecifier) - // ) { - // // e.g. - // // import Foo, * as Type from 'foo' - // // import DefType, * as Type from 'foo' - // // import DefType, * as Type from 'foo' - // const commaToken = util.nullThrows( - // sourceCode.getTokenBefore(namespaceSpecifier, util.isCommaToken), - // util.NullThrowsReasons.MissingToken(',', node.type), - // ); - // // import Def, * as Ns from 'foo' - // // ^^^^^^^^^ remove - // fixesRemoveTypeNamespaceSpecifier.push( - // fixer.removeRange([commaToken.range[0], namespaceSpecifier.range[1]]), - // ); - // // import type * as Ns from 'foo' - // // ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ insert - // yield fixer.insertTextBefore( - // node, - // `import type ${sourceCode.getText( - // namespaceSpecifier, - // )} from ${sourceCode.getText(node.source)};\n`, - // ); - // } - // if ( - // defaultSpecifier && - // report.typeSpecifiers.includes(defaultSpecifier) - // ) { - // if (report.typeSpecifiers.length === node.specifiers.length) { - // const importToken = util.nullThrows( - // sourceCode.getFirstToken(node, isExportToken), - // util.NullThrowsReasons.MissingToken('import', node.type), - // ); - // // import type Type from 'foo' - // // ^^^^ insert - // yield fixer.insertTextAfter(importToken, ' type'); - // } else { - // const commaToken = util.nullThrows( - // sourceCode.getTokenAfter(defaultSpecifier, util.isCommaToken), - // util.NullThrowsReasons.MissingToken(',', defaultSpecifier.type), - // ); - // // import Type , {...} from 'foo' - // // ^^^^^ pick - // const defaultText = sourceCode.text - // .slice(defaultSpecifier.range[0], commaToken.range[0]) - // .trim(); - // yield fixer.insertTextBefore( - // node, - // `import type ${defaultText} from ${sourceCode.getText( - // node.source, - // )};\n`, - // ); - // const afterToken = util.nullThrows( - // sourceCode.getTokenAfter(commaToken, { includeComments: true }), - // util.NullThrowsReasons.MissingToken('any token', node.type), - // ); - // // import Type , {...} from 'foo' - // // ^^^^^^^ remove - // yield fixer.removeRange([ - // defaultSpecifier.range[0], - // afterToken.range[0], - // ]); - // } - // } - // yield* fixesNamedSpecifiers.removeTypeNamedSpecifiers; - // yield* fixesRemoveTypeNamespaceSpecifier; - // yield* afterFixes; }, +}); - /** - * Fixer which inserts "type" into typed exports. - */ - // function* fixToTypeExportByInsertType( - // fixer: TSESLint.RuleFixer, - // node: TSESTree.ExportNamedDeclaration, - // isDefaultImport: boolean, - // ): IterableIterator { - // // export type Foo from 'foo' - // // ^^^^^ insert - // const importToken = util.nullThrows( - // sourceCode.getFirstToken(node, isExportToken), - // util.NullThrowsReasons.MissingToken('import', node.type), - // ); - // yield fixer.insertTextAfter(importToken, ' type'); - - // if (isDefaultImport) { - // // Has default import - // const openingBraceToken = sourceCode.getFirstTokenBetween( - // importToken, - // node.source, - // util.isOpeningBraceToken, - // ); - // if (openingBraceToken) { - // // Only braces. e.g. import Foo, {} from 'foo' - // const commaToken = util.nullThrows( - // sourceCode.getTokenBefore(openingBraceToken, util.isCommaToken), - // util.NullThrowsReasons.MissingToken(',', node.type), - // ); - // const closingBraceToken = util.nullThrows( - // sourceCode.getFirstTokenBetween( - // openingBraceToken, - // node.source, - // util.isClosingBraceToken, - // ), - // util.NullThrowsReasons.MissingToken('}', node.type), - // ); - - // // import type Foo, {} from 'foo' - // // ^^ remove - // yield fixer.removeRange([ - // commaToken.range[0], - // closingBraceToken.range[1], - // ]); - // const specifiersText = sourceCode.text.slice( - // commaToken.range[1], - // closingBraceToken.range[1], - // ); - // if (node.specifiers.length > 1) { - // // import type Foo from 'foo' - // // import type {...} from 'foo' // <- insert - // yield fixer.insertTextAfter( - // node, - // `\nimport type${specifiersText} from ${sourceCode.getText( - // node.source, - // )};`, - // ); - // } - // } - // } - // } - - // function* fixToValueExportInDecoMeta( - // fixer: TSESLint.RuleFixer, - // report: ReportValueExport, - // sourceImports: SourceImports, - // ): IterableIterator { - // const { node } = report; - - // const { defaultSpecifier, namespaceSpecifier, namedSpecifiers } = - // classifySpecifier(node); - - // if (namespaceSpecifier) { - // // e.g. - // // import type * as types from 'foo' - // yield* fixToValueExport(fixer, node); - // return; - // } else if (defaultSpecifier) { - // if ( - // report.valueSpecifiers.includes(defaultSpecifier) && - // namedSpecifiers.length === 0 - // ) { - // // e.g. - // // import type Type from 'foo' - // yield* fixToValueExport(fixer, node); - // return; - // } - // } else { - // if ( - // namedSpecifiers.every(specifier => - // report.valueSpecifiers.includes(specifier), - // ) - // ) { - // // e.g. - // // import type {Type1, Type2} from 'foo' - // yield* fixToValueExport(fixer, node); - // return; - // } - // } - - // const valueNamedSpecifiers = namedSpecifiers.filter(specifier => - // report.valueSpecifiers.includes(specifier), - // ); - - // const fixesNamedSpecifiers = getFixesNamedSpecifiers( - // fixer, - // node, - // valueNamedSpecifiers, - // namedSpecifiers, - // ); - // const afterFixes: TSESLint.RuleFix[] = []; - // if (valueNamedSpecifiers.length) { - // if (sourceImports.valueOnlyNamedImport) { - // const insertTypeNamedSpecifiers = insertToNamedImport( - // fixer, - // sourceImports.valueOnlyNamedImport, - // fixesNamedSpecifiers.typeNamedSpecifiersText, - // ); - // if (sourceImports.valueOnlyNamedImport.range[1] <= node.range[0]) { - // yield insertTypeNamedSpecifiers; - // } else { - // afterFixes.push(insertTypeNamedSpecifiers); - // } - // } else { - // yield fixer.insertTextBefore( - // node, - // `import {${ - // fixesNamedSpecifiers.typeNamedSpecifiersText - // }} from ${sourceCode.getText(node.source)};\n`, - // ); - // } - // } - - // yield* fixesNamedSpecifiers.removeTypeNamedSpecifiers; - - // yield* afterFixes; - // } - - /** - * Remove "type" from an export. - */ - // function* fixToValueExport( - // fixer: TSESLint.RuleFixer, - // node: TSESTree.ExportNamedDeclaration, - // ): IterableIterator { - // // export type { Foo } from 'foo' - // // ^^^^ remove - // const exportToken = util.nullThrows( - // sourceCode.getFirstToken(node, isExportToken), - // util.NullThrowsReasons.MissingToken('export', node.type), - // ); - - // const typeToken = util.nullThrows( - // sourceCode.getFirstTokenBetween( - // exportToken, - // node.specifiers[0]?.local ?? node.source, - // isTypeToken, - // ), - // util.NullThrowsReasons.MissingToken('type', node.type), - // ); +/** + * Helper for identifying if an export specifier resolves to a + * JavaScript value or a TypeScript type. + * + * @returns True/false if is a type or not, or undefined if the specifier + * can't be resolved. + */ +function isSpecifierTypeBased( + parserServices: ParserServices, + specifier: TSESTree.ExportSpecifier, +): boolean | undefined { + const checker = parserServices.program.getTypeChecker(); + const node = parserServices.esTreeNodeToTSNodeMap.get(specifier.exported); + const symbol = checker.getSymbolAtLocation(node); + const aliasedSymbol = checker.getAliasedSymbol(symbol!); + + if (!aliasedSymbol || aliasedSymbol.escapedName === 'unknown') { + return undefined; + } + + return !(aliasedSymbol.flags & SymbolFlags.Value); +} - // const afterToken = util.nullThrows( - // sourceCode.getTokenAfter(typeToken, { includeComments: true }), - // util.NullThrowsReasons.MissingToken('any token', node.type), - // ); - // yield fixer.removeRange([typeToken.range[0], afterToken.range[0]]); - // } - // }, -}); +/** + * Inserts "type" into an export. + * + * Example: + * + * export type { Foo } from 'foo'; + * ^^^^ + */ +function* fixToTypeExportByInsertType( + fixer: TSESLint.RuleFixer, + sourceCode: Readonly, + node: TSESTree.ExportNamedDeclaration, +): IterableIterator { + const exportToken = util.nullThrows( + sourceCode.getFirstToken(node), + util.NullThrowsReasons.MissingToken('export', node.type), + ); + + yield fixer.insertTextAfter(exportToken, ' type'); +} From 582d49c7dd2b87578e9a600bc80a5381481d7a6a Mon Sep 17 00:00:00 2001 From: dzearing Date: Mon, 27 Sep 2021 11:31:05 -0700 Subject: [PATCH 03/16] It works! --- .../src/rules/consistent-type-exports.ts | 164 +++++++++++++----- 1 file changed, 117 insertions(+), 47 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index e1e5a178b23..426d6ece790 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -50,13 +50,14 @@ export default util.createRule({ typeOverValue: 'All exports in the declaration are only used as types. Use `export type`.', valueOverType: 'Use an `export` instead of an `export type`.', - singleExportIsType: 'Export {{typeExports}} is only used as types.', + singleExportIsType: + 'Type export {{exportNames}} is not a value and should be exported using `export type`.', multipleExportsAreTypes: - 'Exports {{typeExports}} should be exported using `export type`.', + 'Type exports {{exportNames}} are not values and should be exported using `export type`.', singleExportisValue: - 'Type export {{typeExports}} is not a type and should be exported using `export`.', + 'Value export {{exportNames}} is exported as a type only and should be exported using `export`.', multipleExportsAreValues: - 'Exports {{typeExports}} are not types and should be exported using `export`.', + 'Value exports {{exportNames}} are exported as types only and should be exported using `export`.', }, schema: [ { @@ -154,6 +155,8 @@ export default util.createRule({ 'Program:exit'(): void { for (const sourceExports of Object.values(sourceExportsMap)) { + console.log('sourceExports', sourceExports); + // If this export has no issues, move on. if (sourceExports.reportValueExports.length === 0) { continue; @@ -175,57 +178,67 @@ export default util.createRule({ }); } else if (!report.typeSpecifiers.length) { // we only have value violations; remove the `type` from the export + // TODO: remove the `type` from the export + context.report({ + node: report.node, + messageId: 'valueOverType', + // *fix(fixer) { + // yield* fixToValueExportByRemovingType( + // fixer, + // sourceCode, + // report.node + // ) + // } + }); } else { // We have both type and value violations. - const isTypeExport = (report.node.exportKind = 'type'); - const exportNames = ( + const isTypeExport = report.node.exportKind === 'type'; + const allExportNames = ( isTypeExport ? report.valueSpecifiers : report.typeSpecifiers ).map(specifier => `${specifier.local.name}`); - const message = ((): { - messageId: MessageIds; - data: Record; - } => { - if (exportNames.length === 1) { - const typeExports = exportNames[0]; - return { - messageId: isTypeExport - ? 'singleExportisValue' - : 'singleExportIsType', - data: { typeExports }, - }; - } else { - const typeExports = [ - exportNames.slice(0, -1).join(', '), - exportNames.slice(-1)[0], - ].join(' and '); - - return { - messageId: isTypeExport - ? 'multipleExportsAreValues' - : 'multipleExportsAreTypes', - data: { typeExports }, - }; - } - })(); + if (allExportNames.length === 1) { + const exportNames = allExportNames[0]; - context.report({ - node: report.node, - ...message, - // *fix(fixer) { - // if (isTypeExport) { - // yield* fixToValueImportInDecoMeta( - // fixer, - // report, - // sourceImports, - // ); - // } else { - // yield* fixToTypeImport(fixer, report, sourceImports); - // } - // }, - }); + context.report({ + node: report.node, + messageId: isTypeExport + ? 'singleExportisValue' + : 'singleExportIsType', + data: { exportNames }, + *fix(fixer) { + yield* fixSeparateNamedExports( + fixer, + sourceCode, + sourceExports, + report, + ); + }, + }); + } else { + const exportNames = [ + allExportNames.slice(0, -1).join(', '), + allExportNames.slice(-1)[0], + ].join(' and '); + + context.report({ + node: report.node, + messageId: isTypeExport + ? 'multipleExportsAreValues' + : 'multipleExportsAreTypes', + data: { exportNames }, + *fix(fixer) { + yield* fixSeparateNamedExports( + fixer, + sourceCode, + sourceExports, + report, + ); + }, + }); + } } } } @@ -279,3 +292,60 @@ function* fixToTypeExportByInsertType( yield fixer.insertTextAfter(exportToken, ' type'); } + +/** + * Separates the exports which mismatch the kind of export the given + * node represents. For example, a type export's named specifiers which + * represent values will be inserted in a separate `export` statement. + */ +function* fixSeparateNamedExports( + fixer: TSESLint.RuleFixer, + sourceCode: Readonly, + sourceExports: SourceExports, + report: ReportValueExport, +): IterableIterator { + const { node, typeSpecifiers, valueSpecifiers } = report; + const separateTypes = node.exportKind !== 'type'; + const specifiersToSeparate = separateTypes ? typeSpecifiers : valueSpecifiers; + const specifierNames = specifiersToSeparate.map( + specifier => specifier.local.name, + ); + + console.log('separating types:', separateTypes, node, specifierNames); + + const exportToken = util.nullThrows( + sourceCode.getFirstToken(node), + util.NullThrowsReasons.MissingToken('export', node.type), + ); + + // Filter the bad exports from the current line. + const filteredSpecifierNames = ( + separateTypes ? valueSpecifiers : typeSpecifiers + ) + .map(specifier => specifier.local.name) + .join(', '); + const openToken = util.nullThrows( + sourceCode.getFirstToken(node, util.isOpeningBraceToken), + util.NullThrowsReasons.MissingToken('{', node.type), + ); + const closeToken = util.nullThrows( + sourceCode.getLastToken(node, util.isClosingBraceToken), + util.NullThrowsReasons.MissingToken('}', node.type), + ); + + console.log('opentoken', openToken); + console.log('closeToken', closeToken); + + yield fixer.replaceTextRange( + [openToken.range[1], closeToken.range[0]], + ` ${filteredSpecifierNames} `, + ); + + // Insert the bad exports into a new export line. + yield fixer.insertTextBefore( + exportToken, + `export ${separateTypes ? 'type ' : ''}{ ${specifierNames.join( + ', ', + )} } from ${sourceCode.getText(node.source!)};\n`, + ); +} From 1b4880a3946c50845d4218e7a9d45333c17c7b9f Mon Sep 17 00:00:00 2001 From: dzearing Date: Mon, 27 Sep 2021 12:42:41 -0700 Subject: [PATCH 04/16] Adding a few tests, removing logging, adding rule to a..ts. --- packages/eslint-plugin/src/configs/all.ts | 1 + .../src/rules/consistent-type-exports.ts | 13 ------- .../rules/consistent-type-exports.test.ts | 36 +++++++++++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts diff --git a/packages/eslint-plugin/src/configs/all.ts b/packages/eslint-plugin/src/configs/all.ts index eff553b5b9b..b2d1ee238ec 100644 --- a/packages/eslint-plugin/src/configs/all.ts +++ b/packages/eslint-plugin/src/configs/all.ts @@ -22,6 +22,7 @@ export = { '@typescript-eslint/consistent-type-assertions': 'error', '@typescript-eslint/consistent-type-definitions': 'error', '@typescript-eslint/consistent-type-imports': 'error', + '@typescript-eslint/consistent-type-exports': 'error', 'default-param-last': 'off', '@typescript-eslint/default-param-last': 'error', 'dot-notation': 'off', diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 426d6ece790..d442a29e0a3 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -1,4 +1,3 @@ -/* eslint-disable no-console */ import { TSESTree, ParserServices, @@ -131,8 +130,6 @@ export default util.createRule({ specifier, ); - console.log('specifier', specifier.local.name, isTypeBased); - if (isTypeBased === true) { typeSpecifiers.push(specifier); } else if (isTypeBased === false) { @@ -155,8 +152,6 @@ export default util.createRule({ 'Program:exit'(): void { for (const sourceExports of Object.values(sourceExportsMap)) { - console.log('sourceExports', sourceExports); - // If this export has no issues, move on. if (sourceExports.reportValueExports.length === 0) { continue; @@ -212,7 +207,6 @@ export default util.createRule({ yield* fixSeparateNamedExports( fixer, sourceCode, - sourceExports, report, ); }, @@ -233,7 +227,6 @@ export default util.createRule({ yield* fixSeparateNamedExports( fixer, sourceCode, - sourceExports, report, ); }, @@ -301,7 +294,6 @@ function* fixToTypeExportByInsertType( function* fixSeparateNamedExports( fixer: TSESLint.RuleFixer, sourceCode: Readonly, - sourceExports: SourceExports, report: ReportValueExport, ): IterableIterator { const { node, typeSpecifiers, valueSpecifiers } = report; @@ -311,8 +303,6 @@ function* fixSeparateNamedExports( specifier => specifier.local.name, ); - console.log('separating types:', separateTypes, node, specifierNames); - const exportToken = util.nullThrows( sourceCode.getFirstToken(node), util.NullThrowsReasons.MissingToken('export', node.type), @@ -333,9 +323,6 @@ function* fixSeparateNamedExports( util.NullThrowsReasons.MissingToken('}', node.type), ); - console.log('opentoken', openToken); - console.log('closeToken', closeToken); - yield fixer.replaceTextRange( [openToken.range[1], closeToken.range[0]], ` ${filteredSpecifierNames} `, diff --git a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts new file mode 100644 index 00000000000..ec2f153f851 --- /dev/null +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -0,0 +1,36 @@ +/* eslint-disable @typescript-eslint/internal/plugin-test-formatting */ +import rule from '../../src/rules/consistent-type-exports'; +import { RuleTester, getFixturesRootDir } from '../RuleTester'; + +const rootDir = getFixturesRootDir(); + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + ecmaVersion: 2020, + sourceType: 'module', + tsconfigRootDir: rootDir, + project: './tsconfig.json', + }, +}); + +ruleTester.run('consistent-type-exports', rule, { + valid: [ + `export { Foo } from 'foo';`, + "export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';", + "export { BlockScope } from '@typescript-eslint/experimental-utils';", + ], + invalid: [ + { + code: `export { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, + output: `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'typeOverValue', + line: 1, + column: 1, + }, + ], + }, + ], +}); From 97bb77770366901c1929187da02aab66b2113e52 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 28 Sep 2021 12:01:48 -0700 Subject: [PATCH 05/16] chore: adding tests, ensuring alias exports work --- .../src/rules/consistent-type-exports.ts | 57 +++++++-- .../rules/consistent-type-exports.test.ts | 113 ++++++++++++++++++ 2 files changed, 157 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index d442a29e0a3..ef3631850fc 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -33,7 +33,7 @@ type MessageIds = | 'valueOverType' | 'singleExportIsType' | 'multipleExportsAreTypes' - | 'singleExportisValue' + | 'singleExportIsValue' | 'multipleExportsAreValues'; export default util.createRule({ @@ -53,7 +53,7 @@ export default util.createRule({ 'Type export {{exportNames}} is not a value and should be exported using `export type`.', multipleExportsAreTypes: 'Type exports {{exportNames}} are not values and should be exported using `export type`.', - singleExportisValue: + singleExportIsValue: 'Value export {{exportNames}} is exported as a type only and should be exported using `export`.', multipleExportsAreValues: 'Value exports {{exportNames}} are exported as types only and should be exported using `export`.', @@ -164,7 +164,7 @@ export default util.createRule({ node: report.node, messageId: 'typeOverValue', *fix(fixer) { - yield* fixToTypeExportByInsertType( + yield* fixExportInsertType( fixer, sourceCode, report.node, @@ -177,13 +177,13 @@ export default util.createRule({ context.report({ node: report.node, messageId: 'valueOverType', - // *fix(fixer) { - // yield* fixToValueExportByRemovingType( - // fixer, - // sourceCode, - // report.node - // ) - // } + *fix(fixer) { + yield* fixExportRemoveType( + fixer, + sourceCode, + report.node, + ); + }, }); } else { // We have both type and value violations. @@ -200,7 +200,7 @@ export default util.createRule({ context.report({ node: report.node, messageId: isTypeExport - ? 'singleExportisValue' + ? 'singleExportIsValue' : 'singleExportIsType', data: { exportNames }, *fix(fixer) { @@ -273,7 +273,7 @@ function isSpecifierTypeBased( * export type { Foo } from 'foo'; * ^^^^ */ -function* fixToTypeExportByInsertType( +function* fixExportInsertType( fixer: TSESLint.RuleFixer, sourceCode: Readonly, node: TSESTree.ExportNamedDeclaration, @@ -286,6 +286,32 @@ function* fixToTypeExportByInsertType( yield fixer.insertTextAfter(exportToken, ' type'); } +/** + * Removes "type" from an export. + * + * Example: + * + * export type { Foo } from 'foo'; + * ^^^^ + */ +function* fixExportRemoveType( + fixer: TSESLint.RuleFixer, + sourceCode: Readonly, + node: TSESTree.ExportNamedDeclaration, +): IterableIterator { + const exportToken = util.nullThrows( + sourceCode.getFirstToken(node), + util.NullThrowsReasons.MissingToken('export', node.type), + ); + + const typeToken = util.nullThrows( + sourceCode.getTokenAfter(exportToken), + util.NullThrowsReasons.MissingToken('type', node.type), + ); + + yield fixer.removeRange([exportToken.range[1], typeToken.range[1]]); +} + /** * Separates the exports which mismatch the kind of export the given * node represents. For example, a type export's named specifiers which @@ -300,7 +326,12 @@ function* fixSeparateNamedExports( const separateTypes = node.exportKind !== 'type'; const specifiersToSeparate = separateTypes ? typeSpecifiers : valueSpecifiers; const specifierNames = specifiersToSeparate.map( - specifier => specifier.local.name, + specifier => + `${specifier.local.name}${ + specifier.exported.name !== specifier.local.name + ? ` as ${specifier.exported.name}` + : '' + }`, ); const exportToken = util.nullThrows( 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 ec2f153f851..52c9cdaca7c 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -32,5 +32,118 @@ ruleTester.run('consistent-type-exports', rule, { }, ], }, + { + code: `export type { BlockScope } from '@typescript-eslint/scope-manager';`, + output: `export { BlockScope } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'valueOverType', + line: 1, + column: 1, + }, + ], + }, + { + code: `export { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';`, + output: + `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';\n` + + `export { BlockScope } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'singleExportIsType', + line: 1, + column: 1, + }, + ], + }, + { + code: `export { AnalyzeOptions, BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, + output: + `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';\n` + + `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'singleExportIsType', + line: 1, + column: 1, + }, + ], + }, + { + code: `export { AnalyzeOptions, BlockScope, Definition, CatchScope } from '@typescript-eslint/scope-manager';`, + output: + `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';\n` + + `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'multipleExportsAreTypes', + line: 1, + column: 1, + }, + ], + }, + { + code: `export type { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';`, + output: + `export { BlockScope } from '@typescript-eslint/scope-manager';\n` + + `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'singleExportIsValue', + line: 1, + column: 1, + }, + ], + }, + { + code: `export type { AnalyzeOptions, BlockScope, Definition } from '@typescript-eslint/scope-manager';`, + output: + `export { BlockScope } from '@typescript-eslint/scope-manager';\n` + + `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'singleExportIsValue', + line: 1, + column: 1, + }, + ], + }, + { + code: `export type { AnalyzeOptions, BlockScope, Definition, CatchScope } from '@typescript-eslint/scope-manager';`, + output: + `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';\n` + + `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'multipleExportsAreValues', + line: 1, + column: 1, + }, + ], + }, + { + code: `export { Definition as Foo } from '@typescript-eslint/scope-manager';`, + output: `export type { Definition as Foo } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'typeOverValue', + line: 1, + column: 1, + }, + ], + }, + { + code: `export { Definition as Foo, BlockScope } from '@typescript-eslint/scope-manager';`, + output: + `export type { Definition as Foo } from '@typescript-eslint/scope-manager';\n` + + `export { BlockScope } from '@typescript-eslint/scope-manager';`, + errors: [ + { + messageId: 'singleExportIsType', + line: 1, + column: 1, + }, + ], + }, ], }); From 71ce5fde9edc9b2b1e07630465636ab07d7ba132 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 28 Sep 2021 12:40:40 -0700 Subject: [PATCH 06/16] chore: factoring out a getSpecifierText helper, updating test formatting --- .../src/rules/consistent-type-exports.ts | 28 ++-- .../rules/consistent-type-exports.test.ts | 145 +++++++++++++----- 2 files changed, 121 insertions(+), 52 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index ef3631850fc..6a746cdaf79 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -325,14 +325,7 @@ function* fixSeparateNamedExports( const { node, typeSpecifiers, valueSpecifiers } = report; const separateTypes = node.exportKind !== 'type'; const specifiersToSeparate = separateTypes ? typeSpecifiers : valueSpecifiers; - const specifierNames = specifiersToSeparate.map( - specifier => - `${specifier.local.name}${ - specifier.exported.name !== specifier.local.name - ? ` as ${specifier.exported.name}` - : '' - }`, - ); + const specifierNames = specifiersToSeparate.map(getSpecifierText).join(', '); const exportToken = util.nullThrows( sourceCode.getFirstToken(node), @@ -343,7 +336,7 @@ function* fixSeparateNamedExports( const filteredSpecifierNames = ( separateTypes ? valueSpecifiers : typeSpecifiers ) - .map(specifier => specifier.local.name) + .map(getSpecifierText) .join(', '); const openToken = util.nullThrows( sourceCode.getFirstToken(node, util.isOpeningBraceToken), @@ -354,16 +347,25 @@ function* fixSeparateNamedExports( util.NullThrowsReasons.MissingToken('}', node.type), ); + // Remove exports from the current line which we're going to re-insert. yield fixer.replaceTextRange( [openToken.range[1], closeToken.range[0]], ` ${filteredSpecifierNames} `, ); - // Insert the bad exports into a new export line. + // Insert the bad exports into a new export line above. yield fixer.insertTextBefore( exportToken, - `export ${separateTypes ? 'type ' : ''}{ ${specifierNames.join( - ', ', - )} } from ${sourceCode.getText(node.source!)};\n`, + `export ${ + separateTypes ? 'type ' : '' + }{ ${specifierNames} } from ${sourceCode.getText(node.source!)};\n`, ); } + +function getSpecifierText(specifier: TSESTree.ExportSpecifier): string { + return `${specifier.local.name}${ + specifier.exported.name !== specifier.local.name + ? ` as ${specifier.exported.name}` + : '' + }`; +} 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 52c9cdaca7c..1daf93b1a67 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -1,4 +1,3 @@ -/* eslint-disable @typescript-eslint/internal/plugin-test-formatting */ import rule from '../../src/rules/consistent-type-exports'; import { RuleTester, getFixturesRootDir } from '../RuleTester'; @@ -16,14 +15,15 @@ const ruleTester = new RuleTester({ ruleTester.run('consistent-type-exports', rule, { valid: [ - `export { Foo } from 'foo';`, + "export { Foo } from 'foo';", "export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';", "export { BlockScope } from '@typescript-eslint/experimental-utils';", ], invalid: [ { - code: `export { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, - output: `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, + code: "export { AnalyzeOptions } from '@typescript-eslint/scope-manager';", + output: + "export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';", errors: [ { messageId: 'typeOverValue', @@ -33,8 +33,8 @@ ruleTester.run('consistent-type-exports', rule, { ], }, { - code: `export type { BlockScope } from '@typescript-eslint/scope-manager';`, - output: `export { BlockScope } from '@typescript-eslint/scope-manager';`, + code: "export type { BlockScope } from '@typescript-eslint/scope-manager';", + output: "export { BlockScope } from '@typescript-eslint/scope-manager';", errors: [ { messageId: 'valueOverType', @@ -44,7 +44,7 @@ ruleTester.run('consistent-type-exports', rule, { ], }, { - code: `export { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';`, + code: "export { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';", output: `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';\n` + `export { BlockScope } from '@typescript-eslint/scope-manager';`, @@ -57,73 +57,113 @@ ruleTester.run('consistent-type-exports', rule, { ], }, { - code: `export { AnalyzeOptions, BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, - output: - `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';\n` + - `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, + code: ` +export { + AnalyzeOptions, + BlockScope, + CatchScope, +} from '@typescript-eslint/scope-manager'; + `, + output: ` +export type { AnalyzeOptions } from '@typescript-eslint/scope-manager'; +export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'singleExportIsType', - line: 1, + line: 2, column: 1, }, ], }, { - code: `export { AnalyzeOptions, BlockScope, Definition, CatchScope } from '@typescript-eslint/scope-manager';`, - output: - `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';\n` + - `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';`, + code: ` +export { + AnalyzeOptions, + BlockScope, + Definition, + CatchScope, +} from '@typescript-eslint/scope-manager'; + `, + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` +export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; +export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'multipleExportsAreTypes', - line: 1, + line: 2, column: 1, }, ], }, { - code: `export type { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';`, - output: - `export { BlockScope } from '@typescript-eslint/scope-manager';\n` + - `export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';`, + code: ` +export type { + AnalyzeOptions, + BlockScope, +} from '@typescript-eslint/scope-manager'; + `, + output: ` +export { BlockScope } from '@typescript-eslint/scope-manager'; +export type { AnalyzeOptions } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'singleExportIsValue', - line: 1, + line: 2, column: 1, }, ], }, { - code: `export type { AnalyzeOptions, BlockScope, Definition } from '@typescript-eslint/scope-manager';`, - output: - `export { BlockScope } from '@typescript-eslint/scope-manager';\n` + - `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';`, + code: ` +export type { + AnalyzeOptions, + BlockScope, + Definition, +} from '@typescript-eslint/scope-manager'; + `, + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` +export { BlockScope } from '@typescript-eslint/scope-manager'; +export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'singleExportIsValue', - line: 1, + line: 2, column: 1, }, ], }, { - code: `export type { AnalyzeOptions, BlockScope, Definition, CatchScope } from '@typescript-eslint/scope-manager';`, - output: - `export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager';\n` + - `export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager';`, + code: ` +export type { + AnalyzeOptions, + BlockScope, + Definition, + CatchScope, +} from '@typescript-eslint/scope-manager'; + `, + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` +export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; +export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'multipleExportsAreValues', - line: 1, + line: 2, column: 1, }, ], }, { - code: `export { Definition as Foo } from '@typescript-eslint/scope-manager';`, - output: `export type { Definition as Foo } from '@typescript-eslint/scope-manager';`, + code: "export { Definition as Foo } from '@typescript-eslint/scope-manager';", + output: + "export type { Definition as Foo } from '@typescript-eslint/scope-manager';", errors: [ { messageId: 'typeOverValue', @@ -133,14 +173,41 @@ ruleTester.run('consistent-type-exports', rule, { ], }, { - code: `export { Definition as Foo, BlockScope } from '@typescript-eslint/scope-manager';`, - output: - `export type { Definition as Foo } from '@typescript-eslint/scope-manager';\n` + - `export { BlockScope } from '@typescript-eslint/scope-manager';`, + code: ` +export { + Definition as Foo, + BlockScope, +} from '@typescript-eslint/scope-manager'; + `, + output: ` +export type { Definition as Foo } from '@typescript-eslint/scope-manager'; +export { BlockScope } from '@typescript-eslint/scope-manager'; + `, errors: [ { messageId: 'singleExportIsType', - line: 1, + line: 2, + column: 1, + }, + ], + }, + { + code: ` +export { + Definition as Foo, + BlockScope as BScope, + CatchScope as CScope, +} from '@typescript-eslint/scope-manager'; + `, + // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting + output: ` +export type { Definition as Foo } from '@typescript-eslint/scope-manager'; +export { BlockScope as BScope, CatchScope as CScope } from '@typescript-eslint/scope-manager'; + `, + errors: [ + { + messageId: 'singleExportIsType', + line: 2, column: 1, }, ], From a5dcc2b79423b9a6bb8990901366113ff1fd7e23 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 28 Sep 2021 13:05:45 -0700 Subject: [PATCH 07/16] chore: adding documentation --- packages/eslint-plugin/README.md | 1 + .../docs/rules/consistent-type-exports.md | 27 ++ .../src/rules/consistent-type-exports.ts | 289 ++++++++---------- 3 files changed, 154 insertions(+), 163 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/consistent-type-exports.md diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md index c8c783dba6f..618044252ac 100644 --- a/packages/eslint-plugin/README.md +++ b/packages/eslint-plugin/README.md @@ -107,6 +107,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int | [`@typescript-eslint/consistent-indexed-object-style`](./docs/rules/consistent-indexed-object-style.md) | Enforce or disallow the use of the record type | | :wrench: | | | [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | | | | | [`@typescript-eslint/consistent-type-definitions`](./docs/rules/consistent-type-definitions.md) | Consistent with type definition either `interface` or `type` | | :wrench: | | +| [`@typescript-eslint/consistent-type-exports`](./docs/rules/consistent-type-exports.md) | Enforces consistent usage of type exports | | :wrench: | :thought_balloon: | | [`@typescript-eslint/consistent-type-imports`](./docs/rules/consistent-type-imports.md) | Enforces consistent usage of type imports | | :wrench: | | | [`@typescript-eslint/explicit-function-return-type`](./docs/rules/explicit-function-return-type.md) | Require explicit return types on functions and class methods | | | | | [`@typescript-eslint/explicit-member-accessibility`](./docs/rules/explicit-member-accessibility.md) | Require explicit accessibility modifiers on class properties and methods | | :wrench: | | diff --git a/packages/eslint-plugin/docs/rules/consistent-type-exports.md b/packages/eslint-plugin/docs/rules/consistent-type-exports.md new file mode 100644 index 00000000000..c3f10f8e6c3 --- /dev/null +++ b/packages/eslint-plugin/docs/rules/consistent-type-exports.md @@ -0,0 +1,27 @@ +# Enforces consistent usage of type exports (`consistent-type-exports`) + +TypeScript 3.8 added support for type-only exports. + +Type-only exports allow you to specify that 1 or more named exports are exported as type-only. This allows +transpilers to drop exports without knowing the types of the dependencies. + +## Rule Details + +This rule aims to standardize the use of type exports style across the codebase. + +Given a class `Button`, and an interface `ButtonProps`, examples of **correct** code: + +```ts +export { Button } from 'some-library'; +export type { ButtonProps } from 'some-library'; +``` + +Examples of **incorrect** code: + +```ts +import { Button, ButtonProps } from 'some-library'; +``` + +## When Not To Use It + +- If you are not using TypeScript 3.8 (or greater), then you will not be able to use this rule, as type-only imports are not allowed. diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 6a746cdaf79..df2ff0c3aa8 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -7,13 +7,7 @@ import { import { SymbolFlags } from 'typescript'; import * as util from '../util'; -type Prefer = 'type-exports' | 'no-type-exports'; - -type Options = [ - { - prefer?: Prefer; - }, -]; +type Options = []; interface SourceExports { source: string; @@ -38,12 +32,14 @@ type MessageIds = export default util.createRule({ name: 'consistent-type-exports', + defaultOptions: [], meta: { type: 'suggestion', docs: { description: 'Enforces consistent usage of type exports', category: 'Stylistic Issues', recommended: false, + requiresTypeChecking: true, }, messages: { typeOverValue: @@ -72,172 +68,139 @@ export default util.createRule({ fixable: 'code', }, - defaultOptions: [ - { - prefer: 'type-exports', - }, - ], - - create(context, [option]) { + create(context) { const sourceCode = context.getSourceCode(); - const prefer = option.prefer ?? 'type-exports'; // const sourceCode = context.getSourceCode(); const sourceExportsMap: { [key: string]: SourceExports } = {}; return { - ...(prefer === 'type-exports' - ? { - ExportNamedDeclaration( - node: TSESTree.ExportNamedDeclaration, - ): void { - // Coerce the node.source.value to a string via asserting. - if ( - node.source?.type !== AST_NODE_TYPES.Literal || - typeof node.source?.value !== 'string' - ) { - return; - } - - const source = node.source.value; - const sourceExports = (sourceExportsMap[source] = - sourceExportsMap[source] || { - source, - reportValueExports: [], - typeOnlyNamedExport: null, - valueOnlyNamedExport: null, - }); - const parserServices = util.getParserServices(context); - - // 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) { - // The export is a type export - sourceExports.typeOnlyNamedExport = node; - } - } else if (!sourceExports.valueOnlyNamedExport) { - // The export is a value export - sourceExports.valueOnlyNamedExport = node; - } + ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration): void { + // Coerce the node.source.value to a string via asserting. + if ( + node.source?.type !== AST_NODE_TYPES.Literal || + typeof node.source?.value !== 'string' + ) { + return; + } + + const source = node.source.value; + const sourceExports = (sourceExportsMap[source] = sourceExportsMap[ + source + ] || { + source, + reportValueExports: [], + typeOnlyNamedExport: null, + valueOnlyNamedExport: null, + }); + const parserServices = util.getParserServices(context); + + // 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) { + // The export is a type export + sourceExports.typeOnlyNamedExport = node; + } + } else if (!sourceExports.valueOnlyNamedExport) { + // The export is a value export + sourceExports.valueOnlyNamedExport = node; + } + + // Next for the current import, we will separate type/value specifiers. + const typeSpecifiers: TSESTree.ExportSpecifier[] = []; + const valueSpecifiers: TSESTree.ExportSpecifier[] = []; + + for (const specifier of node.specifiers) { + const isTypeBased = isSpecifierTypeBased(parserServices, specifier); + + if (isTypeBased === true) { + typeSpecifiers.push(specifier); + } else if (isTypeBased === false) { + // undefined means we don't know. + valueSpecifiers.push(specifier); + } + } + + if ( + (node.exportKind === 'value' && typeSpecifiers.length) || + (node.exportKind === 'type' && valueSpecifiers.length) + ) { + sourceExports.reportValueExports.push({ + node, + typeSpecifiers, + valueSpecifiers, + }); + } + }, - // Next for the current import, we will separate type/value specifiers. - const typeSpecifiers: TSESTree.ExportSpecifier[] = []; - const valueSpecifiers: TSESTree.ExportSpecifier[] = []; - - for (const specifier of node.specifiers) { - const isTypeBased = isSpecifierTypeBased( - parserServices, - specifier, - ); - - if (isTypeBased === true) { - typeSpecifiers.push(specifier); - } else if (isTypeBased === false) { - // undefined means we don't know. - valueSpecifiers.push(specifier); - } - } + 'Program:exit'(): void { + for (const sourceExports of Object.values(sourceExportsMap)) { + // If this export has no issues, move on. + if (sourceExports.reportValueExports.length === 0) { + continue; + } - if ( - (node.exportKind === 'value' && typeSpecifiers.length) || - (node.exportKind === 'type' && valueSpecifiers.length) - ) { - sourceExports.reportValueExports.push({ - node, - typeSpecifiers, - valueSpecifiers, + for (const report of sourceExports.reportValueExports) { + if (!report.valueSpecifiers.length) { + // export is all type-only; convert the entire export to `export type` + context.report({ + node: report.node, + messageId: 'typeOverValue', + *fix(fixer) { + yield* fixExportInsertType(fixer, sourceCode, report.node); + }, + }); + } else if (!report.typeSpecifiers.length) { + // we only have value violations; remove the `type` from the export + // TODO: remove the `type` from the export + context.report({ + node: report.node, + messageId: 'valueOverType', + *fix(fixer) { + yield* fixExportRemoveType(fixer, sourceCode, report.node); + }, + }); + } else { + // We have both type and value violations. + const isTypeExport = report.node.exportKind === 'type'; + const allExportNames = ( + isTypeExport ? report.valueSpecifiers : report.typeSpecifiers + ).map(specifier => `${specifier.local.name}`); + + if (allExportNames.length === 1) { + const exportNames = allExportNames[0]; + + context.report({ + node: report.node, + messageId: isTypeExport + ? 'singleExportIsValue' + : 'singleExportIsType', + data: { exportNames }, + *fix(fixer) { + yield* fixSeparateNamedExports(fixer, sourceCode, report); + }, + }); + } else { + const exportNames = [ + allExportNames.slice(0, -1).join(', '), + allExportNames.slice(-1)[0], + ].join(' and '); + + context.report({ + node: report.node, + messageId: isTypeExport + ? 'multipleExportsAreValues' + : 'multipleExportsAreTypes', + data: { exportNames }, + *fix(fixer) { + yield* fixSeparateNamedExports(fixer, sourceCode, report); + }, }); } - }, - - 'Program:exit'(): void { - for (const sourceExports of Object.values(sourceExportsMap)) { - // If this export has no issues, move on. - if (sourceExports.reportValueExports.length === 0) { - continue; - } - - for (const report of sourceExports.reportValueExports) { - if (!report.valueSpecifiers.length) { - // export is all type-only; convert the entire export to `export type` - context.report({ - node: report.node, - messageId: 'typeOverValue', - *fix(fixer) { - yield* fixExportInsertType( - fixer, - sourceCode, - report.node, - ); - }, - }); - } else if (!report.typeSpecifiers.length) { - // we only have value violations; remove the `type` from the export - // TODO: remove the `type` from the export - context.report({ - node: report.node, - messageId: 'valueOverType', - *fix(fixer) { - yield* fixExportRemoveType( - fixer, - sourceCode, - report.node, - ); - }, - }); - } else { - // We have both type and value violations. - const isTypeExport = report.node.exportKind === 'type'; - const allExportNames = ( - isTypeExport - ? report.valueSpecifiers - : report.typeSpecifiers - ).map(specifier => `${specifier.local.name}`); - - if (allExportNames.length === 1) { - const exportNames = allExportNames[0]; - - context.report({ - node: report.node, - messageId: isTypeExport - ? 'singleExportIsValue' - : 'singleExportIsType', - data: { exportNames }, - *fix(fixer) { - yield* fixSeparateNamedExports( - fixer, - sourceCode, - report, - ); - }, - }); - } else { - const exportNames = [ - allExportNames.slice(0, -1).join(', '), - allExportNames.slice(-1)[0], - ].join(' and '); - - context.report({ - node: report.node, - messageId: isTypeExport - ? 'multipleExportsAreValues' - : 'multipleExportsAreTypes', - data: { exportNames }, - *fix(fixer) { - yield* fixSeparateNamedExports( - fixer, - sourceCode, - report, - ); - }, - }); - } - } - } - } - }, + } } - : {}), + } + }, }; }, }); From 4e62dd639555efea014e7b6176cc904cbc029de7 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 28 Sep 2021 13:56:44 -0700 Subject: [PATCH 08/16] fix: handle the case where the exports have no source --- .../src/rules/consistent-type-exports.ts | 34 +++++++++++------- .../rules/consistent-type-exports.test.ts | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+), 12 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index df2ff0c3aa8..25250ebeabb 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -75,15 +75,8 @@ export default util.createRule({ return { ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration): void { - // Coerce the node.source.value to a string via asserting. - if ( - node.source?.type !== AST_NODE_TYPES.Literal || - typeof node.source?.value !== 'string' - ) { - return; - } - - const source = node.source.value; + // Coerce the source into a string for use as a lookup entry. + const source = getSourceFromExport(node) ?? 'undefined'; const sourceExports = (sourceExportsMap[source] = sourceExportsMap[ source ] || { @@ -286,6 +279,7 @@ function* fixSeparateNamedExports( report: ReportValueExport, ): IterableIterator { const { node, typeSpecifiers, valueSpecifiers } = report; + const source = getSourceFromExport(node); const separateTypes = node.exportKind !== 'type'; const specifiersToSeparate = separateTypes ? typeSpecifiers : valueSpecifiers; const specifierNames = specifiersToSeparate.map(getSpecifierText).join(', '); @@ -319,12 +313,28 @@ function* fixSeparateNamedExports( // Insert the bad exports into a new export line above. yield fixer.insertTextBefore( exportToken, - `export ${ - separateTypes ? 'type ' : '' - }{ ${specifierNames} } from ${sourceCode.getText(node.source!)};\n`, + `export ${separateTypes ? 'type ' : ''}{ ${specifierNames} }${ + source ? ` from '${source}'` : '' + };\n`, ); } +/** + * Returns the source of the export, or undefined if the named export has no source. + */ +function getSourceFromExport( + node: TSESTree.ExportNamedDeclaration, +): string | undefined { + if ( + node.source?.type === AST_NODE_TYPES.Literal && + typeof node.source.value === 'string' + ) { + return node.source.value; + } + + return undefined; +} + function getSpecifierText(specifier: TSESTree.ExportSpecifier): string { return `${specifier.local.name}${ specifier.exported.name !== specifier.local.name 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 1daf93b1a67..95eee66e7e7 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -212,5 +212,40 @@ export { BlockScope as BScope, CatchScope as CScope } from '@typescript-eslint/s }, ], }, + { + code: ` +import { Definition } from '@typescript-eslint/scope-manager'; +export { Definition }; + `, + output: ` +import { Definition } from '@typescript-eslint/scope-manager'; +export type { Definition }; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 3, + column: 1, + }, + ], + }, + { + code: ` +import { CatchScope, Definition } from '@typescript-eslint/scope-manager'; +export { CatchScope, Definition }; + `, + output: ` +import { CatchScope, Definition } from '@typescript-eslint/scope-manager'; +export type { Definition }; +export { CatchScope }; + `, + errors: [ + { + messageId: 'singleExportIsType', + line: 3, + column: 1, + }, + ], + }, ], }); From 7a0e55d15efd6acafcf0e759adaca3820cc3d2e7 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 19 Oct 2021 15:26:24 -0700 Subject: [PATCH 09/16] fix: addressing PR feedback --- .../eslint-plugin/docs/rules/consistent-type-exports.md | 7 ++++--- .../eslint-plugin/src/rules/consistent-type-exports.ts | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/consistent-type-exports.md b/packages/eslint-plugin/docs/rules/consistent-type-exports.md index c3f10f8e6c3..840d533799b 100644 --- a/packages/eslint-plugin/docs/rules/consistent-type-exports.md +++ b/packages/eslint-plugin/docs/rules/consistent-type-exports.md @@ -7,7 +7,7 @@ transpilers to drop exports without knowing the types of the dependencies. ## Rule Details -This rule aims to standardize the use of type exports style across the codebase. +This rule aims to standardize the use of type exports style across a codebase. Given a class `Button`, and an interface `ButtonProps`, examples of **correct** code: @@ -19,9 +19,10 @@ export type { ButtonProps } from 'some-library'; Examples of **incorrect** code: ```ts -import { Button, ButtonProps } from 'some-library'; +export { Button, ButtonProps } from 'some-library'; ``` ## When Not To Use It -- If you are not using TypeScript 3.8 (or greater), then you will not be able to use this rule, as type-only imports are not allowed. +- If you are using a TypeScript version less than 3.8, then you will not be able to use this rule as type exports are not supported. +- If you specifically want to use both export kinds for stylistic reasons, you can disable this rule. diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 25250ebeabb..fbef0afc8e8 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -70,8 +70,8 @@ export default util.createRule({ create(context) { const sourceCode = context.getSourceCode(); - // const sourceCode = context.getSourceCode(); const sourceExportsMap: { [key: string]: SourceExports } = {}; + const parserServices = util.getParserServices(context); return { ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration): void { @@ -85,7 +85,6 @@ export default util.createRule({ typeOnlyNamedExport: null, valueOnlyNamedExport: null, }); - const parserServices = util.getParserServices(context); // Cache the first encountered exports for the package. We will need to come // back to these later when fixing the problems. From 5b018549b1deeaf49bcbd9830b2c9a564529b793 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 19 Oct 2021 15:40:44 -0700 Subject: [PATCH 10/16] fix: updated type export rule metadata with doc metadata changes (removing category) --- packages/eslint-plugin/src/rules/consistent-type-exports.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index fbef0afc8e8..1c142a98e0e 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -37,7 +37,6 @@ export default util.createRule({ type: 'suggestion', docs: { description: 'Enforces consistent usage of type exports', - category: 'Stylistic Issues', recommended: false, requiresTypeChecking: true, }, From 34b1e877fccd53c9b1e8c944f1b194a6ebdd4c25 Mon Sep 17 00:00:00 2001 From: dzearing Date: Tue, 19 Oct 2021 16:06:15 -0700 Subject: [PATCH 11/16] fix: addressing more pr feedback - moving word list formatting to a utility, minor nit cleanup --- .../src/rules/consistent-type-exports.ts | 11 +++-------- .../src/rules/consistent-type-imports.ts | 7 ++----- packages/eslint-plugin/src/util/misc.ts | 19 +++++++++++++++++++ 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 1c142a98e0e..463500404f0 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -18,7 +18,7 @@ interface SourceExports { interface ReportValueExport { node: TSESTree.ExportNamedDeclaration; - typeSpecifiers: TSESTree.ExportSpecifier[]; // It has at least one element. + typeSpecifiers: TSESTree.ExportSpecifier[]; valueSpecifiers: TSESTree.ExportSpecifier[]; } @@ -76,9 +76,7 @@ export default util.createRule({ ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration): void { // Coerce the source into a string for use as a lookup entry. const source = getSourceFromExport(node) ?? 'undefined'; - const sourceExports = (sourceExportsMap[source] = sourceExportsMap[ - source - ] || { + const sourceExports = (sourceExportsMap[source] ||= { source, reportValueExports: [], typeOnlyNamedExport: null, @@ -172,10 +170,7 @@ export default util.createRule({ }, }); } else { - const exportNames = [ - allExportNames.slice(0, -1).join(', '), - allExportNames.slice(-1)[0], - ].join(' and '); + const exportNames = util.formatWordList(allExportNames); context.report({ node: report.node, diff --git a/packages/eslint-plugin/src/rules/consistent-type-imports.ts b/packages/eslint-plugin/src/rules/consistent-type-imports.ts index abefc9a4a8a..d549ab8f728 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-imports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-imports.ts @@ -264,8 +264,9 @@ export default util.createRule({ messageId: MessageIds; data: Record; } => { + const typeImports = util.formatWordList(importNames); + if (importNames.length === 1) { - const typeImports = importNames[0]; if (isTypeImport) { return { messageId: 'aImportInDecoMeta', @@ -278,10 +279,6 @@ export default util.createRule({ }; } } else { - const typeImports = [ - importNames.slice(0, -1).join(', '), - importNames.slice(-1)[0], - ].join(' and '); if (isTypeImport) { return { messageId: 'someImportsInDecoMeta', diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index 95fa4a359c6..da4bb7584cc 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -133,11 +133,30 @@ function getEnumNames(myEnum: Record): T[] { return Object.keys(myEnum).filter(x => isNaN(parseInt(x))) as T[]; } +/** + * Given an array of words, returns an English-friendly concatenation, separated with commas, with + * the `and` clause inserted before the last item. + * + * Example: ['foo', 'bar', 'baz' ] returns the string "foo, bar, and baz". + */ +function formatWordList(words: string[]): string { + if (!words || !words.length) { + return ''; + } + + if (words.length === 1) { + return words[0]; + } + + return [words.slice(0, -1).join(', '), words.slice(-1)[0]].join(' and '); +} + export { arraysAreEqual, Equal, ExcludeKeys, findFirstResult, + formatWordList, getEnumNames, getNameFromIndexSignature, getNameFromMember, From 82f85565a5d2a9aa38200c068ac7e78bfb198432 Mon Sep 17 00:00:00 2001 From: dzearing Date: Wed, 20 Oct 2021 11:31:03 -0700 Subject: [PATCH 12/16] fix: updating documentation with attribute info --- .../eslint-plugin/docs/rules/consistent-type-exports.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/eslint-plugin/docs/rules/consistent-type-exports.md b/packages/eslint-plugin/docs/rules/consistent-type-exports.md index 840d533799b..cb3b01f51df 100644 --- a/packages/eslint-plugin/docs/rules/consistent-type-exports.md +++ b/packages/eslint-plugin/docs/rules/consistent-type-exports.md @@ -26,3 +26,9 @@ export { Button, ButtonProps } from 'some-library'; - If you are using a TypeScript version less than 3.8, then you will not be able to use this rule as type exports are not supported. - If you specifically want to use both export kinds for stylistic reasons, you can disable this rule. + +## Attributes + +- [ ] ✅ Recommended +- [x] 🔧 Fixable +- [x] 💭 Requires type information From 25f7a22670153a2422323d11be7a9bbc0d8d74c2 Mon Sep 17 00:00:00 2001 From: dzearing Date: Wed, 20 Oct 2021 14:59:21 -0700 Subject: [PATCH 13/16] fix: adding utility unit test --- packages/eslint-plugin/src/util/misc.ts | 2 +- .../eslint-plugin/tests/util/misc.test.ts | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 packages/eslint-plugin/tests/util/misc.test.ts diff --git a/packages/eslint-plugin/src/util/misc.ts b/packages/eslint-plugin/src/util/misc.ts index da4bb7584cc..052e7bfd06c 100644 --- a/packages/eslint-plugin/src/util/misc.ts +++ b/packages/eslint-plugin/src/util/misc.ts @@ -140,7 +140,7 @@ function getEnumNames(myEnum: Record): T[] { * Example: ['foo', 'bar', 'baz' ] returns the string "foo, bar, and baz". */ function formatWordList(words: string[]): string { - if (!words || !words.length) { + if (!words?.length) { return ''; } diff --git a/packages/eslint-plugin/tests/util/misc.test.ts b/packages/eslint-plugin/tests/util/misc.test.ts new file mode 100644 index 00000000000..fbf31b7a03b --- /dev/null +++ b/packages/eslint-plugin/tests/util/misc.test.ts @@ -0,0 +1,27 @@ +import * as misc from '../../src/util/misc'; + +describe('formatWordList', () => { + it('can format with no words', () => { + expect(misc.formatWordList([])).toEqual(''); + }); + + it('can format with 1 word', () => { + expect(misc.formatWordList(['foo'])).toEqual('foo'); + }); + + it('can format with 2 words', () => { + expect(misc.formatWordList(['foo', 'bar'])).toEqual('foo and bar'); + }); + + it('can format with 3 words', () => { + expect(misc.formatWordList(['foo', 'bar', 'baz'])).toEqual( + 'foo, bar and baz', + ); + }); + + it('can format with 4 words', () => { + expect(misc.formatWordList(['foo', 'bar', 'baz', 'boz'])).toEqual( + 'foo, bar, baz and boz', + ); + }); +}); From 6285351e2f538fb957908f0676e37f23ddc32b2a Mon Sep 17 00:00:00 2001 From: dzearing Date: Fri, 22 Oct 2021 12:41:31 -0700 Subject: [PATCH 14/16] fix: added 4 more tests, removed schema, tweaked some asserts --- .../src/rules/consistent-type-exports.ts | 12 +-- .../rules/consistent-type-exports.test.ts | 92 +++++++++++++++++++ .../eslint-plugin/tests/util/misc.test.ts | 12 +-- 3 files changed, 98 insertions(+), 18 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 463500404f0..3750667411f 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -53,17 +53,7 @@ export default util.createRule({ multipleExportsAreValues: 'Value exports {{exportNames}} are exported as types only and should be exported using `export`.', }, - schema: [ - { - type: 'object', - properties: { - prefer: { - enum: ['type-exports', 'no-type-exports'], - }, - }, - additionalProperties: false, - }, - ], + schema: [], fixable: 'code', }, 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 95eee66e7e7..531f0c25a82 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -18,6 +18,26 @@ ruleTester.run('consistent-type-exports', rule, { "export { Foo } from 'foo';", "export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';", "export { BlockScope } from '@typescript-eslint/experimental-utils';", + ` +const variable = 1; +class Class {} +enum Enum {} +function Func() {} +namespace ValueNS { + export const x = 1; +} + +export { variable, Class, Enum, Func, ValueNS }; + `, + ` +type Alias = 1; +interface IFace {} +namespace TypeNS { + export type x = 1; +} + +export type { Alias, IFace, TypeNS }; + `, ], invalid: [ { @@ -247,5 +267,77 @@ export { CatchScope }; }, ], }, + { + code: ` +const variable = 1; +class Class {} +enum Enum {} +function Func() {} +namespace ValueNS { + export const x = 1; +} + +export type { + // type-export the values + variable, + Class, + Enum, + Func, + ValueNS, +}; + `, + output: ` +const variable = 1; +class Class {} +enum Enum {} +function Func() {} +namespace ValueNS { + export const x = 1; +} + +export { + // type-export the values + variable, + Class, + Enum, + Func, + ValueNS, +}; + `, + errors: [ + { + messageId: 'valueOverType', + line: 10, + column: 1, + }, + ], + }, + { + code: ` +type Alias = 1; +interface IFace {} +namespace TypeNS { + export type x = 1; +} + +export { Alias, IFace, TypeNS }; + `, + output: ` +type Alias = 1; +interface IFace {} +namespace TypeNS { + export type x = 1; +} + +export type { Alias, IFace, TypeNS }; + `, + errors: [ + { + messageId: 'typeOverValue', + line: 8, + column: 1, + }, + ], + }, ], }); diff --git a/packages/eslint-plugin/tests/util/misc.test.ts b/packages/eslint-plugin/tests/util/misc.test.ts index fbf31b7a03b..c4291f00d9f 100644 --- a/packages/eslint-plugin/tests/util/misc.test.ts +++ b/packages/eslint-plugin/tests/util/misc.test.ts @@ -2,25 +2,23 @@ import * as misc from '../../src/util/misc'; describe('formatWordList', () => { it('can format with no words', () => { - expect(misc.formatWordList([])).toEqual(''); + expect(misc.formatWordList([])).toBe(''); }); it('can format with 1 word', () => { - expect(misc.formatWordList(['foo'])).toEqual('foo'); + expect(misc.formatWordList(['foo'])).toBe('foo'); }); it('can format with 2 words', () => { - expect(misc.formatWordList(['foo', 'bar'])).toEqual('foo and bar'); + expect(misc.formatWordList(['foo', 'bar'])).toBe('foo and bar'); }); it('can format with 3 words', () => { - expect(misc.formatWordList(['foo', 'bar', 'baz'])).toEqual( - 'foo, bar and baz', - ); + expect(misc.formatWordList(['foo', 'bar', 'baz'])).toBe('foo, bar and baz'); }); it('can format with 4 words', () => { - expect(misc.formatWordList(['foo', 'bar', 'baz', 'boz'])).toEqual( + expect(misc.formatWordList(['foo', 'bar', 'baz', 'boz'])).toBe( 'foo, bar, baz and boz', ); }); From 85e6a2aea173d8a4f8c4c1106342b5cb0a4f74e6 Mon Sep 17 00:00:00 2001 From: dzearing Date: Fri, 22 Oct 2021 15:47:58 -0700 Subject: [PATCH 15/16] fix: updating rule to only evaluate non-type exports for types being exported --- .../src/rules/consistent-type-exports.ts | 90 ++++--------- .../rules/consistent-type-exports.test.ts | 123 +----------------- 2 files changed, 30 insertions(+), 183 deletions(-) diff --git a/packages/eslint-plugin/src/rules/consistent-type-exports.ts b/packages/eslint-plugin/src/rules/consistent-type-exports.ts index 3750667411f..98e6bd355e4 100644 --- a/packages/eslint-plugin/src/rules/consistent-type-exports.ts +++ b/packages/eslint-plugin/src/rules/consistent-type-exports.ts @@ -24,11 +24,8 @@ interface ReportValueExport { type MessageIds = | 'typeOverValue' - | 'valueOverType' | 'singleExportIsType' - | 'multipleExportsAreTypes' - | 'singleExportIsValue' - | 'multipleExportsAreValues'; + | 'multipleExportsAreTypes'; export default util.createRule({ name: 'consistent-type-exports', @@ -43,15 +40,11 @@ export default util.createRule({ messages: { typeOverValue: 'All exports in the declaration are only used as types. Use `export type`.', - valueOverType: 'Use an `export` instead of an `export type`.', + singleExportIsType: 'Type export {{exportNames}} is not a value and should be exported using `export type`.', multipleExportsAreTypes: 'Type exports {{exportNames}} are not values and should be exported using `export type`.', - singleExportIsValue: - 'Value export {{exportNames}} is exported as a type only and should be exported using `export`.', - multipleExportsAreValues: - 'Value exports {{exportNames}} are exported as types only and should be exported using `export`.', }, schema: [], fixable: 'code', @@ -85,18 +78,22 @@ export default util.createRule({ sourceExports.valueOnlyNamedExport = node; } - // Next for the current import, we will separate type/value specifiers. + // Next for the current export, we will separate type/value specifiers. const typeSpecifiers: TSESTree.ExportSpecifier[] = []; const valueSpecifiers: TSESTree.ExportSpecifier[] = []; - for (const specifier of node.specifiers) { - const isTypeBased = isSpecifierTypeBased(parserServices, specifier); - - if (isTypeBased === true) { - typeSpecifiers.push(specifier); - } else if (isTypeBased === false) { - // undefined means we don't know. - valueSpecifiers.push(specifier); + // 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) { + const isTypeBased = isSpecifierTypeBased(parserServices, specifier); + + if (isTypeBased === true) { + typeSpecifiers.push(specifier); + } else if (isTypeBased === false) { + // When isTypeBased is undefined, we should avoid reporting them. + valueSpecifiers.push(specifier); + } } } @@ -121,7 +118,7 @@ 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` + // Export is all type-only; convert the entire export to `export type`. context.report({ node: report.node, messageId: 'typeOverValue', @@ -129,31 +126,18 @@ export default util.createRule({ yield* fixExportInsertType(fixer, sourceCode, report.node); }, }); - } else if (!report.typeSpecifiers.length) { - // we only have value violations; remove the `type` from the export - // TODO: remove the `type` from the export - context.report({ - node: report.node, - messageId: 'valueOverType', - *fix(fixer) { - yield* fixExportRemoveType(fixer, sourceCode, report.node); - }, - }); } else { // We have both type and value violations. - const isTypeExport = report.node.exportKind === 'type'; - const allExportNames = ( - isTypeExport ? report.valueSpecifiers : report.typeSpecifiers - ).map(specifier => `${specifier.local.name}`); + const allExportNames = report.typeSpecifiers.map( + specifier => `${specifier.local.name}`, + ); if (allExportNames.length === 1) { const exportNames = allExportNames[0]; context.report({ node: report.node, - messageId: isTypeExport - ? 'singleExportIsValue' - : 'singleExportIsType', + messageId: 'singleExportIsType', data: { exportNames }, *fix(fixer) { yield* fixSeparateNamedExports(fixer, sourceCode, report); @@ -164,9 +148,7 @@ export default util.createRule({ context.report({ node: report.node, - messageId: isTypeExport - ? 'multipleExportsAreValues' - : 'multipleExportsAreTypes', + messageId: 'multipleExportsAreTypes', data: { exportNames }, *fix(fixer) { yield* fixSeparateNamedExports(fixer, sourceCode, report); @@ -225,32 +207,6 @@ function* fixExportInsertType( yield fixer.insertTextAfter(exportToken, ' type'); } -/** - * Removes "type" from an export. - * - * Example: - * - * export type { Foo } from 'foo'; - * ^^^^ - */ -function* fixExportRemoveType( - fixer: TSESLint.RuleFixer, - sourceCode: Readonly, - node: TSESTree.ExportNamedDeclaration, -): IterableIterator { - const exportToken = util.nullThrows( - sourceCode.getFirstToken(node), - util.NullThrowsReasons.MissingToken('export', node.type), - ); - - const typeToken = util.nullThrows( - sourceCode.getTokenAfter(exportToken), - util.NullThrowsReasons.MissingToken('type', node.type), - ); - - yield fixer.removeRange([exportToken.range[1], typeToken.range[1]]); -} - /** * Separates the exports which mismatch the kind of export the given * node represents. For example, a type export's named specifiers which @@ -318,6 +274,10 @@ function getSourceFromExport( return undefined; } +/** + * Returns the specifier text for the export. If it is aliased, we take care to return + * the proper formatting. + */ function getSpecifierText(specifier: TSESTree.ExportSpecifier): string { return `${specifier.local.name}${ specifier.exported.name !== specifier.local.name 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 531f0c25a82..d97f03647ca 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -18,6 +18,7 @@ ruleTester.run('consistent-type-exports', rule, { "export { Foo } from 'foo';", "export type { AnalyzeOptions } from '@typescript-eslint/scope-manager';", "export { BlockScope } from '@typescript-eslint/experimental-utils';", + "export type { BlockScope } from '@typescript-eslint/experimental-utils';", ` const variable = 1; class Class {} @@ -38,6 +39,10 @@ namespace TypeNS { export type { Alias, IFace, TypeNS }; `, + ` +const foo = 1; +export type { foo }; + `, ], invalid: [ { @@ -52,17 +57,6 @@ export type { Alias, IFace, TypeNS }; }, ], }, - { - code: "export type { BlockScope } from '@typescript-eslint/scope-manager';", - output: "export { BlockScope } from '@typescript-eslint/scope-manager';", - errors: [ - { - messageId: 'valueOverType', - line: 1, - column: 1, - }, - ], - }, { code: "export { AnalyzeOptions, BlockScope } from '@typescript-eslint/scope-manager';", output: @@ -118,68 +112,6 @@ export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; }, ], }, - { - code: ` -export type { - AnalyzeOptions, - BlockScope, -} from '@typescript-eslint/scope-manager'; - `, - output: ` -export { BlockScope } from '@typescript-eslint/scope-manager'; -export type { AnalyzeOptions } from '@typescript-eslint/scope-manager'; - `, - errors: [ - { - messageId: 'singleExportIsValue', - line: 2, - column: 1, - }, - ], - }, - { - code: ` -export type { - AnalyzeOptions, - BlockScope, - Definition, -} from '@typescript-eslint/scope-manager'; - `, - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - output: ` -export { BlockScope } from '@typescript-eslint/scope-manager'; -export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; - `, - errors: [ - { - messageId: 'singleExportIsValue', - line: 2, - column: 1, - }, - ], - }, - { - code: ` -export type { - AnalyzeOptions, - BlockScope, - Definition, - CatchScope, -} from '@typescript-eslint/scope-manager'; - `, - // eslint-disable-next-line @typescript-eslint/internal/plugin-test-formatting - output: ` -export { BlockScope, CatchScope } from '@typescript-eslint/scope-manager'; -export type { AnalyzeOptions, Definition } from '@typescript-eslint/scope-manager'; - `, - errors: [ - { - messageId: 'multipleExportsAreValues', - line: 2, - column: 1, - }, - ], - }, { code: "export { Definition as Foo } from '@typescript-eslint/scope-manager';", output: @@ -269,51 +201,6 @@ export { CatchScope }; }, { code: ` -const variable = 1; -class Class {} -enum Enum {} -function Func() {} -namespace ValueNS { - export const x = 1; -} - -export type { - // type-export the values - variable, - Class, - Enum, - Func, - ValueNS, -}; - `, - output: ` -const variable = 1; -class Class {} -enum Enum {} -function Func() {} -namespace ValueNS { - export const x = 1; -} - -export { - // type-export the values - variable, - Class, - Enum, - Func, - ValueNS, -}; - `, - errors: [ - { - messageId: 'valueOverType', - line: 10, - column: 1, - }, - ], - }, - { - code: ` type Alias = 1; interface IFace {} namespace TypeNS { From 13422d8c3e4c9e82c4d2e57b6441192f6a17381f Mon Sep 17 00:00:00 2001 From: dzearing Date: Fri, 22 Oct 2021 16:13:50 -0700 Subject: [PATCH 16/16] fix: updating test cases where the namespace contains mixed value/types, and also where namespace is type only but exported as a non type --- .../rules/consistent-type-exports.test.ts | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) 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 d97f03647ca..301d70e54dc 100644 --- a/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts +++ b/packages/eslint-plugin/tests/rules/consistent-type-exports.test.ts @@ -43,6 +43,13 @@ export type { Alias, IFace, TypeNS }; const foo = 1; export type { foo }; `, + ` +namespace NonTypeNS { + export const x = 1; +} + +export { NonTypeNS }; + `, ], invalid: [ { @@ -205,6 +212,7 @@ type Alias = 1; interface IFace {} namespace TypeNS { export type x = 1; + export const f = 1; } export { Alias, IFace, TypeNS }; @@ -214,14 +222,39 @@ type Alias = 1; interface IFace {} namespace TypeNS { export type x = 1; + export const f = 1; } -export type { Alias, IFace, TypeNS }; +export type { Alias, IFace }; +export { TypeNS }; + `, + errors: [ + { + messageId: 'multipleExportsAreTypes', + line: 9, + column: 1, + }, + ], + }, + { + code: ` +namespace TypeNS { + export interface Foo {} +} + +export { TypeNS }; + `, + output: ` +namespace TypeNS { + export interface Foo {} +} + +export type { TypeNS }; `, errors: [ { messageId: 'typeOverValue', - line: 8, + line: 6, column: 1, }, ],