From ac4960c5c1584480df640c9ab546a05be898ad51 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Sat, 3 Sep 2022 22:57:11 +0930 Subject: [PATCH] improve fixer to cleanup commas and empty braces --- src/rules/consistent-type-specifier-style.js | 103 +++++++++++++----- .../rules/consistent-type-specifier-style.js | 10 +- 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/src/rules/consistent-type-specifier-style.js b/src/rules/consistent-type-specifier-style.js index b3787b0e61..67532e6ac1 100644 --- a/src/rules/consistent-type-specifier-style.js +++ b/src/rules/consistent-type-specifier-style.js @@ -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; @@ -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; }, }); }, @@ -62,11 +68,15 @@ 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; @@ -74,9 +84,13 @@ module.exports = { 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; } @@ -84,8 +98,8 @@ module.exports = { typeSpecifiers.push(specifier); } else if (specifier.importKind === 'typeof') { typeofSpecifiers.push(specifier); - } else { - continue; + } else if (specifier.importKind === 'value' || specifier.importKind == null) { + valueSpecifiers.push(specifier); } } @@ -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 { @@ -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)); } } }, @@ -172,3 +220,8 @@ module.exports = { }; }, }; + +function isComma(token) { + return token.type === 'Punctuator' && token.value === ','; +} + diff --git a/tests/src/rules/consistent-type-specifier-style.js b/tests/src/rules/consistent-type-specifier-style.js index e924e6669c..fa83abd96b 100644 --- a/tests/src/rules/consistent-type-specifier-style.js +++ b/tests/src/rules/consistent-type-specifier-style.js @@ -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.', @@ -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.', @@ -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.', @@ -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: [ { @@ -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.',