Skip to content

Commit e2da592

Browse files
committed
⚒ improve no-path-concat
1 parent ade0b59 commit e2da592

File tree

3 files changed

+319
-28
lines changed

3 files changed

+319
-28
lines changed

Diff for: docs/rules/no-path-concat.md

+13-8
Original file line numberDiff line numberDiff line change
@@ -30,20 +30,25 @@ This rule aims to prevent string concatenation of directory paths in Node.js
3030
Examples of **incorrect** code for this rule:
3131

3232
```js
33-
/*eslint no-path-concat: "error"*/
34-
35-
var fullPath = __dirname + "/foo.js";
36-
37-
var fullPath = __filename + "/foo.js";
33+
/*eslint node/no-path-concat: "error"*/
3834

35+
const fullPath1 = __dirname + "/foo.js";
36+
const fullPath2 = __filename + "/foo.js";
37+
const fullPath3 = `${__dirname}/foo.js`;
38+
const fullPath4 = `${__filename}/foo.js`;
3939
```
4040

4141
Examples of **correct** code for this rule:
4242

4343
```js
44-
/*eslint no-path-concat: "error"*/
45-
46-
var fullPath = dirname + "/foo.js";
44+
/*eslint node/no-path-concat: "error"*/
45+
46+
const fullPath1 = path.join(__dirname, "foo.js");
47+
const fullPath2 = path.join(__filename, "foo.js");
48+
const fullPath3 = __dirname + ".js";
49+
const fullPath4 = __filename + ".map";
50+
const fullPath5 = `${__dirname}_foo.js`;
51+
const fullPath6 = `${__filename}.test.js`;
4752
```
4853

4954
## 🔎 Implementation

Diff for: lib/rules/no-path-concat.js

+181-17
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,158 @@
44
*/
55
"use strict"
66

7+
const path = require("path")
8+
const { READ, ReferenceTracker, getStringIfConstant } = require("eslint-utils")
9+
10+
/**
11+
* Get the first char of the specified template element.
12+
* @param {TemplateLiteral} node The `TemplateLiteral` node to get.
13+
* @param {number} i The number of template elements to get first char.
14+
* @param {Set<Node>} sepNodes The nodes of `path.sep`.
15+
* @param {import("escope").Scope} globalScope The global scope object.
16+
* @param {string[]} outNextChars The array to collect chars.
17+
* @returns {void}
18+
*/
19+
function collectFirstCharsOfTemplateElement(
20+
node,
21+
i,
22+
sepNodes,
23+
globalScope,
24+
outNextChars
25+
) {
26+
const element = node.quasis[i].value.cooked
27+
28+
if (element == null) {
29+
return
30+
}
31+
if (element !== "") {
32+
outNextChars.push(element[0])
33+
return
34+
}
35+
if (node.expressions.length > i) {
36+
collectFirstChars(
37+
node.expressions[i],
38+
sepNodes,
39+
globalScope,
40+
outNextChars
41+
)
42+
}
43+
}
44+
45+
/**
46+
* Get the first char of a given node.
47+
* @param {TemplateLiteral} node The `TemplateLiteral` node to get.
48+
* @param {Set<Node>} sepNodes The nodes of `path.sep`.
49+
* @param {import("escope").Scope} globalScope The global scope object.
50+
* @param {string[]} outNextChars The array to collect chars.
51+
* @returns {void}
52+
*/
53+
function collectFirstChars(node, sepNodes, globalScope, outNextChars) {
54+
switch (node.type) {
55+
case "AssignmentExpression":
56+
collectFirstChars(node.right, sepNodes, globalScope, outNextChars)
57+
break
58+
case "BinaryExpression":
59+
collectFirstChars(node.left, sepNodes, globalScope, outNextChars)
60+
break
61+
case "ConditionalExpression":
62+
collectFirstChars(
63+
node.consequent,
64+
sepNodes,
65+
globalScope,
66+
outNextChars
67+
)
68+
collectFirstChars(
69+
node.alternate,
70+
sepNodes,
71+
globalScope,
72+
outNextChars
73+
)
74+
break
75+
case "LogicalExpression":
76+
collectFirstChars(node.left, sepNodes, globalScope, outNextChars)
77+
collectFirstChars(node.right, sepNodes, globalScope, outNextChars)
78+
break
79+
case "SequenceExpression":
80+
collectFirstChars(
81+
node.expressions[node.expressions.length - 1],
82+
sepNodes,
83+
globalScope,
84+
outNextChars
85+
)
86+
break
87+
case "TemplateLiteral":
88+
collectFirstCharsOfTemplateElement(
89+
node,
90+
0,
91+
sepNodes,
92+
globalScope,
93+
outNextChars
94+
)
95+
break
96+
97+
case "Identifier":
98+
case "MemberExpression":
99+
if (sepNodes.has(node)) {
100+
outNextChars.push(path.sep)
101+
break
102+
}
103+
// fallthrough
104+
default: {
105+
const str = getStringIfConstant(node, globalScope)
106+
if (str) {
107+
outNextChars.push(str[0])
108+
}
109+
}
110+
}
111+
}
112+
113+
/**
114+
* Check if a char is a path separator or not.
115+
* @param {string} c The char to check.
116+
* @returns {boolean} `true` if the char is a path separator.
117+
*/
118+
function isPathSeparator(c) {
119+
return c === "/" || c === path.sep
120+
}
121+
122+
/**
123+
* Check if the given Identifier node is followed by string concatenation with a
124+
* path separator.
125+
* @param {Identifier} node The `__dirname` or `__filename` node to check.
126+
* @param {Set<Node>} sepNodes The nodes of `path.sep`.
127+
* @param {import("escope").Scope} globalScope The global scope object.
128+
* @returns {boolean} `true` if the given Identifier node is followed by string
129+
* concatenation with a path separator.
130+
*/
131+
function isConcat(node, sepNodes, globalScope) {
132+
const parent = node.parent
133+
const nextChars = []
134+
135+
if (
136+
parent.type === "BinaryExpression" &&
137+
parent.operator === "+" &&
138+
parent.left === node
139+
) {
140+
collectFirstChars(
141+
parent.right,
142+
sepNodes,
143+
globalScope,
144+
/* out */ nextChars
145+
)
146+
} else if (parent.type === "TemplateLiteral") {
147+
collectFirstCharsOfTemplateElement(
148+
parent,
149+
parent.expressions.indexOf(node) + 1,
150+
sepNodes,
151+
globalScope,
152+
/* out */ nextChars
153+
)
154+
}
155+
156+
return nextChars.some(isPathSeparator)
157+
}
158+
7159
module.exports = {
8160
meta: {
9161
type: "suggestion",
@@ -19,28 +171,40 @@ module.exports = {
19171
schema: [],
20172
messages: {
21173
usePathFunctions:
22-
"Use path.join() or path.resolve() instead of + to create paths.",
174+
"Use path.join() or path.resolve() instead of string concatenation.",
23175
},
24176
},
25177

26178
create(context) {
27-
const MATCHER = /^__(?:dir|file)name$/u
28-
29179
return {
30-
BinaryExpression(node) {
31-
const left = node.left
32-
const right = node.right
33-
34-
if (
35-
node.operator === "+" &&
36-
((left.type === "Identifier" && MATCHER.test(left.name)) ||
37-
(right.type === "Identifier" &&
38-
MATCHER.test(right.name)))
39-
) {
40-
context.report({
41-
node,
42-
messageId: "usePathFunctions",
43-
})
180+
"Program:exit"() {
181+
const globalScope = context.getScope()
182+
const tracker = new ReferenceTracker(globalScope)
183+
const sepNodes = new Set()
184+
185+
// Collect `paht.sep` references
186+
for (const { node } of tracker.iterateCjsReferences({
187+
path: { sep: { [READ]: true } },
188+
})) {
189+
sepNodes.add(node)
190+
}
191+
for (const { node } of tracker.iterateEsmReferences({
192+
path: { sep: { [READ]: true } },
193+
})) {
194+
sepNodes.add(node)
195+
}
196+
197+
// Verify `__dirname` and `__filename`
198+
for (const { node } of tracker.iterateGlobalReferences({
199+
__dirname: { [READ]: true },
200+
__filename: { [READ]: true },
201+
})) {
202+
if (isConcat(node, sepNodes, globalScope)) {
203+
context.report({
204+
node: node.parent,
205+
messageId: "usePathFunctions",
206+
})
207+
}
44208
}
45209
},
46210
}

0 commit comments

Comments
 (0)