fix!: Behavior of CLI when no arguments are passed #17644

nzakas · 2023-10-12T16:22:53Z

Prerequisites checklist

I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[x] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

When running the CLI without file arguments it silently fails right now. This PR makes the following changes:

When running the CLI in eslintrc mode, failing to use file arguments throws an error.
When running the CLI in flat config mode, failing to use file arguments means that it runs on the current working directory.

Fixes #14308

Is there anything you'd like reviewers to focus on?

To make this work, I changed how lintFiles() works in both ESLint and FlatESLint. In FlatESLint, I decided that an empty string and an empty array are the same and should be treated as .. I'm not sure if that's the right decision, so I'd like to hear some opinions on that.

netlify · 2023-10-12T16:23:01Z

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	`df221bb`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/658367ecab1c410007859b33

mdjermanovic · 2023-10-13T12:20:08Z

I decided that an empty string and an empty array are the same and should be treated as .. I'm not sure if that's the right decision, so I'd like to hear some opinions on that.

Makes sense to me.

lib/eslint/flat-eslint.js

lib/eslint/eslint-helpers.js

nzakas · 2023-10-13T15:32:10Z

Fixed up the edge cases. I also made it so FlatESLint#lintFiles() doesn't require an argument, and if it's missing, then we use ["."]. This is to have no argument behave the same as "" and [], which makes logical sense to me.

fasttime · 2023-10-13T15:58:27Z

Let me point out a use case that I think will be negatively impacted by this change. Sometimes people run ESLint only on a subset of files that is calculated dynamically and that could be possibly empty. For example, for performance reasons, some users will want to lint only changed files in a git repo, so they could be running a command like this (reference):

eslint --fix $(git diff --name-only HEAD | xargs)

After this PR, if a user runs the above command while there are no uncommitted changes, the whole current directory will be linted (or they will get an error if eslintrc is used).

nzakas · 2023-10-13T16:01:03Z

@fasttime for clarification, you're referring to the flat config behavior, which defaults to . and not to the eslintrc behavior, which throws an error.

I assume that ESLint isn't the only tool people are using in this way. There are a lot of tools that default to . when no file arguments are present. How do other tools handle this use case?

fasttime · 2023-10-13T16:22:55Z

I assume that ESLint isn't the only tool people are using in this way. There are a lot of tools that default to . when no file arguments are present. How do other tools handle this use case?

I'd guess that people are using ESLint like this simply because they were able do it so far. npx prettier prints usage info and exits with an error code. The same happens when I run javac (the Java compiler binary) without arguments. On the other hand, ls without arguments just lists the contents of the current directory, so probably there is no standard way to handle this edge case.

nzakas · 2023-10-16T16:01:46Z

I think jest works in such a way that you can either pass no arguments or specific tests? Maybe there's some way to set that up for a precommit hook that we can learn from? (I'm not familiar with Jest, so looking for some insights.)

fasttime · 2023-10-18T12:55:12Z

In fact running jest without arguments prints an error message and exits with an error code:

PS D:\foo> npx jest                  
No tests found, exiting with code 1
Run with `--passWithNoTests` to exit with code 0
In D:\foo
  8 files checked.
  testMatch: **/__tests__/**/*.[jt]s?(x), **/?(*.)+(spec|test).[tj]s?(x) - 0 matches
  testPathIgnorePatterns: \\node_modules\\ - 8 matches
  testRegex:  - 0 matches
Pattern:  - 0 matches

But this behavior can be changed with a flag (--passwithnotests):

PS D:\foo> npx jest --passWithNoTests
No tests found, exiting with code 0

fasttime · 2023-10-18T13:19:17Z

Maybe there's some way to set that up for a precommit hook that we can learn from?

It seems that lint-staged does not run tasks for glob patterns that don't match any staged file, so at least it shouldn't be a concern for this change.

nzakas · 2023-10-18T16:08:12Z

Jest is failing without tests because it didn't find any in the directories that it searches by default. It seems like --passwithnotests might be the best thing to copy.

nzakas · 2023-10-18T16:13:09Z

TSC Summary: This PR changes the default behavior of the ESLint CLI when no file arguments are passed in. In eslintrc mode, the CLI now reports an error; in flat config mode, the CLI now assumes . as the file argument. This may cause a problem for people trying to use the CLI as part of a precommit check. Before this change, the CLI would exit with a zero exit code when no file arguments were present, after this change, it will either error or lint every file, neither of which seems desirable.

TSC Questions:

This now looks like a breaking change, should we add it to v9? (This was originally intended to be completed with the flat config work, so it should have been included?)
Do we want to provide a way to mimic the current behavior? Jest has a --passwithnotests CLI flag that we could mimic for this purpose.

github-actions · 2023-10-30T22:31:16Z

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

nzakas · 2023-11-30T21:23:08Z

@sam3k please be sure to leave comments as to the resolution from the TSC meeting along with removing the "tsc agenda" label.

sam3k · 2023-11-30T21:34:39Z

@sam3k please be sure to leave comments as to the resolution from the TSC meeting along with removing the "tsc agenda" label.

Sounds good

nzakas · 2023-12-06T16:10:36Z

@mdjermanovic this is ready for review

lib/eslint/eslint.js

lib/options.js

docs/src/use/command-line-interface.md

tests/lib/eslint/eslint.js

tests/lib/cli.js

lib/eslint/flat-eslint.js

mdjermanovic · 2023-12-08T20:51:00Z

lib/eslint/flat-eslint.js

-        if (!isNonEmptyString(patterns) && !isArrayOfNonEmptyString(patterns)) {
-            throw new Error("'patterns' must be a non-empty string or an array of non-empty strings");
-        }
+    async lintFiles(patterns = ["."]) {


Since the default is applied here, passOnNoPatterns has no effect when lintFiles is called without arguments.

const eslint = new FlatESLint({ passOnNoPatterns: true }); eslint.lintFiles(""); // lints nothing eslint.lintFiles([]); // lints nothing eslint.lintFiles(); // lints "."

Is that intentional? Might be surprising behavior for the option named "pass on no patterns" as in this case no patterns were provided.

Hmm, that's a good point. I thought API-wise it might make sense to match the CLI behavior when no arguments are passed, but I can see how this can be confusing.

docs/src/use/command-line-interface.md

Fixes #14308

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

linux-foundation-easycla · 2023-12-20T18:48:44Z

The committers listed above are authorized under a signed CLA.

✅ login: nzakas / name: Nicholas C. Zakas (ce58a6a, 061d8c7, 4142780, 3b7ba23, 4049555, 6134a45, bbf82b2, 06b5b91, d2de70c, 70763c4, 25ff62a, 8efb00f, df221bb)
✅ login: mdjermanovic / name: Milos Djermanovic (6134a45, bbf82b2, 06b5b91, d2de70c, 70763c4, 8efb00f)

nzakas · 2023-12-20T18:51:35Z

Argh, totally messed up the rebase on this one. :(

nzakas · 2023-12-20T22:25:37Z

I believe this is ready.

mdjermanovic

LGTM, thanks!

* main: (25 commits) test: ensure that CLI tests run with FlatESLint (eslint#17884) fix!: Behavior of CLI when no arguments are passed (eslint#17644) docs: Update README Revert "feat!: Remove CodePath#currentSegments" (eslint#17890) feat!: Update shouldUseFlatConfig and CLI so flat config is default (eslint#17748) feat!: Remove CodePath#currentSegments (eslint#17756) chore: update dependency markdownlint-cli to ^0.38.0 (eslint#17865) feat!: deprecate no-new-symbol, recommend no-new-native-nonconstructor (eslint#17710) feat!: check for parsing errors in suggestion fixes (eslint#16639) feat!: assert suggestion messages are unique in rule testers (eslint#17532) feat!: `no-invalid-regexp` make allowConstructorFlags case-sensitive (eslint#17533) fix!: no-sequences rule schema correction (eslint#17878) feat!: Update `eslint:recommended` configuration (eslint#17716) feat!: drop support for string configurations in flat config array (eslint#17717) feat!: Remove `SourceCode#getComments()` (eslint#17715) feat!: Remove deprecated context methods (eslint#17698) feat!: Swap FlatESLint-ESLint, FlatRuleTester-RuleTester in API (eslint#17823) feat!: remove formatters except html, json(-with-metadata), and stylish (eslint#17531) feat!: Require Node.js `^18.18.0 || ^20.9.0 || >=21.1.0` (eslint#17725) fix: allow circular references in config (eslint#17752) ...

nzakas requested a review from a team as a code owner October 12, 2023 16:22

eslint-github-bot bot added the bug ESLint is working incorrectly label Oct 12, 2023

mdjermanovic added core Relates to ESLint's core APIs and features accepted There is consensus among the team that this change meets the criteria for inclusion labels Oct 13, 2023

mdjermanovic reviewed Oct 13, 2023

View reviewed changes

lib/eslint/flat-eslint.js Outdated Show resolved Hide resolved

lib/eslint/eslint-helpers.js Show resolved Hide resolved

nzakas added tsc agenda This issue will be discussed by ESLint's TSC at the next meeting breaking This change is backwards-incompatible labels Oct 18, 2023

nzakas marked this pull request as draft October 19, 2023 20:15

sam3k removed the tsc agenda This issue will be discussed by ESLint's TSC at the next meeting label Oct 20, 2023

github-actions bot added the Stale label Oct 30, 2023

fasttime removed the Stale label Oct 31, 2023

nzakas self-assigned this Oct 31, 2023

sam3k mentioned this pull request Dec 1, 2023

docs: Add 2023-11-30 meeting notes eslint/tsc-meetings#472

Merged

mdjermanovic changed the title ~~fix: Behavior of CLI when no arguments are passed~~ fix!: Behavior of CLI when no arguments are passed Dec 8, 2023

mdjermanovic reviewed Dec 8, 2023

View reviewed changes

mdjermanovic reviewed Dec 19, 2023

View reviewed changes

docs/src/use/command-line-interface.md Outdated Show resolved Hide resolved

nzakas marked this pull request as ready for review December 20, 2023 18:38

nzakas and others added 12 commits December 20, 2023 13:43

fix: Behavior of CLI when no arguments are passed

ce58a6a

Fixes #14308

Removed unnecessary check

061d8c7

Fix edge cases

4142780

Add --pass-on-no-patterns flag

3b7ba23

Rename argument

4049555

Update lib/options.js

6134a45

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Update docs/src/use/command-line-interface.md

bbf82b2

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Update tests/lib/eslint/eslint.js

06b5b91

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Update lib/eslint/flat-eslint.js

d2de70c

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Update tests/lib/cli.js

70763c4

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

lintFiles() should not have default param

25ff62a

Update docs/src/use/command-line-interface.md

8efb00f

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

Fix merge bug

df221bb

nzakas force-pushed the issue14308 branch from 2917405 to df221bb Compare December 20, 2023 22:17

mdjermanovic approved these changes Dec 21, 2023

View reviewed changes

mdjermanovic merged commit 12be307 into main Dec 21, 2023
17 checks passed

mdjermanovic deleted the issue14308 branch December 21, 2023 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix!: Behavior of CLI when no arguments are passed #17644

fix!: Behavior of CLI when no arguments are passed #17644

nzakas commented Oct 12, 2023

netlify bot commented Oct 12, 2023 •

edited

mdjermanovic commented Oct 13, 2023

nzakas commented Oct 13, 2023

fasttime commented Oct 13, 2023

nzakas commented Oct 13, 2023

fasttime commented Oct 13, 2023

nzakas commented Oct 16, 2023

fasttime commented Oct 18, 2023

fasttime commented Oct 18, 2023

nzakas commented Oct 18, 2023

nzakas commented Oct 18, 2023 •

edited

github-actions bot commented Oct 30, 2023

nzakas commented Nov 30, 2023

sam3k commented Nov 30, 2023

nzakas commented Dec 6, 2023

mdjermanovic Dec 8, 2023

nzakas Dec 14, 2023

linux-foundation-easycla bot commented Dec 20, 2023 •

edited

nzakas commented Dec 20, 2023

nzakas commented Dec 20, 2023

mdjermanovic left a comment

fix!: Behavior of CLI when no arguments are passed #17644

fix!: Behavior of CLI when no arguments are passed #17644

Conversation

nzakas commented Oct 12, 2023

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

What changes did you make? (Give an overview)

Is there anything you'd like reviewers to focus on?

netlify bot commented Oct 12, 2023 • edited

✅ Deploy Preview for docs-eslint canceled.

mdjermanovic commented Oct 13, 2023

nzakas commented Oct 13, 2023

fasttime commented Oct 13, 2023

nzakas commented Oct 13, 2023

fasttime commented Oct 13, 2023

nzakas commented Oct 16, 2023

fasttime commented Oct 18, 2023

fasttime commented Oct 18, 2023

nzakas commented Oct 18, 2023

nzakas commented Oct 18, 2023 • edited

github-actions bot commented Oct 30, 2023

nzakas commented Nov 30, 2023

sam3k commented Nov 30, 2023

nzakas commented Dec 6, 2023

mdjermanovic Dec 8, 2023

Choose a reason for hiding this comment

nzakas Dec 14, 2023

Choose a reason for hiding this comment

linux-foundation-easycla bot commented Dec 20, 2023 • edited

nzakas commented Dec 20, 2023

nzakas commented Dec 20, 2023

mdjermanovic left a comment

Choose a reason for hiding this comment

netlify bot commented Oct 12, 2023 •

edited

nzakas commented Oct 18, 2023 •

edited

linux-foundation-easycla bot commented Dec 20, 2023 •

edited