Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): adding consistent-type-exports rule #3936

Merged
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
4760ea5
initial seed of rule.
dzearing Sep 23, 2021
815efae
Getting closer!
dzearing Sep 25, 2021
582d49c
It works!
dzearing Sep 27, 2021
1b4880a
Adding a few tests, removing logging, adding rule to a..ts.
dzearing Sep 27, 2021
97bb777
chore: adding tests, ensuring alias exports work
dzearing Sep 28, 2021
71ce5fd
chore: factoring out a getSpecifierText helper, updating test formatting
dzearing Sep 28, 2021
a5dcc2b
chore: adding documentation
dzearing Sep 28, 2021
4e62dd6
fix: handle the case where the exports have no source
dzearing Sep 28, 2021
56a9251
Merge branch 'master' of https://github.com/typescript-eslint/typescr…
dzearing Sep 28, 2021
6c1eb38
Merge branch 'master' into feat/consistent-type-exports
dzearing Oct 8, 2021
4c9f746
Merge branch 'master' into feat/consistent-type-exports
dzearing Oct 13, 2021
358a882
Merge branch 'master' into feat/consistent-type-exports
dzearing Oct 19, 2021
7a0e55d
fix: addressing PR feedback
dzearing Oct 19, 2021
5b01854
fix: updated type export rule metadata with doc metadata changes (re…
dzearing Oct 19, 2021
34b1e87
fix: addressing more pr feedback - moving word list formatting to a u…
dzearing Oct 19, 2021
82f8556
fix: updating documentation with attribute info
dzearing Oct 20, 2021
27acd97
Merge branch 'master' into feat/consistent-type-exports
dzearing Oct 20, 2021
25f7a22
fix: adding utility unit test
dzearing Oct 20, 2021
e242915
Merge branch 'feat/consistent-type-exports' of https://github.com/dze…
dzearing Oct 20, 2021
6285351
fix: added 4 more tests, removed schema, tweaked some asserts
dzearing Oct 22, 2021
85e6a2a
fix: updating rule to only evaluate non-type exports for types being …
dzearing Oct 22, 2021
13422d8
fix: updating test cases where the namespace contains mixed value/typ…
dzearing Oct 22, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/eslint-plugin/README.md
Expand Up @@ -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: | |
Expand Down
34 changes: 34 additions & 0 deletions packages/eslint-plugin/docs/rules/consistent-type-exports.md
@@ -0,0 +1,34 @@
# 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 a 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
export { Button, ButtonProps } from 'some-library';
```

## When Not To Use It

- 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
1 change: 1 addition & 0 deletions packages/eslint-plugin/src/configs/all.ts
Expand Up @@ -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',
Expand Down
337 changes: 337 additions & 0 deletions packages/eslint-plugin/src/rules/consistent-type-exports.ts
@@ -0,0 +1,337 @@
import {
TSESTree,
ParserServices,
AST_NODE_TYPES,
TSESLint,
} from '@typescript-eslint/experimental-utils';
import { SymbolFlags } from 'typescript';
import * as util from '../util';

type Options = [];

interface SourceExports {
source: string;
reportValueExports: ReportValueExport[];
typeOnlyNamedExport: TSESTree.ExportNamedDeclaration | null;
valueOnlyNamedExport: TSESTree.ExportNamedDeclaration | null;
}

interface ReportValueExport {
node: TSESTree.ExportNamedDeclaration;
typeSpecifiers: TSESTree.ExportSpecifier[];
valueSpecifiers: TSESTree.ExportSpecifier[];
}

type MessageIds =
| 'typeOverValue'
| 'valueOverType'
| 'singleExportIsType'
| 'multipleExportsAreTypes'
| 'singleExportIsValue'
| 'multipleExportsAreValues';

export default util.createRule<Options, MessageIds>({
name: 'consistent-type-exports',
defaultOptions: [],
meta: {
type: 'suggestion',
docs: {
description: 'Enforces consistent usage of type exports',
recommended: false,
requiresTypeChecking: true,
},
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: [
{
type: 'object',
properties: {
prefer: {
enum: ['type-exports', 'no-type-exports'],
},
},
additionalProperties: false,
},
dzearing marked this conversation as resolved.
Show resolved Hide resolved
],
fixable: 'code',
},

create(context) {
const sourceCode = context.getSourceCode();
const sourceExportsMap: { [key: string]: SourceExports } = {};
const parserServices = util.getParserServices(context);

return {
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] ||= {
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[] = [];

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);
}
}
dzearing marked this conversation as resolved.
Show resolved Hide resolved

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)) {
// 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 = util.formatWordList(allExportNames);

context.report({
node: report.node,
messageId: isTypeExport
? 'multipleExportsAreValues'
: 'multipleExportsAreTypes',
data: { exportNames },
*fix(fixer) {
yield* fixSeparateNamedExports(fixer, sourceCode, report);
},
});
}
}
}
}
},
};
},
});

/**
* 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);
}

/**
* Inserts "type" into an export.
*
* Example:
*
* export type { Foo } from 'foo';
* ^^^^
*/
function* fixExportInsertType(
fixer: TSESLint.RuleFixer,
sourceCode: Readonly<TSESLint.SourceCode>,
node: TSESTree.ExportNamedDeclaration,
): IterableIterator<TSESLint.RuleFix> {
const exportToken = util.nullThrows(
sourceCode.getFirstToken(node),
util.NullThrowsReasons.MissingToken('export', node.type),
);

yield fixer.insertTextAfter(exportToken, ' type');
}

/**
* Removes "type" from an export.
*
* Example:
*
* export type { Foo } from 'foo';
* ^^^^
*/
function* fixExportRemoveType(
fixer: TSESLint.RuleFixer,
sourceCode: Readonly<TSESLint.SourceCode>,
node: TSESTree.ExportNamedDeclaration,
): IterableIterator<TSESLint.RuleFix> {
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
* represent values will be inserted in a separate `export` statement.
*/
function* fixSeparateNamedExports(
fixer: TSESLint.RuleFixer,
sourceCode: Readonly<TSESLint.SourceCode>,
report: ReportValueExport,
): IterableIterator<TSESLint.RuleFix> {
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(', ');

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(getSpecifierText)
.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),
);

// 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 above.
yield fixer.insertTextBefore(
exportToken,
`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'
dzearing marked this conversation as resolved.
Show resolved Hide resolved
) {
return node.source.value;
}

return undefined;
}

function getSpecifierText(specifier: TSESTree.ExportSpecifier): string {
return `${specifier.local.name}${
specifier.exported.name !== specifier.local.name
? ` as ${specifier.exported.name}`
: ''
}`;
}