Skip to content

Commit

Permalink
improve fixer to cleanup commas and empty braces
Browse files Browse the repository at this point in the history
  • Loading branch information
@bradzacher
bradzacher committed Sep 3, 2022
1 parent 56732f2 commit ac4960c
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 30 deletions.
103 changes: 78 additions & 25 deletions src/rules/consistent-type-specifier-style.js
View file
Expand Up @@ -28,9 +28,12 @@ module.exports = {
}

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;
Expand All @@ -42,16 +45,19 @@ module.exports = {
data: {
kind: node.importKind,
},
*fix(fixer) {
const kindToken = sourceCode.getFirstToken(node, { skip: 1 });
fix(fixer) {
const fixes = [];

const kindToken = sourceCode.getFirstToken(node, { skip: 1 });
if (kindToken) {
yield fixer.remove(kindToken);
fixes.push(fixer.remove(kindToken));
}

for (const specifier of node.specifiers) {
yield fixer.insertTextBefore(specifier, `${node.importKind} `);
fixes.push(fixer.insertTextBefore(specifier, `${node.importKind} `));
}

return fixes;
},
});
},
Expand All @@ -62,30 +68,38 @@ module.exports = {
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) {
// TODO - remove
if (specifier.type !== 'ImportSpecifier') {
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 {
continue;
} else if (specifier.importKind === 'value' || specifier.importKind == null) {
valueSpecifiers.push(specifier);
}
}

Expand All @@ -111,8 +125,8 @@ module.exports = {
data: {
kind: kind.join('/'),
},
*fix(fixer) {
yield fixer.replaceText(node, newImports);
fix(fixer) {
return fixer.replaceText(node, newImports);
},
});
} else {
Expand All @@ -124,25 +138,59 @@ module.exports = {
data: {
kind: specifier.importKind,
},
*fix(fixer) {
// remove just the specifiers and insert the new imports after
yield* removeSpecifiers(typeSpecifiers);
yield* removeSpecifiers(typeofSpecifiers);
yield fixer.insertTextAfter(node, `\n${newImports}`);

function* removeSpecifiers(
specifiers,
) {
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(typeSpecifiers);
removeSpecifiers(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],
]));
}

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

return fixes;

function removeSpecifiers(specifiers) {
for (const specifier of specifiers) {
// remove the trailing comma
const comma = sourceCode.getTokenAfter(
specifier,
token => token.type === 'Punctuator' && token.value === ',',
);
const comma = sourceCode.getTokenAfter(specifier, isComma);
if (comma) {
yield fixer.remove(comma);
fixes.push(fixer.remove(comma));
}
yield fixer.remove(specifier);
fixes.push(fixer.remove(specifier));
}
}
},
Expand Down Expand Up @@ -172,3 +220,8 @@ module.exports = {
};
},
};

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

10 changes: 5 additions & 5 deletions tests/src/rules/consistent-type-specifier-style.js
View file
Expand Up @@ -66,7 +66,7 @@ const COMMON_TESTS = {
},
{
code: "import { Foo, type Bar } from 'Foo';",
output: "import { Foo, } from 'Foo';\nimport type {Bar} from 'Foo';",
output: "import { Foo } from 'Foo';\nimport type {Bar} from 'Foo';",
options: ['prefer-top-level'],
errors: [{
message: 'Prefer using a top-level type-only import instead of inline type specifiers.',
Expand All @@ -84,7 +84,7 @@ const COMMON_TESTS = {
},
{
code: "import Foo, { type Bar } from 'Foo';",
output: "import Foo, { } from 'Foo';\nimport type {Bar} from 'Foo';",
output: "import Foo from 'Foo';\nimport type {Bar} from 'Foo';",
options: ['prefer-top-level'],
errors: [{
message: 'Prefer using a top-level type-only import instead of inline type specifiers.',
Expand Down Expand Up @@ -203,7 +203,7 @@ const FLOW_ONLY = {
},
{
code: "import { Foo, typeof Bar } from 'Foo';",
output: "import { Foo, } from 'Foo';\nimport typeof {Bar} from 'Foo';",
output: "import { Foo } from 'Foo';\nimport typeof {Bar} from 'Foo';",
options: ['prefer-top-level'],
errors: [{
message: 'Prefer using a top-level typeof-only import instead of inline typeof specifiers.',
Expand All @@ -221,7 +221,7 @@ const FLOW_ONLY = {
},
{
code: "import { Foo, type Bar, typeof Baz } from 'Foo';",
output: "import { Foo, } from 'Foo';\nimport type {Bar} from 'Foo';\nimport typeof {Baz} from 'Foo';",
output: "import { Foo } from 'Foo';\nimport type {Bar} from 'Foo';\nimport typeof {Baz} from 'Foo';",
options: ['prefer-top-level'],
errors: [
{
Expand All @@ -236,7 +236,7 @@ const FLOW_ONLY = {
},
{
code: "import Foo, { typeof Bar } from 'Foo';",
output: "import Foo, { } from 'Foo';\nimport typeof {Bar} from 'Foo';",
output: "import Foo from 'Foo';\nimport typeof {Bar} from 'Foo';",
options: ['prefer-top-level'],
errors: [{
message: 'Prefer using a top-level typeof-only import instead of inline typeof specifiers.',
Expand Down

0 comments on commit ac4960c

Please sign in to comment.