Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: (eslintignore breaks when cwd is using forward slash on windows) #17042

Closed
1 task
noe132 opened this issue Mar 30, 2023 · 10 comments · Fixed by #17277
Closed
1 task

Bug: (eslintignore breaks when cwd is using forward slash on windows) #17042

noe132 opened this issue Mar 30, 2023 · 10 comments · Fixed by #17277
Assignees
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features repro:yes

Comments

@noe132
Copy link

noe132 commented Mar 30, 2023

Environment

Node version: 19.7.0
npm version: 9.5.0
Local ESLint version: 8.37.0
Global ESLint version: N/A
Operating System: windows 10 x64

What parser are you using?

Default (Espree)

What did you do?

When passing cwd option to eslint with a path with forward slash on windows (C:/path/to/project), eslintignore breaks and errors were reported from ignored files.

folder structure:

src/1.js
src/dist/2.js
.eslintignore
.eslintrc.js
lint.js
src/1.js
console.log(1)
src/dist/2.js
console.log(2)
.eslintignore
src/dist
lint.js
const eslint = require('eslint');

const run = async (cwd) => {
  // 1. Create an instance.
  const instance = new eslint.ESLint({ cwd, extensions: ['.js'] });

  // 2. Lint files.
  const results = await instance.lintFiles(["./src"]);

  // 3. Format the results.
  const formatter = await instance.loadFormatter("stylish");
  const resultText = formatter.format(results);

  // 4. Output it.
  console.log(resultText);
};

const main = async () => {
  await run(__dirname);
  console.log('-----------');
  await run(__dirname.replaceAll("\\", "/"));
};

main();
.eslintrc.js
module.exports = {
  parserOptions: {
    ecmaVersion: 2020,
  },
  'root': true,
  'rules': {
    'semi': 'error',
  },
};
result
C:\src\eslinttest\src\1.js
  1:15  error  Missing semicolon  semi

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

-----------

C:\src\eslinttest\src\1.js
  1:15  error  Missing semicolon  semi

C:\src\eslinttest\src\dist\2.js
  1:15  error  Missing semicolon  semi

✖ 2 problems (2 errors, 0 warnings)
  2 errors and 0 warnings potentially fixable with the `--fix` option.

What did you expect to happen?

eslintignore working as expected

What actually happened?

linting ignored files

Participation

  • I am willing to submit a pull request for this issue.

Additional comments

Where I get this problem? Here.
I'm using vite-plugin-checker on my project. It pass a root param recieved from vite to eslint cwd option which happens to be a path with forward slash on windows.

@noe132 noe132 added bug ESLint is working incorrectly repro:needed labels Mar 30, 2023
@mdjermanovic mdjermanovic added repro:yes core Relates to ESLint's core APIs and features and removed repro:needed labels Mar 31, 2023
@mdjermanovic
Copy link
Member

Hi @noe132!

Thanks for the detailed description. I was able to reproduce this.

The problem seems to be that function getCommonAncestorPath expects given paths to have path.sep as the path separator, but gets the exact cwd that was passed into the ESLint constructor, in this case with forward slashes, and also gets a few more paths with backslashes from the system, and then doesn't work well with that mix.

I think the easiest solution would be to update the ESLint class to path.normalize() the cwd argument right away, and from that point on work with the normalized version only. @nzakas what do you think?

@noe132 noe132 changed the title Bug: (eslintignore breaks when cwd is using backslash on windows) Bug: (eslintignore breaks when cwd is using forward slash on windows) Mar 31, 2023
@nzakas
Copy link
Member

nzakas commented Mar 31, 2023

That seems reasonable, though I'm curious if this is also an issue for FlatESLint?

@github-actions
Copy link

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Apr 30, 2023
@nzakas
Copy link
Member

nzakas commented May 3, 2023

@mdjermanovic following up here, do you think this is also an issue for FlatESLint?

@github-actions github-actions bot removed the Stale label May 3, 2023
@github-actions
Copy link

github-actions bot commented Jun 2, 2023

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Jun 2, 2023
@Rec0iL99
Copy link
Member

Rec0iL99 commented Jun 5, 2023

@mdjermanovic following up here, do you think this is also an issue for FlatESLint?

@mdjermanovic waiting for your response on this. Thanks.

@Rec0iL99 Rec0iL99 removed the Stale label Jun 5, 2023
@fasttime
Copy link
Member

fasttime commented Jun 5, 2023

FlatESLint does not care about .eslintignore files, but it seems to me that the supported ways to ignore stuff with the flat config all behave well on Windows even when cwd contains forward slashes.

  • with ignorePatterns: ['src/dist'] in the FlatESLint constructor options.
  • with a global ignores pattern in the config: { ignores: ['src/dist'] }.
  • with a non-global ignores pattern like ignores: ['src/dist/**'].
  • with a negated files pattern like files: ['!src/dist/**'].

Maybe there is more to test, but so far I could not reproduce the issue described here with the flat config.

@mdjermanovic
Copy link
Member

I didn't manage to find any observable problems either, but it might be good to do path.normalize() anyway so that rules can always expect context.cwd with local path separators.

@snitin315
Copy link
Contributor

Marking this as accepted :)

@snitin315 snitin315 added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jun 10, 2023
@mdjermanovic
Copy link
Member

I can work on this.

@mdjermanovic mdjermanovic self-assigned this Jun 12, 2023
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Dec 12, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Dec 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly core Relates to ESLint's core APIs and features repro:yes
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants