Disallow string concatenation when using __dirname and __filename (no-path-concat) #403

Closed
feross opened this Issue Feb 5, 2016 · 4 comments

Projects

None yet

2 participants

@feross
Owner
feross commented Feb 5, 2016

See: http://eslint.org/docs/2.0.0/rules/no-path-concat

Replace this:

var fullPath = __dirname + "/foo.js";

With this:

var fullPath = path.join(__dirname, "foo.js");

From eslint docs:

However, there are a few problems with this. First, you can't be sure what type of system the script is running on. Node.js can be run on any computer, including Windows, which uses a different path separator. It's very easy, therefore, to create an invalid path using string concatenation and assuming Unix-style separators. There's also the possibility of having double separators, or otherwise ending up with an invalid path.

In order to avoid any confusion as to how to create the correct path, Node.js provides the path module. This module uses system-specific information to always return the correct value.

@feross feross referenced this issue Feb 5, 2016
Closed

Release proposal: standard v6 #399

25 of 25 tasks complete
@feross
Owner
feross commented Feb 5, 2016
# tests 427
# pass  395
# fail  32

7.5% of packages either don't work on Windows, or their tests don't work on Windows.

I think we should enable this, because even though this will make people change their code, this is actually a real problem – a programmer error — in their code that should be fixed.

@feross
Owner
feross commented Feb 5, 2016

After fixing up all the packages that I have push access to, only 21/427 (5%) are failing. All the cases I fixed up were real cases where that code wouldn't work in a cross-platform way on Windows.

I think this is a good rule.

@jprichardson
Collaborator

ACK. I think this change makes sense.

@feross feross added a commit to feross/standard-packages that referenced this issue Feb 6, 2016
@feross disable packages that fail because of "no-path-concat" rule f65042b
@feross
Owner
feross commented Feb 6, 2016

Released in standard 6.0.0.

@feross feross closed this Feb 6, 2016
@hden hden referenced this issue in hden/socketio-wildcard Mar 17, 2016
Merged

update dependency #18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment