oxsecurity / megalinter

🦙 MegaLinter analyzes 50 languages, 22 formats, 21 tooling formats, excessive copy-pastes, spelling mistakes and security issues in your repository sources with a GitHub Action, other CI tools or locally.
https://megalinter.io
GNU Affero General Public License v3.0
1.81k stars 215 forks source link

Unable to use ESLint flat config for linting JavaScript #3570

Open theetrain opened 1 month ago

theetrain commented 1 month ago

Describe the bug

I tried pointing MegaLinter to my eslint.config.js, but it yielded the error:

❌ Linted [JAVASCRIPT] files with [eslint]: Found 1 error(s) - (1.49s)
- Using [eslint v8.57.0] https://megalinter.io/7.11.1/descriptors/javascript_eslint
- MegaLinter key: [JAVASCRIPT_ES]
- Rules config: [/eslint.config.js]
- Number of files analyzed: [20]
--Error detail:
Invalid option '--eslintrc' - perhaps you meant '--ignore'?
You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details.

To Reproduce

  1. Add these settings to .mega-linter.yml
JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.js
  1. Run megalinter

Expected behavior

Flat ESLint config file is picked up, and runs without error.

ethanchristensen01 commented 1 month ago

I tried getting around this by removing the --eslintrc option.

# .mega-linter.yml
# ...other configs
JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.mjs
JAVASCRIPT_ES_COMMAND_REMOVE_ARGUMENTS: ["--eslintrc"]

When I run the linter with this configuration, I get this error:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/megalinter/run.py", line 14, in <module>
    linter.run()
  File "/megalinter/MegaLinter.py", line 245, in run
    self.process_linters_serial(active_linters)
  File "/megalinter/MegaLinter.py", line 297, in process_linters_serial
    linter.run()
  File "/megalinter/Linter.py", line 793, in run
    return_code, stdout = self.process_linter()
                          ^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/Linter.py", line 936, in process_linter
    command = self.build_lint_command(file)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/linters/EslintLinter.py", line 12, in build_lint_command
    cmd = super().build_lint_command(file)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/megalinter/Linter.py", line 1286, in build_lint_command
    cmd.remove(arg)
ValueError: list.remove(x): x not in list
ethanchristensen01 commented 1 month ago

I found that replacing --eslintrc with --no-eslintrc fixed that error, but I got another one:

- Using [eslint v8.57.0] https://megalinter.io/7.11.1/descriptors/javascript_eslint
- MegaLinter key: [JAVASCRIPT_ES]
- Rules config: [/eslint.config.mjs]
- Ignore file: [/.eslintignore]
- Number of files analyzed: [305]
--Error detail:
Invalid option '--ignore-path' - perhaps you meant '--ignore-pattern'?
You're using eslint.config.js, some command line flags are no longer available. Please see https://eslint.org/docs/latest/use/command-line-interface for details.

It seems javascript.megalinter-descriptor defines the ignore option to be --ignore-path, which doesn't work for flat config.

--ignore-pattern is valid for both configs, but it requires a list of globs instead of an ignore file.

echoix commented 1 month ago

Is the flat config supported before version 9 of eslint? I see the last release of Megalinter had eslint 8.57.0.

ethanchristensen01 commented 1 month ago

Yes, the eslint website has a migration guide which mentions the flat config is the default in eslint 9, but is also available in eslint 8.

echoix commented 1 month ago

In that migration guide, I read that using an ignore file isn't supported anymore. That would explain why --ignore-path doesn't work. They say to

https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files

The old way of ignoring (what Megalinter is still set up for), has docs here: https://eslint.org/docs/latest/use/configure/ignore-deprecated

You can try to extrapolate what config is created by the linter's descriptor here (or use debug logging if possible to see what would be called):

https://github.com/oxsecurity/megalinter/blob/c0409fa8f071903bd51621800f7aad68869687ec/megalinter/descriptors/javascript.megalinter-descriptor.yml#L31-L53

And finally, at the end of all this, I'd like to know your experience on if it would be possible to support both current config and flat config in Megalinter, or we need to force all users to use v9 and migrate their configs+project configs and those who can't would need to use an older Megalinter. Since the eslintignore doesn't exist anymore, is there a way we could have a working base preset/config for users that don't have a config yet?

ethanchristensen01 commented 1 month ago

It depends on what we can do in EslintLinter.py

ESlint v8 and v9 still support both formats, the default just changed between the two versions. We can set an environment variable to override it (ESLINT_USE_FLAT_CONFIG=false).

Figuring out which format a user wants could be done in a few ways:

(A flat config file has the name "eslint.config.[mc]?js")

Is there currently a default .eslintignore? Does the user's .eslintignore override it, or extend it?

nvuillam commented 1 month ago

I knew the topic about eslint 9 would come at some point, I've not completely understood how required is the use of eslint.config.js as in beta the test cases still work :/

ethanchristensen01 commented 1 month ago

Frankly, I knew nothing about the flat config until just a few days ago. It seems like support for eslintrc will be dropped when v10 releases, so megalinter may eventually need to support two versions of eslint. 😕

Maybe the FlatCompat utility can help?

ethanchristensen01 commented 1 month ago

So, I have some good news.

Under the right circumstances, this configuration does currently work:

JAVASCRIPT_ES_CONFIG_FILE: ./eslint.config.mjs # or .js, .cjs
JAVASCRIPT_ES_COMMAND_REMOVE_ARGUMENTS: ["--no-eslintrc"] # Not valid option for eslint with flat config

Here are the requirements:

  1. There must be no .eslintignore in the root directory.

    • Having .eslintignore causes the Invalid option '--ignore-path' error
  2. ESLINT_FLAT_CONFIG (as an environment variable) must be true

    • If it's false, an error occurs which depends on the config. I was getting this:
      YAMLException: Cannot read config file: /tmp/lint/eslint.config.mjs
      Error: end of the stream or a document separator is expected

      This is because the old eslintrc parser is trying (and failing) to read the new config file.

    • Right now, I'm running megalinter through the npx tool. This is how I set the environment variable:
      npx mega-linter-runner -e "ENABLE_LINTERS=JAVASCRIPT_ES" -e "ESLINT_USE_FLAT_CONFIG=true"

      It would be nice to know if this can be set in .mega-linter.yml :)

This solution, although hacky, should work for supporting flat config in ESlint 8.

nvuillam commented 1 month ago

@ethanchristensen01 thanks for the analysis :)

What is your suggestion so we can by default use eslint.config.js by default according to eslint 9 and still avoid a crash for MegaLinter users who have eslint 8 config ? ( we can play with python to detect files at the root of the repo and use different arguments / env vars depending what we find ^^ )

echoix commented 1 month ago

It's weird that you had to use the env var ESLINT_USE_FLAT_CONFIG and also have your eslint.config.js at the root of the repo. The docs specifically say that it is "or".

https://eslint.org/docs/latest/use/configure/migration-guide#start-using-flat-config-files

That might be something on our end, where if there isn't any valid config files (we need to add more files names), we add a default file that makes it so the repo has both files in the root of the repo.

ethanchristensen01 commented 1 month ago

@echoix You might be right. Adding .eslintrc and .eslintignore might be causing that.

I noticed these errors at the bottom of my lint logs.

[Updated Sources Reporter] Unable to copy .eslintignore to /tmp/lint/megalinter-reports/updated_sources/.eslintignore ([Errno 2] No such file or directory: '.eslintignore')
[Updated Sources Reporter] Unable to copy MyProject/.eslintignore to /tmp/lint/megalinter-reports/updated_sources/MyProject/.eslintignore ([Errno 2] No such file or directory: 'MyProject/.eslintignore')
[Updated Sources Reporter] copied X fixed source files in folder /tmp/lint/megalinter-reports/updated_sources.

(last line added for context)

This might suggest there's a file cache of some kind, and it just happens to be working for me because it thinks I still have a .eslintignore in my project. That might prevent the default .eslintignore from getting copied in. (totally just a guess, haven't tested this theory yet)

@nvuillam Good question!

For now, I'm assuming we're going to add a configuration variable like JAVASCRIPT_ES_CONFIG_USE_FLAT_FILE.

I made a flowchart that should hopefully cover all cases!

flowchart TD;
    start(((Start)))
    q0["Has the current ESLint version abandoned eslintrc support?"]
    y0[["Use flat config"]]
    q1["Is the 'JAVASCRIPT_ES_USE_FLAT_CONFIG' configuration variable set?"]
    y1q1["Is that variable true?"]
    y1y1[["Use flat config"]]
    y1n1[["Use eslintrc config"]]
    q2["Is the environment variable 'ESLINT_USE_FLAT_CONFIG' set?"]
    q3["Is the configuration variable 'JAVASCRIPT_ES_CONFIG_FILE' set?"]
    y3q1["Is it eslint.config.{js, cjs, mjs}?"]
    y3y1[["Use flat config"]]
    y3n1q1["Are we using ESLint 9+?"]
    y3n1y1[["Use flat config"]]
    y3n1n1[["Use eslintrc config"]]
    q4["Is eslint.config.{js, cjs, mjs} in the project root?"]
    y4[["Use flat config"]]
    q5["Is .eslintrc* in the project root?"]
    y5[["Use eslintrc config"]]

    start --> q0
    q0 -->|Yes| y0
    q0 -.->|No| q1
    q1 -->|Yes| y1q1
    q1 -.->|No| q2
    y1q1 -->|Yes| y1y1
    y1q1 -.->|No| y1n1
    q2 -->|Yes| y1q1
    q2 -.->|No| q3
    q3 -->|Yes| y3q1
    q3 -.->|No| q4
    y3q1 -->|Yes| y3y1
    y3q1 -.->|No| y3n1q1
    y3n1q1 -->|Yes| y3n1y1
    y3n1q1 -.->|No| y3n1n1
    q4 -->|Yes| y4
    q4 -.->|No| q5
    q5 -->|Yes| y5
    q5 -.->|No| y3n1q1
nvuillam commented 1 month ago

@ethanchristensen01 amazing ! Let's implement that for new release :) ( after the one of today ^^ )

github-actions[bot] commented 2 days ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

If you think this issue should stay open, please remove the O: stale 🤖 label or comment on the issue.

echoix commented 2 days ago

I saw a discussion pass by on this in the super-linter repo.