Skip to content

Commit 7e2bf41

Browse files
papandreoupapandreou
authored
feat: export-style fixable (eslint-community#17)
* Make the exports style fixable under some circumstances * Handle generators * Preserve comments from inside the object literal * Consistent semicolons in fixed output * Test that we only replace top-level module.exports = {...} * Also replace module.exports.foo references * Split fixing code into two functions to avoid exceeding the complexity budget * Also fix shorthand property occurrences * Make helper functions * Return undefined instead of [] Both work and satisfy the linter * Simplify a bit Seems like in-between comments are only attached to one of the nodes * Don't use SourceCode#getComments (deprecated) mysticatea#208 (comment) * Be more defensive and try to match the patterns that we do want to fix eslint-community#17 (comment) * Return null instead of undefined eslint-community#17 (comment) eslint-community#17 (comment) eslint-community#17 (comment) * Add a few more test cases eslint-community#17 (comment)
1 parent e20cc18 commit 7e2bf41

File tree

2 files changed

+270
-1
lines changed

2 files changed

+270
-1
lines changed

‎lib/rules/exports-style.js‎

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,90 @@ function getExportsNodes(scope) {
139139
return variable.references.map(reference => reference.identifier)
140140
}
141141

142+
function getReplacementForProperty(property, sourceCode) {
143+
if (property.type !== "Property" || property.kind !== "init") {
144+
// We don't have a nice syntax for adding these directly on the exports object. Give up on fixing the whole thing:
145+
// property.kind === 'get':
146+
// module.exports = { get foo() { ... } }
147+
// property.kind === 'set':
148+
// module.exports = { set foo() { ... } }
149+
// property.type === 'SpreadElement':
150+
// module.exports = { ...foo }
151+
return null
152+
}
153+
154+
let fixedValue = sourceCode.getText(property.value)
155+
if (property.method) {
156+
fixedValue = `function${
157+
property.value.generator ? "*" : ""
158+
} ${fixedValue}`
159+
if (property.value.async) {
160+
fixedValue = `async ${fixedValue}`
161+
}
162+
}
163+
const lines = sourceCode
164+
.getCommentsBefore(property)
165+
.map(comment => sourceCode.getText(comment))
166+
if (property.key.type === "Literal" || property.computed) {
167+
// String or dynamic key:
168+
// module.exports = { [ ... ]: ... } or { "foo": ... }
169+
lines.push(
170+
`exports[${sourceCode.getText(property.key)}] = ${fixedValue};`
171+
)
172+
} else if (property.key.type === "Identifier") {
173+
// Regular identifier:
174+
// module.exports = { foo: ... }
175+
lines.push(`exports.${property.key.name} = ${fixedValue};`)
176+
} else {
177+
// Some other unknown property type. Conservatively give up on fixing the whole thing.
178+
return null
179+
}
180+
lines.push(
181+
...sourceCode
182+
.getCommentsAfter(property)
183+
.map(comment => sourceCode.getText(comment))
184+
)
185+
return lines.join("\n")
186+
}
187+
188+
// Check for a top level module.exports = { ... }
189+
function isModuleExportsObjectAssignment(node) {
190+
return (
191+
node.parent.type === "AssignmentExpression" &&
192+
node.parent.parent.type === "ExpressionStatement" &&
193+
node.parent.parent.parent.type === "Program" &&
194+
node.parent.right.type === "ObjectExpression"
195+
)
196+
}
197+
198+
// Check for module.exports.foo or module.exports.bar reference or assignment
199+
function isModuleExportsReference(node) {
200+
return (
201+
node.parent.type === "MemberExpression" && node.parent.object === node
202+
)
203+
}
204+
205+
function fixModuleExports(node, sourceCode, fixer) {
206+
if (isModuleExportsReference(node)) {
207+
return fixer.replaceText(node, "exports")
208+
}
209+
if (!isModuleExportsObjectAssignment(node)) {
210+
return null
211+
}
212+
const statements = []
213+
const properties = node.parent.right.properties
214+
for (const property of properties) {
215+
const statement = getReplacementForProperty(property, sourceCode)
216+
if (statement) {
217+
statements.push(statement)
218+
} else {
219+
// No replacement available, give up on the whole thing
220+
return null
221+
}
222+
}
223+
return fixer.replaceText(node.parent, statements.join("\n\n"))
224+
}
225+
142226
module.exports = {
143227
meta: {
144228
docs: {
@@ -148,7 +232,7 @@ module.exports = {
148232
url: "https://github.com/weiran-zsd/eslint-plugin-node/blob/HEAD/docs/rules/exports-style.md",
149233
},
150234
type: "suggestion",
151-
fixable: null,
235+
fixable: "code",
152236
schema: [
153237
{
154238
//
@@ -253,6 +337,9 @@ module.exports = {
253337
loc: getLocation(node),
254338
message:
255339
"Unexpected access to 'module.exports'. Use 'exports' instead.",
340+
fix(fixer) {
341+
return fixModuleExports(node, sourceCode, fixer)
342+
},
256343
})
257344
}
258345

‎tests/lib/rules/exports-style.js‎

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,27 +68,31 @@ new RuleTester().run("exports-style", rule, {
6868
invalid: [
6969
{
7070
code: "exports = {foo: 1}",
71+
output: null,
7172
globals: { module: false, exports: true },
7273
errors: [
7374
"Unexpected access to 'exports'. Use 'module.exports' instead.",
7475
],
7576
},
7677
{
7778
code: "exports.foo = 1",
79+
output: null,
7880
globals: { module: false, exports: true },
7981
errors: [
8082
"Unexpected access to 'exports'. Use 'module.exports' instead.",
8183
],
8284
},
8385
{
8486
code: "module.exports = exports = {foo: 1}",
87+
output: null,
8588
globals: { module: false, exports: true },
8689
errors: [
8790
"Unexpected access to 'exports'. Use 'module.exports' instead.",
8891
],
8992
},
9093
{
9194
code: "exports = module.exports = {foo: 1}",
95+
output: null,
9296
globals: { module: false, exports: true },
9397
errors: [
9498
"Unexpected access to 'exports'. Use 'module.exports' instead.",
@@ -97,6 +101,7 @@ new RuleTester().run("exports-style", rule, {
97101

98102
{
99103
code: "exports = {foo: 1}",
104+
output: null,
100105
options: ["module.exports"],
101106
globals: { module: false, exports: true },
102107
errors: [
@@ -105,6 +110,7 @@ new RuleTester().run("exports-style", rule, {
105110
},
106111
{
107112
code: "exports.foo = 1",
113+
output: null,
108114
options: ["module.exports"],
109115
globals: { module: false, exports: true },
110116
errors: [
@@ -113,6 +119,7 @@ new RuleTester().run("exports-style", rule, {
113119
},
114120
{
115121
code: "module.exports = exports = {foo: 1}",
122+
output: null,
116123
options: ["module.exports"],
117124
globals: { module: false, exports: true },
118125
errors: [
@@ -121,6 +128,7 @@ new RuleTester().run("exports-style", rule, {
121128
},
122129
{
123130
code: "exports = module.exports = {foo: 1}",
131+
output: null,
124132
options: ["module.exports"],
125133
globals: { module: false, exports: true },
126134
errors: [
@@ -130,6 +138,7 @@ new RuleTester().run("exports-style", rule, {
130138

131139
{
132140
code: "exports = {foo: 1}",
141+
output: null,
133142
options: ["exports"],
134143
globals: { module: false, exports: true },
135144
errors: [
@@ -138,6 +147,7 @@ new RuleTester().run("exports-style", rule, {
138147
},
139148
{
140149
code: "module.exports = {foo: 1}",
150+
output: "exports.foo = 1;",
141151
options: ["exports"],
142152
globals: { module: false, exports: true },
143153
errors: [
@@ -146,14 +156,183 @@ new RuleTester().run("exports-style", rule, {
146156
},
147157
{
148158
code: "module.exports.foo = 1",
159+
output: "exports.foo = 1",
149160
options: ["exports"],
150161
globals: { module: false, exports: true },
151162
errors: [
152163
"Unexpected access to 'module.exports'. Use 'exports' instead.",
153164
],
154165
},
166+
{
167+
code: "module.exports = { a: 1 }",
168+
output: "exports.a = 1;",
169+
options: ["exports"],
170+
globals: { module: false, exports: true },
171+
errors: [
172+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
173+
],
174+
},
175+
{
176+
code: "module.exports = { a: 1, b: 2 }",
177+
output: "exports.a = 1;\n\nexports.b = 2;",
178+
options: ["exports"],
179+
globals: { module: false, exports: true },
180+
errors: [
181+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
182+
],
183+
},
184+
{
185+
code:
186+
"module.exports = { // before a\na: 1, // between a and b\nb: 2 // after b\n}",
187+
output:
188+
"// before a\nexports.a = 1;\n\n// between a and b\nexports.b = 2;\n// after b",
189+
options: ["exports"],
190+
globals: { module: false, exports: true },
191+
errors: [
192+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
193+
],
194+
},
195+
{
196+
code: "foo(module.exports = {foo: 1})",
197+
output: null,
198+
options: ["exports"],
199+
globals: { module: false, exports: true },
200+
errors: [
201+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
202+
],
203+
},
204+
{
205+
code:
206+
"if(foo){ module.exports = { foo: 1};} else { module.exports = {foo: 2};}",
207+
output: null,
208+
options: ["exports"],
209+
globals: { module: false, exports: true },
210+
errors: [
211+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
212+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
213+
],
214+
},
215+
{
216+
code: "function bar() { module.exports = { foo: 1 }; }",
217+
output: null,
218+
options: ["exports"],
219+
globals: { module: false, exports: true },
220+
errors: [
221+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
222+
],
223+
},
224+
{
225+
code: "module.exports = { get a() {} }",
226+
output: null,
227+
options: ["exports"],
228+
globals: { module: false, exports: true },
229+
errors: [
230+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
231+
],
232+
},
233+
{
234+
code: "module.exports = { set a(a) {} }",
235+
output: null,
236+
options: ["exports"],
237+
globals: { module: false, exports: true },
238+
errors: [
239+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
240+
],
241+
},
242+
{
243+
code: "module.exports = { a }",
244+
output: "exports.a = a;",
245+
options: ["exports"],
246+
parserOptions: { ecmaVersion: 6 },
247+
globals: { module: false, exports: true },
248+
errors: [
249+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
250+
],
251+
},
252+
{
253+
code: "module.exports = { ...a }",
254+
output: null,
255+
options: ["exports"],
256+
parserOptions: { ecmaVersion: 9 },
257+
globals: { module: false, exports: true },
258+
errors: [
259+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
260+
],
261+
},
262+
{
263+
code: "module.exports = { ['a' + 'b']: 1 }",
264+
output: "exports['a' + 'b'] = 1;",
265+
options: ["exports"],
266+
parserOptions: { ecmaVersion: 6 },
267+
globals: { module: false, exports: true },
268+
errors: [
269+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
270+
],
271+
},
272+
{
273+
code: "module.exports = { 'foo': 1 }",
274+
output: "exports['foo'] = 1;",
275+
options: ["exports"],
276+
parserOptions: { ecmaVersion: 6 },
277+
globals: { module: false, exports: true },
278+
errors: [
279+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
280+
],
281+
},
282+
{
283+
code: "module.exports = { foo(a) {} }",
284+
output: "exports.foo = function (a) {};",
285+
options: ["exports"],
286+
parserOptions: { ecmaVersion: 8 },
287+
globals: { module: false, exports: true },
288+
errors: [
289+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
290+
],
291+
},
292+
{
293+
code: "module.exports = { *foo(a) {} }",
294+
output: "exports.foo = function* (a) {};",
295+
options: ["exports"],
296+
parserOptions: { ecmaVersion: 6 },
297+
globals: { module: false, exports: true },
298+
errors: [
299+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
300+
],
301+
},
302+
{
303+
code: "module.exports = { async foo(a) {} }",
304+
output: "exports.foo = async function (a) {};",
305+
options: ["exports"],
306+
parserOptions: { ecmaVersion: 8 },
307+
globals: { module: false, exports: true },
308+
errors: [
309+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
310+
],
311+
},
312+
{
313+
code: "module.exports.foo()",
314+
output: "exports.foo()",
315+
options: ["exports"],
316+
parserOptions: { ecmaVersion: 8 },
317+
globals: { module: false, exports: true },
318+
errors: [
319+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
320+
],
321+
},
322+
{
323+
code: "a = module.exports.foo + module.exports['bar']",
324+
output: "a = exports.foo + exports['bar']",
325+
options: ["exports"],
326+
parserOptions: { ecmaVersion: 8 },
327+
globals: { module: false, exports: true },
328+
errors: [
329+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
330+
"Unexpected access to 'module.exports'. Use 'exports' instead.",
331+
],
332+
},
155333
{
156334
code: "module.exports = exports = {foo: 1}",
335+
output: null,
157336
options: ["exports"],
158337
globals: { module: false, exports: true },
159338
errors: [
@@ -163,6 +342,7 @@ new RuleTester().run("exports-style", rule, {
163342
},
164343
{
165344
code: "exports = module.exports = {foo: 1}",
345+
output: null,
166346
options: ["exports"],
167347
globals: { module: false, exports: true },
168348
errors: [
@@ -172,6 +352,7 @@ new RuleTester().run("exports-style", rule, {
172352
},
173353
{
174354
code: "module.exports = exports = {foo: 1}; exports = obj",
355+
output: null,
175356
options: ["exports", { allowBatchAssign: true }],
176357
globals: { module: false, exports: true },
177358
errors: [
@@ -180,6 +361,7 @@ new RuleTester().run("exports-style", rule, {
180361
},
181362
{
182363
code: "exports = module.exports = {foo: 1}; exports = obj",
364+
output: null,
183365
options: ["exports", { allowBatchAssign: true }],
184366
globals: { module: false, exports: true },
185367
errors: [

0 commit comments

Comments
 (0)