Exit code is 1 even when all violations are marked as TODOs

[bartocc]

Steps to reproduce

UPDATE_TODO=1 yarn eslint . --format @lint-todo/eslint-formatter-todo --quiet
yarn eslint . --format @lint-todo/eslint-formatter-todo --quiet

echo $? # => 1

This causes my CI to fail, and I don't think the env var FORMAT_TODO_AS will be of any help here 🧐

Am I missing something?

Using ESLint 8.25 in an ember.js 4.3 project

[scalvert]

Hey @bartocc, sorry you're having an issue. Let me take a peek and see if I can reproduce.

[scalvert]

It looks like a PR was merged 25 days ago that changes the way errors are calculated, and thus what exit code is subsequently reported. I'll have to think on this a bit to see if there's any way to address this.

[scalvert]

To unblock yourself @bartocc, you could pin eslint@8.24.0 to ensure the old behavior is maintained.

[bartocc]

hey @scalvert , thx for investigating this!
I confirm pinning eslint@8.24.0 is a good workaround 👍

Hope you'll find the proper fix for this!

Thx again

[jackjennings]

I noticed that --quiet was the culprit for me. The application that I'm working on uses eslint 8.7.0 at the moment, and also had the exit code 1. Removing the --quiet flag resolved the issue and allowed this library to work correctly.

[bartocc]

I noticed that --quiet was the culprit for me. The application that I'm working on uses eslint 8.7.0 at the moment, and also had the exit code 1. Removing the --quiet flag resolved the issue and allowed this library to work correctly.

I'll have to double check @jackjennings , but I believe that with eslint@8.25, with or without --quiet, the exit code was 1

2022-11-16 edit:
I DO see exit code 1 with the --quiet flag

[scalvert]

I opened an issue in eslint. I can utilize the workaround, but it feels wrong to exit in a formatter directly.

[steveszc]

I'm seeing exit code 1 when using --quiet even on eslint@8.24, but it exits 0 without the --quiet flag.

[steveszc]

I think the --quiet issue may be a seperate bug...
Looks like when the --quiet flag is present the errorCount calculation is based on non-warning messages (which includes todos) rather than only error messages. This results in the --quiet output showing all the todos and the todos show up in the error count.

eslint-formatter-todo/src/formatter.ts

Line 278 in 79792ef

return message.severity !== Severity.warn;

[steveszc]

In addition to previous comment, the issue with --quiet always exiting non-zero is an issue in eslint itself, similar to the issue that is causing the non-zero exit in 8.25+

When the --quiet flag is present eslint filters out non-error results before passing the results into the formater, which means eslint-formatter-todo never even sees the errors to mutate them into todos, but when eslint later calculates the error code it does so based on the original unfiltered results.

https://github.com/eslint/eslint/blob/740b20826fadc5322ea5547c1ba41793944e571d/lib/cli.js#L427-L443

[rondale-sc]

@steveszc I think I ran into this also. The solution for me at present is to lock to 8.24 until @scalvert updates his ticket with eslint anyways. 👍

Thank you for posting this update to help me validate what I was seeing :)

[steveszc]

FYI I was able to get zero exit codes with and without --quiet by pinning to eslint@8.24.0 and adding the following two patch-package to get --quiet working.

// patches/eslint+8.24.0.patch

diff --git a/node_modules/eslint/lib/cli.js b/node_modules/eslint/lib/cli.js
index 2fca65c..1cfcd2b 100644
--- a/node_modules/eslint/lib/cli.js
+++ b/node_modules/eslint/lib/cli.js
@@ -407,7 +407,7 @@ const cli = {
             resultsToPrint = ActiveESLint.getErrorResults(resultsToPrint);
         }
 
-        if (await printResults(engine, resultsToPrint, options.format, options.outputFile)) {
+        if (await printResults(engine, results, options.format, options.outputFile)) {
 
             // Errors and warnings from the original unfiltered results should determine the exit code
             const { errorCount, fatalErrorCount, warningCount } = countErrors(results);

// patches/@lint-todo+eslint-formatter-todo+4.0.3.patch

diff --git a/node_modules/@lint-todo/eslint-formatter-todo/lib/formatter.js b/node_modules/@lint-todo/eslint-formatter-todo/lib/formatter.js
index 32eef67..21bca13 100644
--- a/node_modules/@lint-todo/eslint-formatter-todo/lib/formatter.js
+++ b/node_modules/@lint-todo/eslint-formatter-todo/lib/formatter.js
@@ -192,9 +192,10 @@ function pushResult(results, todo) {
 }
 function filterResults(results) {
     const filtered = [];
+
     results.forEach((result) => {
         const filteredMessages = result.messages.filter((message) => {
-            return message.severity !== utils_1.Severity.warn;
+            return message.severity === utils_1.Severity.error;
         });
         if (filteredMessages.length > 0) {
             filtered.push({
@@ -207,6 +208,7 @@ function filterResults(results) {
             });
         }
     });
+
     return filtered;
 }
 function findResult(results, todo) {

[tylerbecks]

😭 I'm running into this too after the beautiful release from this package to support the --quiet flag today.

[bartocc]

I've edited my previous comment to confirm that I also see exit code 1 with the --quiet flag

[steveszc]

I went ahead and created a separate issue for the --quiet mode output issue: #415

[steveszc]

Also created a separate issue for the --quiet mode non-zero exist code, which is different that the issue causing non-zero exits in eslint@8.25+
#417

[wagenet]

@scalvert looks like this is something that will need to be fixed in this package, probably with a direct exit :/

[scalvert]

Yep. Totally cool with pull requests to help here!