Skip to content

Commit

Permalink
feat: export-style fixable (#17)
Browse files Browse the repository at this point in the history 
* 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
#17 (comment)

* Return null instead of undefined
#17 (comment)
#17 (comment)
#17 (comment)

* Add a few more test cases
#17 (comment)
  • Loading branch information
@papandreou
papandreou committed Apr 25, 2022
1 parent e20cc18 commit 7e2bf41
Show file tree
Hide file tree
Showing 2 changed files with 270 additions and 1 deletion.
89 changes: 88 additions & 1 deletion lib/rules/exports-style.js
View file
Expand Up @@ -139,6 +139,90 @@ function getExportsNodes(scope) {
return variable.references.map(reference => reference.identifier)
}

function getReplacementForProperty(property, sourceCode) {
if (property.type !== "Property" || property.kind !== "init") {
// We don't have a nice syntax for adding these directly on the exports object. Give up on fixing the whole thing:
// property.kind === 'get':
// module.exports = { get foo() { ... } }
// property.kind === 'set':
// module.exports = { set foo() { ... } }
// property.type === 'SpreadElement':
// module.exports = { ...foo }
return null
}

let fixedValue = sourceCode.getText(property.value)
if (property.method) {
fixedValue = `function${
property.value.generator ? "*" : ""
} ${fixedValue}`
if (property.value.async) {
fixedValue = `async ${fixedValue}`
}
}
const lines = sourceCode
.getCommentsBefore(property)
.map(comment => sourceCode.getText(comment))
if (property.key.type === "Literal" || property.computed) {
// String or dynamic key:
// module.exports = { [ ... ]: ... } or { "foo": ... }
lines.push(
`exports[${sourceCode.getText(property.key)}] = ${fixedValue};`
)
} else if (property.key.type === "Identifier") {
// Regular identifier:
// module.exports = { foo: ... }
lines.push(`exports.${property.key.name} = ${fixedValue};`)
} else {
// Some other unknown property type. Conservatively give up on fixing the whole thing.
return null
}
lines.push(
...sourceCode
.getCommentsAfter(property)
.map(comment => sourceCode.getText(comment))
)
return lines.join("\n")
}

// Check for a top level module.exports = { ... }
function isModuleExportsObjectAssignment(node) {
return (
node.parent.type === "AssignmentExpression" &&
node.parent.parent.type === "ExpressionStatement" &&
node.parent.parent.parent.type === "Program" &&
node.parent.right.type === "ObjectExpression"
)
}

// Check for module.exports.foo or module.exports.bar reference or assignment
function isModuleExportsReference(node) {
return (
node.parent.type === "MemberExpression" && node.parent.object === node
)
}

function fixModuleExports(node, sourceCode, fixer) {
if (isModuleExportsReference(node)) {
return fixer.replaceText(node, "exports")
}
if (!isModuleExportsObjectAssignment(node)) {
return null
}
const statements = []
const properties = node.parent.right.properties
for (const property of properties) {
const statement = getReplacementForProperty(property, sourceCode)
if (statement) {
statements.push(statement)
} else {
// No replacement available, give up on the whole thing
return null
}
}
return fixer.replaceText(node.parent, statements.join("\n\n"))
}

module.exports = {
meta: {
docs: {
Expand All @@ -148,7 +232,7 @@ module.exports = {
url: "https://github.com/weiran-zsd/eslint-plugin-node/blob/HEAD/docs/rules/exports-style.md",
},
type: "suggestion",
fixable: null,
fixable: "code",
schema: [
{
//
Expand Down Expand Up @@ -253,6 +337,9 @@ module.exports = {
loc: getLocation(node),
message:
"Unexpected access to 'module.exports'. Use 'exports' instead.",
fix(fixer) {
return fixModuleExports(node, sourceCode, fixer)
},
})
}

Expand Down
182 changes: 182 additions & 0 deletions tests/lib/rules/exports-style.js
View file
Expand Up @@ -68,27 +68,31 @@ new RuleTester().run("exports-style", rule, {
invalid: [
{
code: "exports = {foo: 1}",
output: null,
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'exports'. Use 'module.exports' instead.",
],
},
{
code: "exports.foo = 1",
output: null,
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'exports'. Use 'module.exports' instead.",
],
},
{
code: "module.exports = exports = {foo: 1}",
output: null,
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'exports'. Use 'module.exports' instead.",
],
},
{
code: "exports = module.exports = {foo: 1}",
output: null,
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'exports'. Use 'module.exports' instead.",
Expand All @@ -97,6 +101,7 @@ new RuleTester().run("exports-style", rule, {

{
code: "exports = {foo: 1}",
output: null,
options: ["module.exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -105,6 +110,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "exports.foo = 1",
output: null,
options: ["module.exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -113,6 +119,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "module.exports = exports = {foo: 1}",
output: null,
options: ["module.exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -121,6 +128,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "exports = module.exports = {foo: 1}",
output: null,
options: ["module.exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -130,6 +138,7 @@ new RuleTester().run("exports-style", rule, {

{
code: "exports = {foo: 1}",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -138,6 +147,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "module.exports = {foo: 1}",
output: "exports.foo = 1;",
options: ["exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -146,14 +156,183 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "module.exports.foo = 1",
output: "exports.foo = 1",
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { a: 1 }",
output: "exports.a = 1;",
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { a: 1, b: 2 }",
output: "exports.a = 1;\n\nexports.b = 2;",
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code:
"module.exports = { // before a\na: 1, // between a and b\nb: 2 // after b\n}",
output:
"// before a\nexports.a = 1;\n\n// between a and b\nexports.b = 2;\n// after b",
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "foo(module.exports = {foo: 1})",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code:
"if(foo){ module.exports = { foo: 1};} else { module.exports = {foo: 2};}",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "function bar() { module.exports = { foo: 1 }; }",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { get a() {} }",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { set a(a) {} }",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { a }",
output: "exports.a = a;",
options: ["exports"],
parserOptions: { ecmaVersion: 6 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { ...a }",
output: null,
options: ["exports"],
parserOptions: { ecmaVersion: 9 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { ['a' + 'b']: 1 }",
output: "exports['a' + 'b'] = 1;",
options: ["exports"],
parserOptions: { ecmaVersion: 6 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { 'foo': 1 }",
output: "exports['foo'] = 1;",
options: ["exports"],
parserOptions: { ecmaVersion: 6 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { foo(a) {} }",
output: "exports.foo = function (a) {};",
options: ["exports"],
parserOptions: { ecmaVersion: 8 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { *foo(a) {} }",
output: "exports.foo = function* (a) {};",
options: ["exports"],
parserOptions: { ecmaVersion: 6 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = { async foo(a) {} }",
output: "exports.foo = async function (a) {};",
options: ["exports"],
parserOptions: { ecmaVersion: 8 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports.foo()",
output: "exports.foo()",
options: ["exports"],
parserOptions: { ecmaVersion: 8 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "a = module.exports.foo + module.exports['bar']",
output: "a = exports.foo + exports['bar']",
options: ["exports"],
parserOptions: { ecmaVersion: 8 },
globals: { module: false, exports: true },
errors: [
"Unexpected access to 'module.exports'. Use 'exports' instead.",
"Unexpected access to 'module.exports'. Use 'exports' instead.",
],
},
{
code: "module.exports = exports = {foo: 1}",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -163,6 +342,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "exports = module.exports = {foo: 1}",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -172,6 +352,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "module.exports = exports = {foo: 1}; exports = obj",
output: null,
options: ["exports", { allowBatchAssign: true }],
globals: { module: false, exports: true },
errors: [
Expand All @@ -180,6 +361,7 @@ new RuleTester().run("exports-style", rule, {
},
{
code: "exports = module.exports = {foo: 1}; exports = obj",
output: null,
options: ["exports", { allowBatchAssign: true }],
globals: { module: false, exports: true },
errors: [
Expand Down

0 comments on commit 7e2bf41

Please sign in to comment.