Skip to content

Commit

Permalink
we only check if at least one branch (conditional or logical) has at …
Browse files Browse the repository at this point in the history
…least one require
  • Loading branch information
jean-michelet committed Jan 17, 2024
1 parent eae16eb commit ed0837c
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 29 deletions.
2 changes: 1 addition & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function runASTAnalysis(
return {
...source.getResult(isMinified),
dependencies: source.dependencies,
isOneLineRequire: source.dependencies.size <= 1 || isOneLineExpressionExport(body)
isOneLineRequire: isOneLineExpressionExport(body)
};
}

Expand Down
18 changes: 12 additions & 6 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Import Third-party Dependencies
import {
getCallExpressionIdentifier,
getMemberExpressionIdentifier
getCallExpressionIdentifier
} from "@nodesecure/estree-ast-utils";

export function notNullOrUndefined(value) {
Expand Down Expand Up @@ -47,16 +46,16 @@ export function isOneLineExpressionExport(body) {
return false;
}

return exportAssignmentHasRequireLeaves(body[0].expression.right);
return exportAssignmentHasRequireLeave(body[0].expression.right);
}

export function exportAssignmentHasRequireLeaves(expr) {
export function exportAssignmentHasRequireLeave(expr) {
if (expr.type === "LogicalExpression") {
return exportAssignmentHasRequireLeaves(expr.left) && exportAssignmentHasRequireLeaves(expr.right);
return atLeastOneBranchHasRequireLeave(expr.left, expr.right);
}

if (expr.type === "ConditionalExpression") {
return exportAssignmentHasRequireLeaves(expr.consequent) && exportAssignmentHasRequireLeaves(expr.alternate);
return atLeastOneBranchHasRequireLeave(expr.consequent, expr.alternate);
}

if (expr.type === "CallExpression") {
Expand All @@ -79,6 +78,13 @@ export function exportAssignmentHasRequireLeaves(expr) {
return false;
}

function atLeastOneBranchHasRequireLeave(left, right) {
return [
exportAssignmentHasRequireLeave(left),
exportAssignmentHasRequireLeave(right)
].some((hasRequire) => hasRequire);
}

export function rootLocation() {
return { start: { line: 0, column: 0 }, end: { line: 0, column: 0 } };
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,30 @@ import { runASTAnalysis } from "../../index.js";

const validTestCases = [
["module.exports = require('fs') || require('constants');", ["fs", "constants"]],
["module.exports = foo === foo ? require('fs').constants.foo || require('constants') : require('someModule');",
["fs", "constants", "someModule"]
],
["module.exports = !ok ? require('fs') : require('someModule').constants.foo || require('constants');",
["fs", "someModule", "constants"]
],
["module.exports = !ok ? require('fs') : require('someModule').constants.foo ? require('constants') : require('foo');",
["fs", "someModule", "constants", "foo"]
],
// Actually, if dependencies are equal or less than one, leaves that are not require callees are ignored
["module.exports = notRequire('fs') || require('constants');", ["constants"]],
["module.exports = require('constants') ? require('fs') : require('foo');", ["constants", "fs", "foo"]],

// should have at least one branch has a `require` callee
["module.exports = require('constants') || {};", ["constants"]],
["module.exports = {} || require('constants');", ["constants"]],
["module.exports = require('constants') ? require('fs') : {};", ["constants", "fs"]],
["module.exports = require('constants') ? {} : require('fs');", ["constants", "fs"]],

// should apply to nested conditions
["module.exports = (require('constants') || {}) || (require('foo') || {});", ["constants", "foo"]],
["module.exports = require('constants') ? (require('fs') || {}) : ({} || require('foo'));", ["constants", "fs", "foo"]],
["module.exports = require('constants') ? ({} || require('fs')) : (require('foo') || {});", ["constants", "fs", "foo"]],
["module.exports = require('constants') ? (require('fs') ? {} : require('bar')) : {};", ["constants", "fs", "bar"]],
["module.exports = require('constants') ? {} : (require('fs') ? {} : require('bar'));", ["constants", "fs", "bar"]],

// test condition that are not `require` callees, here `notRequire('someModule')`, are ignored
["module.exports = !ok ? require('fs') : notRequire('someModule') ? require('constants') : require('foo');",
["fs", "constants", "foo"]
["module.exports = notRequire('someModule') ? require('constants') : require('foo');",
["constants", "foo"]
],
["module.exports = ok ? (notRequire('someModule') ? require('constants') : require('foo')) : {};",
["constants", "foo"]
],
["module.exports = ok ? {} : (notRequire('someModule') ? require('constants') : require('foo'));",
["constants", "foo"]
]
];

Expand All @@ -36,15 +45,16 @@ test("it should return isOneLineRequire true given a single line CJS export with
});

const invalidTestCases = [
["module.exports = foo === foo ? require('fs').constants.foo || notRequire('constants') : require('someModule');",
["fs", "someModule"]
],
["module.exports = !ok ? notRequire('fs') : require('someModule').constants.foo || require('constants');",
["someModule", "constants"]
],
["module.exports = !ok ? require('fs') : require('someModule') ? notRequire('constants') : require('foo');",
["fs", "someModule", "foo"]
]
// should have at least one `require` callee
["module.exports = notRequire('foo') || {};", []],
["module.exports = {} || notRequire('foo');", []],
["module.exports = require('constants') ? {} : {};", ["constants"]],

// same behavior should apply to nested conditions
["module.exports = (notRequire('foo') || {}) || (notRequire('foo') || {});", []],
["module.exports = require('constants') ? (notRequire('foo') || {}) : (notRequire('foo') || {});", ["constants"]],
["module.exports = require('constants') ? (notRequire('foo') || {}) : (notRequire('foo') || {});", ["constants"]],
["module.exports = require('constants') ? (require('constants') ? {} : {}) : (require('constants') ? {} : {});", ["constants"]]
];

test("it should return isOneLineRequire false given a single line CJS export with illegal callees", () => {
Expand Down

0 comments on commit ed0837c

Please sign in to comment.