Fix: ensure config files are files #42

tjenkinson · 2021-07-03T15:14:34Z

Currently if a directory name matches a config file it will be treated as a file and then fail later when we try and read it.

This is causing an error for us in a fixture where we have a directory called "package.json".

Refs rollup/plugins#927

eslint-github-bot · 2021-07-03T15:14:38Z

Hi @tjenkinson!, thanks for the Pull Request

The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.
There should be a space following the initial tag and colon, for example 'New: Message'.
The first letter of the tag should be in uppercase

Read more about contributing to ESLint here

tjenkinson · 2021-07-03T15:15:54Z

@nzakas @mdjermanovic please could someone click the button to run the workflow?

tjenkinson · 2021-07-03T15:16:46Z

Locally I'm getting

1) ConfigArrayFactory
       loading config files should work properly.
         Plugins
           should load information from a YML file and load plugins:
     YAMLException: Cannot read config file: /private/var/folders/c1/5j985n355jgdqhvpwwvvhpc00000gn/T/eslintrc/config-array-factory/plugins/.eslintrc.yml
Error: bad indentation of a mapping entry at line 3, column 29:
                                rules:
                                ^
      at generateError (node_modules/js-yaml/lib/js-yaml/loader.js:167:10)

even on 'main' when trying to run the tests

eslint-github-bot · 2021-07-03T15:23:02Z

Hi @tjenkinson!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.
There should be a space following the initial tag and colon, for example 'New: Message'.
The first letter of the tag should be in uppercase

Read more about contributing to ESLint here

tjenkinson · 2021-07-03T15:26:32Z

lib/config-array-factory.js

@@ -495,7 +495,7 @@ class ConfigArrayFactory {
                basePath
            );

-            if (fs.existsSync(ctx.filePath)) {
+            if (fs.existsSync(ctx.filePath) && fs.statSync(ctx.filePath).isFile()) {


a similar check is done here:

https://github.com/eslint/eslint/blob/e837d811e9275e43ccc266aba10e9c0833e0376c/lib/init/npm-utils.js#L33

tjenkinson · 2021-07-03T15:43:24Z

#44 should fix the test failure

eslint-github-bot · 2021-07-03T17:08:35Z

Hi @tjenkinson!, thanks for the Pull Request

The pull request title isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.

The commit message tag must be one of the following:

The Tag is one of the following:
- Fix - for a bug fix.
- Update - either for a backwards-compatible enhancement or for a rule change that adds reported problems.
- New - implements a new feature.
- Breaking - for a backwards-incompatible enhancement or feature.
- Docs - changes to documentation only.
- Build - changes to build process only.
- Upgrade - for a dependency upgrade.
- Chore - for anything that isn't user-facing (for example, refactoring, adding tests, etc.).
You can use the labels of the issue you are working on to determine the best tag.
There should be a space following the initial tag and colon, for example 'New: Message'.
The first letter of the tag should be in uppercase

Read more about contributing to ESLint here

Currently if a directory name matches a config file it will be treated as a file and then fail later when we try and read it.

mdjermanovic

Thanks for the PR, I can reproduce this bug with directories named package.json, .eslintrc etc.

The change LGTM, thanks!

no longer needed since eslint/eslintrc#42 and eslint/eslint#14808

eslint-github-bot bot added the triage label Jul 3, 2021

tjenkinson force-pushed the ensure-config-files-are-files branch from 299adf7 to 20ebd85 Compare July 3, 2021 15:25

tjenkinson commented Jul 3, 2021

View reviewed changes

tjenkinson marked this pull request as ready for review July 3, 2021 15:27

Fix: ensure confg files are files

4c77284

Currently if a directory name matches a config file it will be treated as a file and then fail later when we try and read it.

tjenkinson force-pushed the ensure-config-files-are-files branch from 19ae64b to 4c77284 Compare July 3, 2021 17:08

mdjermanovic changed the title ~~ensure config files are files~~ Fix: ensure config files are files Jul 3, 2021

mdjermanovic added accepted bug Something isn't working and removed triage labels Jul 3, 2021

mdjermanovic approved these changes Jul 3, 2021

View reviewed changes

nzakas approved these changes Jul 6, 2021

View reviewed changes

nzakas merged commit ce78027 into eslint:main Jul 6, 2021

tjenkinson mentioned this pull request Aug 19, 2021

Remove .eslintignore workaround and update eslint to at least 7.31.0 rollup/plugins#977

Open

tjenkinson added a commit to rollup/plugins that referenced this pull request Aug 21, 2021

remove eslint workaoround

94cb301

no longer needed since eslint/eslintrc#42 and eslint/eslint#14808

tjenkinson added a commit to rollup/plugins that referenced this pull request Aug 21, 2021

remove eslint workaround

9e70b29

no longer needed since eslint/eslintrc#42 and eslint/eslint#14808

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: ensure config files are files #42

Fix: ensure config files are files #42

tjenkinson commented Jul 3, 2021 •

edited

eslint-github-bot bot commented Jul 3, 2021

tjenkinson commented Jul 3, 2021

tjenkinson commented Jul 3, 2021 •

edited

eslint-github-bot bot commented Jul 3, 2021

tjenkinson Jul 3, 2021

tjenkinson commented Jul 3, 2021

eslint-github-bot bot commented Jul 3, 2021

mdjermanovic left a comment

Fix: ensure config files are files #42

Fix: ensure config files are files #42

Conversation

tjenkinson commented Jul 3, 2021 • edited

eslint-github-bot bot commented Jul 3, 2021

tjenkinson commented Jul 3, 2021

tjenkinson commented Jul 3, 2021 • edited

eslint-github-bot bot commented Jul 3, 2021

tjenkinson Jul 3, 2021

Choose a reason for hiding this comment

tjenkinson commented Jul 3, 2021

eslint-github-bot bot commented Jul 3, 2021

mdjermanovic left a comment

Choose a reason for hiding this comment

tjenkinson commented Jul 3, 2021 •

edited

tjenkinson commented Jul 3, 2021 •

edited