Skip to content

Commit ac4960c

Browse files
bradzacherbradzacher
committed
improve fixer to cleanup commas and empty braces
1 parent 56732f2 commit ac4960c

File tree

2 files changed

+83
-30
lines changed

2 files changed

+83
-30
lines changed

Diff for: ‎src/rules/consistent-type-specifier-style.js

+78-25
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,12 @@ module.exports = {
2828
}
2929

3030
if (
31+
// no specifiers (import type {} from '') have no specifiers to mark as inline
3132
node.specifiers.length === 0 ||
3233
(node.specifiers.length === 1 &&
34+
// default imports are both "inline" and "top-level"
3335
(node.specifiers[0].type === 'ImportDefaultSpecifier' ||
36+
// namespace imports are both "inline" and "top-level"
3437
node.specifiers[0].type === 'ImportNamespaceSpecifier'))
3538
) {
3639
return;
@@ -42,16 +45,19 @@ module.exports = {
4245
data: {
4346
kind: node.importKind,
4447
},
45-
*fix(fixer) {
46-
const kindToken = sourceCode.getFirstToken(node, { skip: 1 });
48+
fix(fixer) {
49+
const fixes = [];
4750

51+
const kindToken = sourceCode.getFirstToken(node, { skip: 1 });
4852
if (kindToken) {
49-
yield fixer.remove(kindToken);
53+
fixes.push(fixer.remove(kindToken));
5054
}
5155

5256
for (const specifier of node.specifiers) {
53-
yield fixer.insertTextBefore(specifier, `${node.importKind} `);
57+
fixes.push(fixer.insertTextBefore(specifier, `${node.importKind} `));
5458
}
59+
60+
return fixes;
5561
},
5662
});
5763
},
@@ -62,30 +68,38 @@ module.exports = {
6268
return {
6369
ImportDeclaration(node) {
6470
if (
71+
// already top-level is valid
6572
node.importKind === 'type' ||
6673
node.importKind === 'typeof' ||
74+
// no specifiers (import {} from '') cannot have inline - so is valid
6775
node.specifiers.length === 0 ||
6876
(node.specifiers.length === 1 &&
77+
// default imports are both "inline" and "top-level"
6978
(node.specifiers[0].type === 'ImportDefaultSpecifier' ||
79+
// namespace imports are both "inline" and "top-level"
7080
node.specifiers[0].type === 'ImportNamespaceSpecifier'))
7181
) {
7282
return;
7383
}
7484

7585
const typeSpecifiers = [];
7686
const typeofSpecifiers = [];
87+
const valueSpecifiers = [];
88+
let defaultSpecifier = null;
7789
for (const specifier of node.specifiers) {
78-
// TODO - remove
79-
if (specifier.type !== 'ImportSpecifier') {
90+
if (specifier.type === 'ImportDefaultSpecifier') {
91+
defaultSpecifier = specifier;
92+
continue;
93+
} else if (specifier.type !== 'ImportSpecifier') {
8094
continue;
8195
}
8296

8397
if (specifier.importKind === 'type') {
8498
typeSpecifiers.push(specifier);
8599
} else if (specifier.importKind === 'typeof') {
86100
typeofSpecifiers.push(specifier);
87-
} else {
88-
continue;
101+
} else if (specifier.importKind === 'value' || specifier.importKind == null) {
102+
valueSpecifiers.push(specifier);
89103
}
90104
}
91105

@@ -111,8 +125,8 @@ module.exports = {
111125
data: {
112126
kind: kind.join('/'),
113127
},
114-
*fix(fixer) {
115-
yield fixer.replaceText(node, newImports);
128+
fix(fixer) {
129+
return fixer.replaceText(node, newImports);
116130
},
117131
});
118132
} else {
@@ -124,25 +138,59 @@ module.exports = {
124138
data: {
125139
kind: specifier.importKind,
126140
},
127-
*fix(fixer) {
128-
// remove just the specifiers and insert the new imports after
129-
yield* removeSpecifiers(typeSpecifiers);
130-
yield* removeSpecifiers(typeofSpecifiers);
131-
yield fixer.insertTextAfter(node, `\n${newImports}`);
132-
133-
function* removeSpecifiers(
134-
specifiers,
135-
) {
141+
fix(fixer) {
142+
const fixes = [];
143+
144+
// if there are no value specifiers, then the other report fixer will be called, not this one
145+
146+
if (valueSpecifiers.length > 0) {
147+
// import { Value, type Type } from 'mod';
148+
149+
// we can just remove the type specifiers
150+
removeSpecifiers(typeSpecifiers);
151+
removeSpecifiers(typeofSpecifiers);
152+
153+
// make the import nicely formatted by also removing the trailing comma after the last value import
154+
// eg
155+
// import { Value, type Type } from 'mod';
156+
// to
157+
// import { Value } from 'mod';
158+
// not
159+
// import { Value, } from 'mod';
160+
const maybeComma = sourceCode.getTokenAfter(valueSpecifiers[valueSpecifiers.length - 1]);
161+
if (isComma(maybeComma)) {
162+
fixes.push(fixer.remove(maybeComma));
163+
}
164+
} else if (defaultSpecifier) {
165+
// import Default, { type Type } from 'mod';
166+
167+
// remove the entire curly block so we don't leave an empty one behind
168+
// NOTE - the default specifier *must* be the first specifier always!
169+
// so a comma exists that we also have to clean up or else it's bad syntax
170+
const comma = sourceCode.getTokenAfter(defaultSpecifier, isComma);
171+
const closingBrace = sourceCode.getTokenAfter(
172+
node.specifiers[node.specifiers.length - 1],
173+
token => token.type === 'Punctuator' && token.value === '}',
174+
);
175+
fixes.push(fixer.removeRange([
176+
comma.range[0],
177+
closingBrace.range[1],
178+
]));
179+
}
180+
181+
// insert the new imports after the old declaration
182+
fixes.push(fixer.insertTextAfter(node, `\n${newImports}`));
183+
184+
return fixes;
185+
186+
function removeSpecifiers(specifiers) {
136187
for (const specifier of specifiers) {
137188
// remove the trailing comma
138-
const comma = sourceCode.getTokenAfter(
139-
specifier,
140-
token => token.type === 'Punctuator' && token.value === ',',
141-
);
189+
const comma = sourceCode.getTokenAfter(specifier, isComma);
142190
if (comma) {
143-
yield fixer.remove(comma);
191+
fixes.push(fixer.remove(comma));
144192
}
145-
yield fixer.remove(specifier);
193+
fixes.push(fixer.remove(specifier));
146194
}
147195
}
148196
},
@@ -172,3 +220,8 @@ module.exports = {
172220
};
173221
},
174222
};
223+
224+
function isComma(token) {
225+
return token.type === 'Punctuator' && token.value === ',';
226+
}
227+

Diff for: ‎tests/src/rules/consistent-type-specifier-style.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ const COMMON_TESTS = {
6666
},
6767
{
6868
code: "import { Foo, type Bar } from 'Foo';",
69-
output: "import { Foo, } from 'Foo';\nimport type {Bar} from 'Foo';",
69+
output: "import { Foo } from 'Foo';\nimport type {Bar} from 'Foo';",
7070
options: ['prefer-top-level'],
7171
errors: [{
7272
message: 'Prefer using a top-level type-only import instead of inline type specifiers.',
@@ -84,7 +84,7 @@ const COMMON_TESTS = {
8484
},
8585
{
8686
code: "import Foo, { type Bar } from 'Foo';",
87-
output: "import Foo, { } from 'Foo';\nimport type {Bar} from 'Foo';",
87+
output: "import Foo from 'Foo';\nimport type {Bar} from 'Foo';",
8888
options: ['prefer-top-level'],
8989
errors: [{
9090
message: 'Prefer using a top-level type-only import instead of inline type specifiers.',
@@ -203,7 +203,7 @@ const FLOW_ONLY = {
203203
},
204204
{
205205
code: "import { Foo, typeof Bar } from 'Foo';",
206-
output: "import { Foo, } from 'Foo';\nimport typeof {Bar} from 'Foo';",
206+
output: "import { Foo } from 'Foo';\nimport typeof {Bar} from 'Foo';",
207207
options: ['prefer-top-level'],
208208
errors: [{
209209
message: 'Prefer using a top-level typeof-only import instead of inline typeof specifiers.',
@@ -221,7 +221,7 @@ const FLOW_ONLY = {
221221
},
222222
{
223223
code: "import { Foo, type Bar, typeof Baz } from 'Foo';",
224-
output: "import { Foo, } from 'Foo';\nimport type {Bar} from 'Foo';\nimport typeof {Baz} from 'Foo';",
224+
output: "import { Foo } from 'Foo';\nimport type {Bar} from 'Foo';\nimport typeof {Baz} from 'Foo';",
225225
options: ['prefer-top-level'],
226226
errors: [
227227
{
@@ -236,7 +236,7 @@ const FLOW_ONLY = {
236236
},
237237
{
238238
code: "import Foo, { typeof Bar } from 'Foo';",
239-
output: "import Foo, { } from 'Foo';\nimport typeof {Bar} from 'Foo';",
239+
output: "import Foo from 'Foo';\nimport typeof {Bar} from 'Foo';",
240240
options: ['prefer-top-level'],
241241
errors: [{
242242
message: 'Prefer using a top-level typeof-only import instead of inline typeof specifiers.',

0 commit comments

Comments
 (0)