Using eslint support and seeing warnings for ignored files

[eamodio]

Current behavior

I've switched GitLens over to use the new eslint support (thanks for that!) but I'm running into a minor issue. At first, it was trying to lint one of my json files, but I added that file to my .eslintignore file. But it still seems to be trying to lint that file (and another file from node_modules which is ignored by default in eslint), which outputs a warning for each file -- saying that it is ignored

WARNING in C:\Users\Eric\code\vscode-gitlens\node_modules\vsls\vscode.ts
WARNING in C:\Users\Eric\code\vscode-gitlens\node_modules\vsls\vscode.ts(undefined,undefined):
    undefined: File ignored by default. Use "--ignore-pattern '!node_modules/*'" to override.

WARNING in C:\Users\Eric\code\vscode-gitlens\src\emojis.json
WARNING in C:\Users\Eric\code\vscode-gitlens\src\emojis.json(undefined,undefined):
    undefined: File ignored because of a matching ignore pattern. Use "--no-ignore" to override.

Expected behavior

I would expect no warnings to be output for those ignored files

Steps to reproduce the issue

I don't have a simple sample, but you can see it with the GitLens repo (on the develop branch)

1. Clone https://github.com/eamodio/vscode-gitlens.git
2. Checkout develop
3. Run npm install to install all the deps
4. Run npm run watch

Issue reproduction repository

https://github.com/eamodio/vscode-gitlens.git

Environment

• fork-ts-checker-webpack-plugin: 1.4.2
• typescript: 3.5.3
• eslint: 6.0.1
• webpack: 4.35.3
• os: Windows 10

[johnnyreilly]

Thanks for reporting @eamodio!

Yeah I think there's two issues here possibly:

1. We should be excluding all .json files from linting.
2. We should be excluding everything from node_modules from linting

The reason that you're seeing the errors is because of the way we invoke ESLint - we pass it specific files for linting which ESLint would typically ignore but that the TypeScript compiler does not. If you pass specific files to ESLint, even when they're files it should not lint, it gives it a go. See
https://eslint.org/docs/1.0.0/developer-guide/nodejs-api#executeonfiles

I'm on my phone right now, but would you be able to try something? Change the ApiIncrementalChecker here to something like this:

      if (
        this.isFileExcluded(updatedFile) ||
        updatedFile.endsWith('.json') || // we don't lint JSON
        updatedFile.includes('node_modules') // we don't lint node_modules
      ) {
        continue;
      }

Likewise the IncrementalChecker here

    const filesToLint = this.files
      .keys()
      .filter(
        filePath =>
          !this.files.getData(filePath).linted &&
          !IncrementalChecker.isFileExcluded(filePath, this.linterExclusions) &&
        !updatedFile.endsWith('.json') && // we don't lint JSON
        !updatedFile.includes('node_modules') // we don't lint node_modules

      );

Another thought, in the Node JS API which we use, there is an .isPathIgnored(path) method:

https://eslint.org/docs/1.0.0/developer-guide/nodejs-api#ispathignored

Possibly we should use this in createEslinter.ts like so:

      if (eslinter.isPathIgnored(filepath))
          return undefined;

      const lints = eslinter.executeOnFiles([filepath]);
      return lints;

Do you want to include that in your tweaks? Should hopefully mean that your ignores are, um, ignored 😄

If this works for you, would be able to submit a PR? I can help get that merged and released.

[eamodio]

@johnnyreilly I have to crash right now, but I will try that out tomorrow and let you know. Thanks!

[johnnyreilly]

Awesome - BTW there's one failing test on Windows. So if you're running your tests on Windows either ignore that or use WSL instead 😄

Note to self: make all tests cross platform again!

[piotr-oles]

🎉 This issue has been resolved in version 1.4.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀