Skip to content

Commit

Permalink
[New] consistent-type-specifier-style: add rule
Browse files Browse the repository at this point in the history
  • Loading branch information
bradzacher authored and ljharb committed Jun 9, 2022
1 parent 395e26b commit 591c83d
Show file tree
Hide file tree
Showing 9 changed files with 730 additions and 4 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
## [Unreleased]

### Added
- [`consistent-type-specifier-style`]: add rule ([#2473], thanks [@bradzacher])
- [`newline-after-import`]: add `considerComments` option ([#2399], thanks [@pri1311])
- [`no-cycle`]: add `allowUnsafeDynamicCyclicDependency` option ([#2387], thanks [@GerkinDev])
- [`no-restricted-paths`]: support arrays for `from` and `target` options ([#2466], thanks [@AdriAt360])
Expand Down Expand Up @@ -964,6 +965,7 @@ for info on changes for earlier releases.
[`import/external-module-folders` setting]: ./README.md#importexternal-module-folders
[`internal-regex` setting]: ./README.md#importinternal-regex

[`consistent-type-specifier-style`]: ./docs/rules/consistent-type-specifier-style.md
[`default`]: ./docs/rules/default.md
[`dynamic-import-chunkname`]: ./docs/rules/dynamic-import-chunkname.md
[`export`]: ./docs/rules/export.md
Expand Down Expand Up @@ -1016,6 +1018,7 @@ for info on changes for earlier releases.
[#2506]: https://github.com/import-js/eslint-plugin-import/pull/2506
[#2503]: https://github.com/import-js/eslint-plugin-import/pull/2503
[#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490
[#2473]: https://github.com/import-js/eslint-plugin-import/pull/2473
[#2466]: https://github.com/import-js/eslint-plugin-import/pull/2466
[#2440]: https://github.com/import-js/eslint-plugin-import/pull/2440
[#2438]: https://github.com/import-js/eslint-plugin-import/pull/2438
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
* Forbid anonymous values as default exports ([`no-anonymous-default-export`])
* Prefer named exports to be grouped together in a single export declaration ([`group-exports`])
* Enforce a leading comment with the webpackChunkName for dynamic imports ([`dynamic-import-chunkname`])
* Enforce or ban the use of inline type-only markers for named imports ([`consistent-type-specifier-style`])

[`first`]: ./docs/rules/first.md
[`exports-last`]: ./docs/rules/exports-last.md
Expand All @@ -114,6 +115,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
[`no-default-export`]: ./docs/rules/no-default-export.md
[`no-named-export`]: ./docs/rules/no-named-export.md
[`dynamic-import-chunkname`]: ./docs/rules/dynamic-import-chunkname.md
[`consistent-type-specifier-style`]: ./docs/rules/consistent-type-specifier-style.md

## `eslint-plugin-import` for enterprise

Expand Down
87 changes: 87 additions & 0 deletions docs/rules/consistent-type-specifier-style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
# import/consistent-type-specifier-style

In both Flow and TypeScript you can mark an import as a type-only import by adding a "kind" marker to the import. Both languages support two positions for marker.

**At the top-level** which marks all names in the import as type-only and applies to named, default, and namespace (for TypeScript) specifiers:

```ts
import type Foo from 'Foo';
import type {Bar} from 'Bar';
// ts only
import type * as Bam from 'Bam';
// flow only
import typeof Baz from 'Baz';
```

**Inline** with to the named import, which marks just the specific name in the import as type-only. An inline specifier is only valid for named specifiers, and not for default or namespace specifiers:

```ts
import {type Foo} from 'Foo';
// flow only
import {typeof Bar} from 'Bar';
```

## Rule Details

This rule either enforces or bans the use of inline type-only markers for named imports.

This rule includes a fixer that will automatically convert your specifiers to the correct form - however the fixer will not respect your preferences around de-duplicating imports. If this is important to you, consider using the [`import/no-duplicates`] rule.

[`import/no-duplicates`]: ./no-duplicates.md

## Options

The rule accepts a single string option which may be one of:

- `'prefer-inline'` - enforces that named type-only specifiers are only ever written with an inline marker; and never as part of a top-level, type-only import.
- `'prefer-top-level'` - enforces that named type-only specifiers only ever written as part of a top-level, type-only import; and never with an inline marker.

By default the rule will use the `prefer-inline` option.

## Examples

### `prefer-top-level`

❌ Invalid with `["error", "prefer-top-level"]`

```ts
import {type Foo} from 'Foo';
import Foo, {type Bar} from 'Foo';
// flow only
import {typeof Foo} from 'Foo';
```

✅ Valid with `["error", "prefer-top-level"]`

```ts
import type {Foo} from 'Foo';
import type Foo, {Bar} from 'Foo';
// flow only
import typeof {Foo} from 'Foo';
```

### `prefer-inline`

❌ Invalid with `["error", "prefer-inline"]`

```ts
import type {Foo} from 'Foo';
import type Foo, {Bar} from 'Foo';
// flow only
import typeof {Foo} from 'Foo';
```

✅ Valid with `["error", "prefer-inline"]`

```ts
import {type Foo} from 'Foo';
import Foo, {type Bar} from 'Foo';
// flow only
import {typeof Foo} from 'Foo';
```

## When Not To Use It

If you aren't using Flow or TypeScript 4.5+, then this rule does not apply and need not be used.

If you don't care about, and don't want to standardize how named specifiers are imported then you should not use this rule.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@
"safe-publish-latest": "^2.0.0",
"semver": "^6.3.0",
"sinon": "^2.4.1",
"typescript": "^2.8.1 || ~3.9.5",
"typescript": "^2.8.1 || ~3.9.5 || ~4.5.2",
"typescript-eslint-parser": "^15 || ^20 || ^22"
},
"peerDependencies": {
Expand Down
1 change: 1 addition & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export const rules = {
'group-exports': require('./rules/group-exports'),
'no-relative-packages': require('./rules/no-relative-packages'),
'no-relative-parent-imports': require('./rules/no-relative-parent-imports'),
'consistent-type-specifier-style': require('./rules/consistent-type-specifier-style'),

'no-self-import': require('./rules/no-self-import'),
'no-cycle': require('./rules/no-cycle'),
Expand Down
216 changes: 216 additions & 0 deletions src/rules/consistent-type-specifier-style.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
import docsUrl from '../docsUrl';

function isComma(token) {
return token.type === 'Punctuator' && token.value === ',';
}

function removeSpecifiers(fixes, fixer, sourceCode, specifiers) {
for (const specifier of specifiers) {
// remove the trailing comma
const comma = sourceCode.getTokenAfter(specifier, isComma);
if (comma) {
fixes.push(fixer.remove(comma));
}
fixes.push(fixer.remove(specifier));
}
}

module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'Enforce or ban the use of inline type-only markers for named imports',
url: docsUrl('consistent-type-specifier-style'),
},
fixable: 'code',
schema: [
{
type: 'string',
enum: ['prefer-inline', 'prefer-top-level'],
default: 'prefer-inline',
},
],
},

create(context) {
const sourceCode = context.getSourceCode();

if (context.options[0] === 'prefer-inline') {
return {
ImportDeclaration(node) {
if (node.importKind === 'value' || node.importKind == null) {
// top-level value / unknown is valid
return;
}

if (
// no specifiers (import type {} from '') have no specifiers to mark as inline
node.specifiers.length === 0 ||
(node.specifiers.length === 1 &&
// default imports are both "inline" and "top-level"
(node.specifiers[0].type === 'ImportDefaultSpecifier' ||
// namespace imports are both "inline" and "top-level"
node.specifiers[0].type === 'ImportNamespaceSpecifier'))
) {
return;
}

context.report({
node,
message: 'Prefer using inline {{kind}} specifiers instead of a top-level {{kind}}-only import.',
data: {
kind: node.importKind,
},
fix(fixer) {
const kindToken = sourceCode.getFirstToken(node, { skip: 1 });

return [].concat(
kindToken ? fixer.remove(kindToken) : [],
node.specifiers.map((specifier) => fixer.insertTextBefore(specifier, `${node.importKind} `)),
);
},
});
},
};
}

// prefer-top-level
return {
ImportDeclaration(node) {
if (
// already top-level is valid
node.importKind === 'type' ||
node.importKind === 'typeof' ||
// no specifiers (import {} from '') cannot have inline - so is valid
node.specifiers.length === 0 ||
(node.specifiers.length === 1 &&
// default imports are both "inline" and "top-level"
(node.specifiers[0].type === 'ImportDefaultSpecifier' ||
// namespace imports are both "inline" and "top-level"
node.specifiers[0].type === 'ImportNamespaceSpecifier'))
) {
return;
}

const typeSpecifiers = [];
const typeofSpecifiers = [];
const valueSpecifiers = [];
let defaultSpecifier = null;
for (const specifier of node.specifiers) {
if (specifier.type === 'ImportDefaultSpecifier') {
defaultSpecifier = specifier;
continue;
} else if (specifier.type !== 'ImportSpecifier') {
continue;
}

if (specifier.importKind === 'type') {
typeSpecifiers.push(specifier);
} else if (specifier.importKind === 'typeof') {
typeofSpecifiers.push(specifier);
} else if (specifier.importKind === 'value' || specifier.importKind == null) {
valueSpecifiers.push(specifier);
}
}

const typeImport = getImportText(typeSpecifiers, 'type');
const typeofImport = getImportText(typeofSpecifiers, 'typeof');
const newImports = `${typeImport}\n${typeofImport}`.trim();

if (typeSpecifiers.length + typeofSpecifiers.length === node.specifiers.length) {
// all specifiers have inline specifiers - so we replace the entire import
const kind = [].concat(
typeSpecifiers.length > 0 ? 'type' : [],
typeofSpecifiers.length > 0 ? 'typeof' : [],
);

context.report({
node,
message: 'Prefer using a top-level {{kind}}-only import instead of inline {{kind}} specifiers.',
data: {
kind: kind.join('/'),
},
fix(fixer) {
return fixer.replaceText(node, newImports);
},
});
} else {
// remove specific specifiers and insert new imports for them
for (const specifier of typeSpecifiers.concat(typeofSpecifiers)) {
context.report({
node: specifier,
message: 'Prefer using a top-level {{kind}}-only import instead of inline {{kind}} specifiers.',
data: {
kind: specifier.importKind,
},
fix(fixer) {
const fixes = [];

// if there are no value specifiers, then the other report fixer will be called, not this one

if (valueSpecifiers.length > 0) {
// import { Value, type Type } from 'mod';

// we can just remove the type specifiers
removeSpecifiers(fixes, fixer, sourceCode, typeSpecifiers);
removeSpecifiers(fixes, fixer, sourceCode, typeofSpecifiers);

// make the import nicely formatted by also removing the trailing comma after the last value import
// eg
// import { Value, type Type } from 'mod';
// to
// import { Value } from 'mod';
// not
// import { Value, } from 'mod';
const maybeComma = sourceCode.getTokenAfter(valueSpecifiers[valueSpecifiers.length - 1]);
if (isComma(maybeComma)) {
fixes.push(fixer.remove(maybeComma));
}
} else if (defaultSpecifier) {
// import Default, { type Type } from 'mod';

// remove the entire curly block so we don't leave an empty one behind
// NOTE - the default specifier *must* be the first specifier always!
// so a comma exists that we also have to clean up or else it's bad syntax
const comma = sourceCode.getTokenAfter(defaultSpecifier, isComma);
const closingBrace = sourceCode.getTokenAfter(
node.specifiers[node.specifiers.length - 1],
token => token.type === 'Punctuator' && token.value === '}',
);
fixes.push(fixer.removeRange([
comma.range[0],
closingBrace.range[1],
]));
}

return fixes.concat(
// insert the new imports after the old declaration
fixer.insertTextAfter(node, `\n${newImports}`),
);
},
});
}
}

function getImportText(
specifiers,
kind,
) {
const sourceString = sourceCode.getText(node.source);
if (specifiers.length === 0) {
return '';
}

const names = specifiers.map(s => {
if (s.imported.name === s.local.name) {
return s.imported.name;
}
return `${s.imported.name} as ${s.local.name}`;
});
// insert a fresh top-level import
return `import ${kind} {${names.join(', ')}} from ${sourceString};`;
}
},
};
},
};
3 changes: 2 additions & 1 deletion tests/src/core/getExports.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import semver from 'semver';
import sinon from 'sinon';
import eslintPkg from 'eslint/package.json';
import typescriptPkg from 'typescript/package.json';
import * as tsConfigLoader from 'tsconfig-paths/lib/tsconfig-loader';
import ExportMap from '../../../src/ExportMap';

Expand Down Expand Up @@ -351,7 +352,7 @@ describe('ExportMap', function () {
configs.push(['array form', { '@typescript-eslint/parser': ['.ts', '.tsx'] }]);
}

if (semver.satisfies(eslintPkg.version, '<6')) {
if (semver.satisfies(eslintPkg.version, '<6') && semver.satisfies(typescriptPkg.version, '<4')) {
configs.push(['array form', { 'typescript-eslint-parser': ['.ts', '.tsx'] }]);
}

Expand Down
Loading

0 comments on commit 591c83d

Please sign in to comment.