Fix: formatter printing path of files with todo (#9)

Fixed a bug where the formatter was printing the path of todo files when
there were other errors in other files. The check if all messages are
todos should be done for each result message array and not all results
in total.

__tests__/__fixtures__/eslint-with-errors-warnings-todos.json

@@ -0,0 +1,153 @@
+{
+  "results": [
+    {
+      "filePath": "{{path}}/app/errors-only.js",
+      "messages": [
+        {
+          "ruleId": "no-prototype-builtins",
+          "severity": 2,
+          "message": "Do not access Object.prototype method 'hasOwnProperty' from target object.",
+          "line": 25,
+          "column": 21,
+          "nodeType": "CallExpression",
+          "messageId": "prototypeBuildIn",
+          "endLine": 25,
+          "endColumn": 35
+        },
+        {
+          "ruleId": "no-prototype-builtins",
+          "severity": 2,
+          "message": "Do not access Object.prototype method 'hasOwnProperty' from target object.",
+          "line": 26,
+          "column": 19,
+          "nodeType": "CallExpression",
+          "messageId": "prototypeBuildIn",
+          "endLine": 26,
+          "endColumn": 33
+        },
+        {
+          "ruleId": "no-prototype-builtins",
+          "severity": 2,
+          "message": "Do not access Object.prototype method 'hasOwnProperty' from target object.",
+          "line": 32,
+          "column": 34,
+          "nodeType": "CallExpression",
+          "messageId": "prototypeBuildIn",
+          "endLine": 32,
+          "endColumn": 48
+        }
+      ],
+      "errorCount": 3,
+      "warningCount": 0,
+      "fixableErrorCount": 0,
+      "fixableWarningCount": 0,
+      "source": ""
+    },
+    {
+      "filePath": "{{path}}/app/warnings-only.js",
+      "messages": [
+        {
+          "ruleId": "no-alert",
+          "severity": 1,
+          "message": "Unexpected alert.",
+          "line": 3,
+          "column": 3,
+          "nodeType": "CallExpression",
+          "messageId": "unexpected",
+          "endLine": 2,
+          "endColumn": 14
+        }
+      ],
+      "errorCount": 0,
+      "warningCount": 1,
+      "fixableErrorCount": 0,
+      "fixableWarningCount": 0,
+      "source": ""
+    },
+    {
+      "filePath": "{{path}}/app/todos-only.js",
+      "messages": [
+        {
+          "ruleId": "no-redeclare",
+          "severity": -1,
+          "message": "'window' is already defined as a built-in global variable.",
+          "line": 1,
+          "column": 11,
+          "nodeType": "Block",
+          "messageId": "redeclaredAsBuiltin",
+          "endLine": 1,
+          "endColumn": 17,
+          "fix": {
+            "range": [0, 1],
+            "text": ""
+          }
+        },
+        {
+          "ruleId": "no-redeclare",
+          "severity": -1,
+          "message": "'XMLHttpRequest' is already defined as a built-in global variable.",
+          "line": 1,
+          "column": 19,
+          "nodeType": "Block",
+          "messageId": "redeclaredAsBuiltin",
+          "endLine": 1,
+          "endColumn": 33
+        }
+      ],
+      "errorCount": 0,
+      "warningCount": 0,
+      "todoCount": 2,
+      "fixableErrorCount": 0,
+      "fixableWarningCount": 0,
+      "source": ""
+    },
+    {
+      "filePath": "{{path}}/app/errors-warnings-todo.js",
+      "messages": [
+        {
+          "ruleId": "no-redeclare",
+          "severity": 2,
+          "message": "'window' is already defined as a built-in global variable.",
+          "line": 1,
+          "column": 11,
+          "nodeType": "Block",
+          "messageId": "redeclaredAsBuiltin",
+          "endLine": 1,
+          "endColumn": 17,
+          "fix": {
+            "range": [0, 1],
+            "text": ""
+          }
+        },
+        {
+          "ruleId": "no-redeclare",
+          "severity": -1,
+          "message": "'XMLHttpRequest' is already defined as a built-in global variable.",
+          "line": 2,
+          "column": 19,
+          "nodeType": "Block",
+          "messageId": "redeclaredAsBuiltin",
+          "endLine": 1,
+          "endColumn": 33
+        },
+        {
+          "ruleId": "no-alert",
+          "severity": 1,
+          "message": "Unexpected alert.",
+          "line": 3,
+          "column": 3,
+          "nodeType": "CallExpression",
+          "messageId": "unexpected",
+          "endLine": 2,
+          "endColumn": 14
+        }
+      ],
+      "errorCount": 1,
+      "warningCount": 1,
+      "todoCount": 1,
+      "fixableErrorCount": 1,
+      "fixableWarningCount": 0,
+      "source": ""
+    }
+  ]
+}

__tests__/__fixtures__/fixtures.ts

@@ -1,6 +1,7 @@
 import type { CLIEngine, ESLint } from 'eslint';
 import { deepCopy } from '../__utils__/deep-copy';
 import { updatePaths } from '../__utils__/update-paths';
+import * as eslintWithErrorsWarningsTodos from './eslint-with-errors-warnings-todos.json';
 import * as eslintWithErrors from './eslint-with-errors.json';
 import * as eslintWithTodos from './eslint-with-todos.json';
 
@@ -11,15 +12,19 @@ const fixtures = {
   eslintWithTodos: <ESLint.LintResult[]>(
     (<CLIEngine.LintReport>(eslintWithTodos as unknown)).results
   ),
+  eslintWithErrorsWarningsTodos: <ESLint.LintResult[]>(
+    (<CLIEngine.LintReport>(eslintWithErrorsWarningsTodos as unknown)).results
+  ),
 };
 
 export default {
-  eslintWithErrors(tmp?: string): ESLint.LintResult[] {
-    const eslintWithErrors = deepCopy(fixtures.eslintWithErrors);
-    return tmp ? updatePaths(tmp, eslintWithErrors) : eslintWithErrors;
+  eslintWithErrors(tmp: string): ESLint.LintResult[] {
+    return updatePaths(tmp, deepCopy(fixtures.eslintWithErrors));
+  },
+  eslintWithTodos(tmp: string): ESLint.LintResult[] {
+    return updatePaths(tmp, deepCopy(fixtures.eslintWithTodos));
   },
-  eslintWithTodos(tmp?: string): ESLint.LintResult[] {
-    const eslintWithTodos = deepCopy(fixtures.eslintWithTodos);
-    return tmp ? updatePaths(tmp, eslintWithTodos) : eslintWithTodos;
+  eslintWithErrorsWarningsTodos(tmp: string): ESLint.LintResult[] {
+    return updatePaths(tmp, deepCopy(fixtures.eslintWithErrorsWarningsTodos));
   },
 };

__tests__/unit/formatter-test.ts

@@ -92,4 +92,28 @@ describe('formatter', () => {
       "
     `);
   });
+
+  it('should only return errors and warnings if includeTodo is false and there are errors, warnings, and todo items', async () => {
+    const results = fixtures.eslintWithErrorsWarningsTodos('/stable/path');
+
+    expect(stripAnsi(formatter(results))).toMatchInlineSnapshot(`
+      "
+      /stable/path/app/errors-only.js
+         25:21  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
+         26:19  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
+         32:34  error  Do not access Object.prototype method 'hasOwnProperty' from target object  no-prototype-builtins
+
+      /stable/path/app/warnings-only.js
+         3:3  warning  Unexpected alert  no-alert
+
+      /stable/path/app/errors-warnings-todo.js
+         1:11  error    'window' is already defined as a built-in global variable  no-redeclare
+         3:3   warning  Unexpected alert                                           no-alert
+
+      ✖ 6 problems (4 errors, 2 warnings)
+        1 error and warnings potentially fixable with the \`--fix\` option.
+
+      "
+    `);
+  });
 });

__tests__/unit/mutate-errors-to-todos-test.ts

@@ -1,5 +1,6 @@
 import { buildTodoData } from '@ember-template-lint/todo-utils';
 import { dirSync } from 'tmp';
+import { TODO_SEVERITY } from '../../src/constants';
 import { mutateTodoErrorsToTodos } from '../../src/mutate-errors-to-todos';
 import fixtures from '../__fixtures__/fixtures';
 
@@ -34,7 +35,7 @@ describe('mutate-errors-to-todos', () => {
       );
 
       result.messages.forEach((message) => {
-        expect(message.severity).toEqual(-1);
+        expect(message.severity).toEqual(TODO_SEVERITY);
       });
     });
   });

src/constants.ts

@@ -0,0 +1,2 @@
+export const TODO_SEVERITY = -1;
+export const ERROR_SEVERITY = 2;

src/formatter.ts

@@ -1,7 +1,8 @@
 import { blueBright, dim, red, reset, underline, yellow } from 'chalk';
+import type { ESLint } from 'eslint';
 import stripAnsi from 'strip-ansi';
 import table from 'text-table';
-import type { ESLint } from 'eslint';
+import { ERROR_SEVERITY, TODO_SEVERITY } from './constants';
 import type {
   TodoFormatterCounts,
   TodoFormatterOptions,
@@ -18,7 +19,7 @@ export function formatter(
 
   let output = '\n';
 
-  output += formatResults(results, counts, { includeTodo });
+  output += formatResults(results, { includeTodo });
 
   output += formatSummary(counts, { includeTodo });
 
@@ -30,7 +31,6 @@ export function formatter(
 
 function formatResults(
   results: ESLint.LintResult[],
-  counts: TodoFormatterCounts,
   options: TodoFormatterOptions
 ): string {
   let output = '';
@@ -42,16 +42,16 @@ function formatResults(
       return;
     }
 
-    const areAllMessagesTodo =
-      counts.errorCount === 0 &&
-      counts.warningCount === 0 &&
-      counts.todoCount > 0;
+    const areAllMessagesTodo = messages.every(
+      (message) => message.severity === TODO_SEVERITY
+    );
 
     if (options.includeTodo || !areAllMessagesTodo) {
       output += `${underline(result.filePath)}\n`;
     }
 
-    output += `${formatMessages(messages, options)}\n\n`;
+    output += `${formatMessages(messages, options)}`;
+    output += !options.includeTodo && areAllMessagesTodo ? '' : '\n\n';
   });
 
   return output;
@@ -62,13 +62,15 @@ function formatMessages(
   options: TodoFormatterOptions
 ): string {
   const messageRows = messages
-    .filter((message) => message.severity !== -1 || options.includeTodo)
+    .filter(
+      (message) => message.severity !== TODO_SEVERITY || options.includeTodo
+    )
     .map((message) => {
       let messageType;
 
-      if (message.fatal || message.severity === 2) {
+      if (message.fatal || message.severity === ERROR_SEVERITY) {
         messageType = red('error');
-      } else if (message.severity === -1) {
+      } else if (message.severity === TODO_SEVERITY) {
         messageType = blueBright('todo');
       } else {
         messageType = yellow('warning');

src/mutate-errors-to-todos.ts

@@ -4,6 +4,7 @@ import {
   _buildTodoDatum,
 } from '@ember-template-lint/todo-utils';
 import type { ESLint, Linter } from 'eslint';
+import { ERROR_SEVERITY, TODO_SEVERITY } from './constants';
 import type { TodoResultMessage } from './types';
 
 /**
@@ -17,20 +18,24 @@ export async function mutateTodoErrorsToTodos(
 ): Promise<void> {
   results.forEach((result) => {
     (result.messages as TodoResultMessage[]).forEach((message) => {
-      if (message.severity !== 2) {
+      if (message.severity !== ERROR_SEVERITY) {
         return;
       }
 
       // we only mutate errors that are present in the todo map, so check if it's there first
-      const todoDatum = _buildTodoDatum(baseDir, result, message as Linter.LintMessage);
+      const todoDatum = _buildTodoDatum(
+        baseDir,
+        result,
+        message as Linter.LintMessage
+      );
 
       const todoHash = todoFilePathFor(todoDatum);
 
       if (!todoMap.has(todoHash)) {
         return;
       }
 
-      message.severity = -1;
+      message.severity = TODO_SEVERITY;
 
       result.errorCount -= 1;
       result.todoCount = Number.isInteger(result.todoCount)