IgnoredPaths option "dotfiles" causes issue with relative paths

[rawalter21]

When the working directory is outside of the directory in which the Javascript files are stored the relative paths can cause the "dotfiles" option to ignore files incorrectly. For example, if the relative path starts with a dot like in the following example:

../path-to-file/file.js

the "dotfiles" option will cause all of these files to be ignored. You are able to get around this by setting dotfiles to false but this will also cause files that begin with a dot to be considered. Also, the dotfiles attribute is not in the documentation so it was not immediately clear what was going on.

This issue was introduced as part of the fix for #3948

[eslintbot]

Hi @rawalter21, thanks for the issue. It looks like there's not enough information for us to know how to help you. If you're reporting a bug, please be sure to include:

1. The version of ESLint you are using (run eslint -v)
2. What you did (the source code and ESLint configuration)
3. The actual ESLint output complete with numbers
4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

If it's something else, please just provide as much additional information as possible. Thanks!

[ilyavolodin]

Sorry, I'm not sure I follow exactly what you mean. Could you provide a few examples of files being ignored and patterns that cause it?

[rawalter21]

Here is some more info to help debug this:

1. The version of ESLint you are using (run eslint -v)
   We are currently running version 2.1.0
2. What you did (the source code and ESLint configuration)
   We are were using gulp-eslint as follows:

gulp.task('eslint', ['get-javascript-paths'], function() {
    return gulp.src(allJavascriptFiles)
        .pipe(eslint('.eslintrc.json'))
        .pipe(eslint.format())
        .pipe(eslint.failAfterError())
        .pipe(eslint.result(function (result) {
            // Called for each ESLint result.
            console.log('ESLint result: ' + result.filePath);
            console.log('# Messages: ' + result.messages.length);
            console.log('# Warnings: ' + result.warningCount);
            console.log('# Errors: ' + result.errorCount);
        }));
});

This is an example file path that is being used by eslint:

../app_storefront_core/cartridge/js/pages/content/two-column.js

I was only able to get this, and the other files, to appear by adding:

"dotfile": true

to the .eslintrc.json file.

Otherwise eslint would ignore all of the files as they all begin with '../' and output nothing.

In terms of directory structure, we have a top level folder structure that looks like this:

app_storefront_core
gulp_builder
...other folders...

and within gulp_builder is gulpfile.js which contains the task I pasted above. All of the javascript files are within app_storefront_core/cartridge/js/**

[nzakas]

@rawalter21 can you confirm that this isn't just a problem with gulp-eslint? We don't manage the Gulp plugin, and I think it's a good idea to confirm with them.

[ilyavolodin]

From what I can gather, it sounds like gulp issue. I'm not sure what dotfile options is for, but I don't think we have anything like that in ESLint.

[rawalter21]

I apologize for not adding this earlier but here is the offending code (eslint/lib/ignored-paths.js line 162) that was added in commit #3948:

if (options.dotfiles !== true) {
    addPattern(this.ig.default, ".*");
}

You are correct in that this is techincally a gulp-eslint issue but there was no issue until the above code was added. What is happening is that the ".*" pattern is causing the relative paths given by gulp-eslint to be ignored. I understand why this pattern was added (to ignore files that start with a period) but maybe the pattern could be updated to ignore the beginning dot on relative paths if thats possible. We were able to work around the problem by adding the option:

dotfiles: true

to the list of options given to eslint but this should only be a temporary fix. If there is no way around this I will start an issue on the gulp-eslint project to have them fix the issue if possible.

[nzakas]

@rawalter21 unless you can confirm that this is a problem using ESLint outside of Gulp, then the best course of action is to followup with gulp-eslint.

[rawalter21]

In this case it would always be a problem if you passed in relative paths (as opposed to absolute paths) no matter how ESLint is used assuming the paths start with a dot. So its not just an issue with gulp-eslint. If the assumption is that ESLint will only take absolute paths or relative paths that don't start with a dot then yes, it is a problem that needs to be resolved on the gulp-eslint side. Really though, the path shouldn't even matter when the ignore check is done as it should only be checking the file and not the whole path but currently it is not.

[matthewrose]

I also can confirm that this problem happens with all relative paths due to the pattern ".*" matching any relative path that starts in a dot, such as ../path-to-js. This is in the eslint codebase here.

[platinumazure]

I'll create a repository and try to reproduce this tonight.

[platinumazure]

Turns out the offending code is invoked only by autoconfig, which I didn't realize until I crawled through the source code (and which nobody has yet mentioned). I can reproduce when trying to use autoconfig.

This repository shows the bug in action. Follow the steps in README.md to recreate this bug. As far as I'm concerned, this is a real bug. I don't know much about globbing but I'm wondering if the .* could be changed into **/.* or something similar.

I'm not sure how gulp-eslint factors into this-- the command in the comment above seems like it should be linting, not running autoconfig. That issue should be sent to gulp-eslint.

[ilyavolodin]

Interesting. I think I've actually ran into this before, but didn't pay enough attention to notice it. Is it possible to reproduce this without autoconfig? I think autoconfig uses slightly different pattern matching then core itself.

[platinumazure]

I couldn't reproduce with ordinary linting runs. I can keep trying. Are there other ways to specify paths to lint besides CLI args?

[ilyavolodin]

Not that I know of, how about navigating into build directory and running eslint ../src/* from there?

[IanVS]

Autoconfig uses the same glob-utils that the rest of core does, but maybe it's an issue with the cwd. I'll do some investigating.

[platinumazure]

@ilyavolodin Tried that earlier, it doesn't reproduce the issue.

[IanVS]

I've done a little more digging and I'm starting to understand this. What @rawalter21 said in his first comment is correct, we have a very simple test for "dotfiles" (which is anything starting with a .), so patterns like ../example.js are being treated as dotfiles and ignored.

Now that I'm starting to understand the code and the issue, I should be able to come up with some kind of solution this weekend.

[platinumazure]

@IanVS If we reconcile cli-engine to use the same glob-utils function, will that introduce the issue in linting as well?

[IanVS]

Not to worry, I'll make sure it doesn't. 😁

[IanVS]

Status update: so far I've done a lot of investigating, cleaned up the tests, and added a failing test for this issue:

eslint/tests/lib/ignored-paths.js

Line 355 in 8f07d0a

assert.isFalse(ignoredPaths.contains("../foo.js"));

Now hopefully I'll be able to make progress on actually solving it tomorrow.

[platinumazure]

@IanVS Great to hear! Do you still want me to keep my repository around, or may I remove it?

[IanVS]

You can remove it. Thanks for the demonstration.

On Sun, Feb 28, 2016, 18:37 Kevin Partington notifications@github.com
wrote:

@IanVS https://github.com/IanVS Great to hear! Do you still want me to
keep my repository around, or may I remove it?

—
Reply to this email directly or view it on GitHub
#5314 (comment).

[IanVS]

Another update: I made some more progress on branch issue5314, and although all the ignored-paths tests are passing, I've broken a number of other tests and I'll need a few more days to finish.

[IanVS]

@rawalter21, could you try using master now that we've reworked some of our ignore logic, to see if you're still having problems? I believe we've fixed it, but it would be great if you can confirm.

[BarryThePenguin]

@IanVS in my investigation today, it looks like brace expansion has been lost in the switch from minimatch to node-ignore. So now --ignore-pattern **/*.{test,mock}.js doesn't work. I can open a new issue if that's needed

[IanVS]

I don't think brace expansion is legal in a .gitignore file, is it? Since version 2.0.0 the .eslintignore should work the same as .gitignore. And --ignore-pattern is treated as a single line of an eslintignore file.

[BarryThePenguin]

Not according to the current eslint documentation

[IanVS]

Ah, good catch. That's a bug in the documentation. I just tested brace expansion in a .gitignore file and it does not work. So, to accomplish what you want, you can use:

eslint . --ignore-pattern *.test.js --ignore-pattern *.mock.js

If you'd like to submit an issue for the documentation issue, that would be great.

[BarryThePenguin]

Ah! Fantastic. This is a good answer. Thank you.

Out of curiosity, is there a reason the gitignore doesn't use brace expansion?

[IanVS]

As far as I understand, it isn't a valid glob on all posix operating systems.

[BarryThePenguin]

Sure. One more question, the related PR #5461 isn't released yet?

[BarryThePenguin]

The reason I ask is your above suggestion doesn't appear to work, even on master... Ignore me. I don't know how to use ignore patterns 😖