diff --git a/docs/rules/no-path-concat.md b/docs/rules/no-path-concat.md index 621e600e..515ebc35 100644 --- a/docs/rules/no-path-concat.md +++ b/docs/rules/no-path-concat.md @@ -30,20 +30,25 @@ This rule aims to prevent string concatenation of directory paths in Node.js Examples of **incorrect** code for this rule: ```js -/*eslint no-path-concat: "error"*/ - -var fullPath = __dirname + "/foo.js"; - -var fullPath = __filename + "/foo.js"; +/*eslint node/no-path-concat: "error"*/ +const fullPath1 = __dirname + "/foo.js"; +const fullPath2 = __filename + "/foo.js"; +const fullPath3 = `${__dirname}/foo.js`; +const fullPath4 = `${__filename}/foo.js`; ``` Examples of **correct** code for this rule: ```js -/*eslint no-path-concat: "error"*/ - -var fullPath = dirname + "/foo.js"; +/*eslint node/no-path-concat: "error"*/ + +const fullPath1 = path.join(__dirname, "foo.js"); +const fullPath2 = path.join(__filename, "foo.js"); +const fullPath3 = __dirname + ".js"; +const fullPath4 = __filename + ".map"; +const fullPath5 = `${__dirname}_foo.js`; +const fullPath6 = `${__filename}.test.js`; ``` ## 🔎 Implementation diff --git a/lib/rules/no-path-concat.js b/lib/rules/no-path-concat.js index 32555bdb..5029ee22 100644 --- a/lib/rules/no-path-concat.js +++ b/lib/rules/no-path-concat.js @@ -4,6 +4,158 @@ */ "use strict" +const path = require("path") +const { READ, ReferenceTracker, getStringIfConstant } = require("eslint-utils") + +/** + * Get the first char of the specified template element. + * @param {TemplateLiteral} node The `TemplateLiteral` node to get. + * @param {number} i The number of template elements to get first char. + * @param {Set} sepNodes The nodes of `path.sep`. + * @param {import("escope").Scope} globalScope The global scope object. + * @param {string[]} outNextChars The array to collect chars. + * @returns {void} + */ +function collectFirstCharsOfTemplateElement( + node, + i, + sepNodes, + globalScope, + outNextChars +) { + const element = node.quasis[i].value.cooked + + if (element == null) { + return + } + if (element !== "") { + outNextChars.push(element[0]) + return + } + if (node.expressions.length > i) { + collectFirstChars( + node.expressions[i], + sepNodes, + globalScope, + outNextChars + ) + } +} + +/** + * Get the first char of a given node. + * @param {TemplateLiteral} node The `TemplateLiteral` node to get. + * @param {Set} sepNodes The nodes of `path.sep`. + * @param {import("escope").Scope} globalScope The global scope object. + * @param {string[]} outNextChars The array to collect chars. + * @returns {void} + */ +function collectFirstChars(node, sepNodes, globalScope, outNextChars) { + switch (node.type) { + case "AssignmentExpression": + collectFirstChars(node.right, sepNodes, globalScope, outNextChars) + break + case "BinaryExpression": + collectFirstChars(node.left, sepNodes, globalScope, outNextChars) + break + case "ConditionalExpression": + collectFirstChars( + node.consequent, + sepNodes, + globalScope, + outNextChars + ) + collectFirstChars( + node.alternate, + sepNodes, + globalScope, + outNextChars + ) + break + case "LogicalExpression": + collectFirstChars(node.left, sepNodes, globalScope, outNextChars) + collectFirstChars(node.right, sepNodes, globalScope, outNextChars) + break + case "SequenceExpression": + collectFirstChars( + node.expressions[node.expressions.length - 1], + sepNodes, + globalScope, + outNextChars + ) + break + case "TemplateLiteral": + collectFirstCharsOfTemplateElement( + node, + 0, + sepNodes, + globalScope, + outNextChars + ) + break + + case "Identifier": + case "MemberExpression": + if (sepNodes.has(node)) { + outNextChars.push(path.sep) + break + } + // fallthrough + default: { + const str = getStringIfConstant(node, globalScope) + if (str) { + outNextChars.push(str[0]) + } + } + } +} + +/** + * Check if a char is a path separator or not. + * @param {string} c The char to check. + * @returns {boolean} `true` if the char is a path separator. + */ +function isPathSeparator(c) { + return c === "/" || c === path.sep +} + +/** + * Check if the given Identifier node is followed by string concatenation with a + * path separator. + * @param {Identifier} node The `__dirname` or `__filename` node to check. + * @param {Set} sepNodes The nodes of `path.sep`. + * @param {import("escope").Scope} globalScope The global scope object. + * @returns {boolean} `true` if the given Identifier node is followed by string + * concatenation with a path separator. + */ +function isConcat(node, sepNodes, globalScope) { + const parent = node.parent + const nextChars = [] + + if ( + parent.type === "BinaryExpression" && + parent.operator === "+" && + parent.left === node + ) { + collectFirstChars( + parent.right, + sepNodes, + globalScope, + /* out */ nextChars + ) + } else if (parent.type === "TemplateLiteral") { + collectFirstCharsOfTemplateElement( + parent, + parent.expressions.indexOf(node) + 1, + sepNodes, + globalScope, + /* out */ nextChars + ) + } + + return nextChars.some(isPathSeparator) +} + module.exports = { meta: { type: "suggestion", @@ -19,28 +171,40 @@ module.exports = { schema: [], messages: { usePathFunctions: - "Use path.join() or path.resolve() instead of + to create paths.", + "Use path.join() or path.resolve() instead of string concatenation.", }, }, create(context) { - const MATCHER = /^__(?:dir|file)name$/u - return { - BinaryExpression(node) { - const left = node.left - const right = node.right - - if ( - node.operator === "+" && - ((left.type === "Identifier" && MATCHER.test(left.name)) || - (right.type === "Identifier" && - MATCHER.test(right.name))) - ) { - context.report({ - node, - messageId: "usePathFunctions", - }) + "Program:exit"() { + const globalScope = context.getScope() + const tracker = new ReferenceTracker(globalScope) + const sepNodes = new Set() + + // Collect `paht.sep` references + for (const { node } of tracker.iterateCjsReferences({ + path: { sep: { [READ]: true } }, + })) { + sepNodes.add(node) + } + for (const { node } of tracker.iterateEsmReferences({ + path: { sep: { [READ]: true } }, + })) { + sepNodes.add(node) + } + + // Verify `__dirname` and `__filename` + for (const { node } of tracker.iterateGlobalReferences({ + __dirname: { [READ]: true }, + __filename: { [READ]: true }, + })) { + if (isConcat(node, sepNodes, globalScope)) { + context.report({ + node: node.parent, + messageId: "usePathFunctions", + }) + } } }, } diff --git a/tests/lib/rules/no-path-concat.js b/tests/lib/rules/no-path-concat.js index d52d8f4f..bf36aa9e 100644 --- a/tests/lib/rules/no-path-concat.js +++ b/tests/lib/rules/no-path-concat.js @@ -4,15 +4,33 @@ */ "use strict" +/*eslint-disable no-template-curly-in-string */ + +const path = require("path") const RuleTester = require("eslint").RuleTester const rule = require("../../../lib/rules/no-path-concat") -new RuleTester().run("no-path-concat", rule, { +new RuleTester({ + parserOptions: { + ecmaVersion: 2015, + }, + globals: { + __dirname: "readonly", + __filename: "readonly", + require: "readonly", + }, +}).run("no-path-concat", rule, { valid: [ 'var fullPath = dirname + "foo.js";', 'var fullPath = __dirname == "foo.js";', "if (fullPath === __dirname) {}", "if (__dirname === fullPath) {}", + 'var fullPath = "/foo.js" + __filename;', + 'var fullPath = "/foo.js" + __dirname;', + 'var fullPath = __filename + ".map";', + "var fullPath = `${__filename}.map`;", + 'var fullPath = __filename + (test ? ".js" : ".ts");', + 'var fullPath = __filename + (ext || ".js");', ], invalid: [ @@ -35,7 +53,91 @@ new RuleTester().run("no-path-concat", rule, { ], }, { - code: 'var fullPath = "/foo.js" + __filename;', + code: "var fullPath = `${__dirname}/foo.js`;", + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, + { + code: "var fullPath = `${__filename}/foo.js`;", + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, + { + code: + 'var path = require("path"); var fullPath = `${__dirname}${path.sep}foo.js`;', + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, + { + code: + 'var path = require("path"); var fullPath = `${__filename}${path.sep}foo.js`;', + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, + { + code: + 'var path = require("path"); var fullPath = __dirname + path.sep + `foo.js`;', + errors: [ + { + messageId: "usePathFunctions", + type: "BinaryExpression", + }, + ], + }, + { + code: 'var fullPath = __dirname + "/" + "foo.js";', + errors: [ + { + messageId: "usePathFunctions", + type: "BinaryExpression", + }, + ], + }, + { + code: 'var fullPath = __dirname + ("/" + "foo.js");', + errors: [ + { + messageId: "usePathFunctions", + type: "BinaryExpression", + }, + ], + }, + { + code: 'var fullPath = __dirname + (test ? "/foo.js" : "/bar.js");', + errors: [ + { + messageId: "usePathFunctions", + type: "BinaryExpression", + }, + ], + }, + { + code: 'var fullPath = __dirname + (extraPath || "/default.js");', + errors: [ + { + messageId: "usePathFunctions", + type: "BinaryExpression", + }, + ], + }, + { + code: `var fullPath = __dirname + "\\${path.sep}foo.js";`, errors: [ { messageId: "usePathFunctions", @@ -44,7 +146,7 @@ new RuleTester().run("no-path-concat", rule, { ], }, { - code: 'var fullPath = "/foo.js" + __dirname;', + code: `var fullPath = __filename + "\\${path.sep}foo.js";`, errors: [ { messageId: "usePathFunctions", @@ -52,5 +154,25 @@ new RuleTester().run("no-path-concat", rule, { }, ], }, + { + code: `var fullPath = \`\${__dirname}\\${path.sep}foo.js\`;`, + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, + { + code: `var fullPath = \`\${__filename}\\${path.sep}foo.js\`;`, + errors: [ + { + messageId: "usePathFunctions", + type: "TemplateLiteral", + }, + ], + }, ], }) + +/*eslint-enable no-template-curly-in-string */