Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make most variants of module.exports = { ... } fixable #17

Merged
merged 15 commits into from
Apr 25, 2022
Merged

Make most variants of module.exports = { ... } fixable #17

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f2efcd0
Make the exports style fixable under some circumstances
papandreou Mar 3, 2020
c802465
Handle generators
papandreou Mar 4, 2020
0d616ad
Preserve comments from inside the object literal
papandreou Mar 4, 2020
9181e71
Consistent semicolons in fixed output
papandreou Mar 5, 2020
f3e8691
Test that we only replace top-level module.exports = {...}
papandreou Mar 5, 2020
49e5582
Also replace module.exports.foo references
papandreou Mar 5, 2020
8b1d4ca
Split fixing code into two functions to avoid exceeding the complexit…
papandreou Mar 5, 2020
de3d152
Also fix shorthand property occurrences
papandreou Mar 5, 2020
7bec563
Make helper functions
papandreou Mar 5, 2020
435bfb4
Return undefined instead of []
papandreou Mar 5, 2020
f877a55
Simplify a bit
papandreou Mar 5, 2020
8d7cec4
Don't use SourceCode#getComments (deprecated)
papandreou Jun 9, 2020
e783167
Be more defensive and try to match the patterns that we do want to fix
papandreou Apr 24, 2022
a4ec500
Return null instead of undefined
papandreou Apr 24, 2022
8425c80
Add a few more test cases
papandreou Apr 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
86 changes: 85 additions & 1 deletion lib/rules/exports-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,87 @@ function getExportsNodes(scope) {
return variable.references.map(reference => reference.identifier)
}

function getReplacementForProperty(property, sourceCode) {
if (property.kind === "get" || property.type === "SpreadElement") {
// module.exports = { get foo() { ... } }
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
// module.exports = { foo }
// module.exports = { ...foo }
// We don't have a similarly nice syntax for adding these directly
// on the exports object. Give up on fixing the whole thing.
return undefined
papandreou marked this conversation as resolved.
Show resolved Hide resolved
}
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 undefined
}
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 undefined
papandreou marked this conversation as resolved.
Show resolved Hide resolved
}
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 undefined
papandreou marked this conversation as resolved.
Show resolved Hide resolved
}
}
return fixer.replaceText(node.parent, statements.join("\n\n"))
}

module.exports = {
meta: {
docs: {
Expand All @@ -149,7 +230,7 @@ module.exports = {
"https://github.com/mysticatea/eslint-plugin-node/blob/v11.0.0/docs/rules/exports-style.md",
},
type: "suggestion",
fixable: null,
fixable: "code",
schema: [
{
//
Expand Down Expand Up @@ -254,6 +335,9 @@ module.exports = {
loc: getLocation(node),
message:
"Unexpected access to 'module.exports'. Use 'exports' instead.",
fix(fixer) {
return fixModuleExports(node, sourceCode, fixer)
},
})
}

Expand Down
153 changes: 153 additions & 0 deletions tests/lib/rules/exports-style.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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,154 @@ 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: "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 = { 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.",
],
},
aladdin-add marked this conversation as resolved.
Show resolved Hide resolved
{
code: "module.exports = exports = {foo: 1}",
output: null,
options: ["exports"],
globals: { module: false, exports: true },
errors: [
Expand All @@ -163,6 +313,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 +323,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 +332,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