Repository: sverweij/dependency-cruiser
Pull request #908 (2024-02-03 18:07:42)

refactor(cli|report): moves responsibility to ensure output ends with an EOL to the individual reporters
## Description

- moves responsibility to ensure output ends with an EOL to the individual reporters
- adjusts most reporters to emit an EOL (except the null reporter, which _exists_ to emit nothing but null)
- adds a unit test that guarantees current & future reporters will still have an EOL in their output

## Motivation and Context

Fixes a personal OCD issue where the `null` reporter emitted an EOL disturbing adding an empty line to the diagnostics reporter (I see that thing a lot ...). 

Drawback: this solution is less simple/ elegant than the previous one (see diff).

## How Has This Been Tested?

- [x] green ci
- [x] additional non-regression tests

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Documentation only change
- [x] Refactor (non-breaking change which fixes an issue without changing functionality)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist

- [x] :book:

  - My change doesn't require a documentation update, or ...
  - it _does_ and I have updated it

- [x] :balance_scale:
  - The contribution will be subject to [The MIT license](https://github.com/sverweij/dependency-cruiser/blob/main/LICENSE), and I'm OK with that.
  - The contribution is my own original work.
  - I am ok with the stuff in [**CONTRIBUTING.md**](https://github.com/sverweij/dependency-cruiser/blob/main/.github/CONTRIBUTING.md).


codeclimate[bot]: Code Climate has analyzed commit bf500b7b and detected **0 issues** on this pull request.

View more on [Code Climate](https://codeclimate.com/github/sverweij/dependency-cruiser/pull/908).

codeclimate[bot]: Code Climate has analyzed commit cf36cb3a and detected **0 issues** on this pull request.

View more on [Code Climate](https://codeclimate.com/github/sverweij/dependency-cruiser/pull/908).


Patch:
diff --git a/src/cli/utl/io.mjs b/src/cli/utl/io.mjs
index 4875e5b5..2b037cf7 100644
--- a/src/cli/utl/io.mjs
+++ b/src/cli/utl/io.mjs
@@ -40,13 +40,15 @@ function writeToStdOut(pString, pBufferSize = PIPE_BUFFER_SIZE) {
   }
 }
 export function write(pOutputTo, pContent) {
-  const lContentWithTrailingNewline = pContent.endsWith("\n")
-    ? pContent
-    : `${pContent}\n`;
+  // previously we checked here to ensure pContent ended on an EOL (and if it
+  // didn't we added one). As for one or two reporters we DON'T want this
+  // to happen automatically (e.g. the 'null' one) we've moved that check
+  // to the individual reporters and with a unit test ensured that all
+  // reporters (current and future) keep doing that.
   if ("-" === pOutputTo) {
-    writeToStdOut(lContentWithTrailingNewline);
+    writeToStdOut(pContent);
   } else {
-    writeToFile(pOutputTo, lContentWithTrailingNewline);
+    writeToFile(pOutputTo, pContent);
   }
 }
 
diff --git a/src/report/anon/index.mjs b/src/report/anon/index.mjs
index 44e552a2..5ab845b2 100644
--- a/src/report/anon/index.mjs
+++ b/src/report/anon/index.mjs
@@ -1,6 +1,8 @@
 import has from "lodash/has.js";
 import { anonymizePath, WHITELIST_RE } from "./anonymize-path.mjs";
 
+const EOL = "\n";
+
 function anonymizePathArray(pPathArray, pWordList) {
   // the coverage ignore is here because the || [] branch isn't taken when running
   // tests and with the current setup of the anonymize module that's not going
@@ -182,11 +184,15 @@ export default function reportAnonymous(pResults, pAnonymousReporterOptions) {
       pResults?.summary?.optionsUsed?.reporterOptions?.anon?.wordlist ?? [];
   }
   return {
-    output: JSON.stringify(
-      anonymize(pResults, sanitizeWordList(lAnonymousReporterOptions.wordlist)),
-      null,
-      "  ",
-    ),
+    output:
+      JSON.stringify(
+        anonymize(
+          pResults,
+          sanitizeWordList(lAnonymousReporterOptions.wordlist),
+        ),
+        null,
+        "  ",
+      ) + EOL,
     exitCode: 0,
   };
 }
diff --git a/src/report/baseline.mjs b/src/report/baseline.mjs
index dfe0978f..ac7a171e 100644
--- a/src/report/baseline.mjs
+++ b/src/report/baseline.mjs
@@ -1,4 +1,5 @@
 const DEFAULT_JSON_INDENT = 2;
+const EOL = "\n";
 
 /**
  * Sample plugin
@@ -12,11 +13,12 @@ const DEFAULT_JSON_INDENT = 2;
  */
 export default function baseline(pCruiseResult) {
   return {
-    output: JSON.stringify(
-      pCruiseResult.summary.violations,
-      null,
-      DEFAULT_JSON_INDENT
-    ),
+    output:
+      JSON.stringify(
+        pCruiseResult.summary.violations,
+        null,
+        DEFAULT_JSON_INDENT,
+      ) + EOL,
     exitCode: 0,
   };
 }
diff --git a/src/report/dot-webpage/dot-module.mjs b/src/report/dot-webpage/dot-module.mjs
index 117cbcc1..9afebf86 100644
--- a/src/report/dot-webpage/dot-module.mjs
+++ b/src/report/dot-webpage/dot-module.mjs
@@ -1,7 +1,7 @@
 // @ts-check
 import { spawnSync } from "node:child_process";
 import dotModuleReporter from "../dot/dot-module.mjs";
-import { wrappapiccha } from "./wrap-in-html.mjs";
+import { wrapInHTML } from "./wrap-in-html.mjs";
 
 const CONSTANTS = {
   exec: "dot",
@@ -10,7 +10,7 @@ const CONSTANTS = {
 
 /**
  * @param {string} pDot Dot program
- * @param {IDotPictureReporterOptions} pOptions
+ * @param {IDotWebpageReporterOptions} pOptions
  * @returns {string} the dot program converted to svg, wrapped in html
  */
 function convert(pDot, pOptions) {
@@ -35,7 +35,7 @@ function convert(pDot, pOptions) {
 }
 
 /**
- * @param {IDotPictureReporterOptions} pOptions
+ * @param {IDotWebpageReporterOptions} pOptions
  * @returns {boolean}
  */
 function isAvailable(pOptions) {
@@ -51,7 +51,7 @@ function isAvailable(pOptions) {
  * @typedef {import("../../../types/cruise-result.mjs").ICruiseResult} ICruiseResult
  * @typedef {import("../../../types/reporter-options.mjs").IDotReporterOptions} IDotReporterOptions
  * @typedef {import("../../../types/dependency-cruiser.mjs").IReporterOutput} IReporterOutput
- * @typedef {IDotReporterOptions & { spawnFunction?: typeof spawnSync }} IDotPictureReporterOptions
+ * @typedef {IDotReporterOptions & { spawnFunction?: typeof spawnSync }} IDotWebpageReporterOptions
  */
 
 /**
@@ -59,19 +59,21 @@ function isAvailable(pOptions) {
  * and a system call to the GraphViz dot executable.
  *
  * @param {ICruiseResult} pResults
- * @param {IDotPictureReporterOptions} pPicchaReporterOptions
+ * @param {IDotWebpageReporterOptions} pDotWebpageReporterOptions
  * @returns {IReporterOutput}
  */
-export default function piccha(pResults, pPicchaReporterOptions) {
-  const { output } = dotModuleReporter(pResults, pPicchaReporterOptions);
+export default function dotWebpage(pResults, pDotWebpageReporterOptions) {
+  const { output } = dotModuleReporter(pResults, pDotWebpageReporterOptions);
 
-  if (!isAvailable(pPicchaReporterOptions)) {
+  if (!isAvailable(pDotWebpageReporterOptions)) {
     throw new Error(
-      "GraphViz dot, which is required for the 'x-dot-webpage' reporter doesn't seem to be available on this system. See the GraphViz download page for instruction on how to get it on your system: https://www.graphviz.org/download/",
+      "GraphViz dot, which is required for the 'x-dot-webpage' reporter doesn't " +
+        "seem to be available on this system. See the GraphViz download page for " +
+        "instruction on how to get it on your system: https://www.graphviz.org/download/",
     );
   }
   return {
-    output: wrappapiccha(convert(output, pPicchaReporterOptions)),
+    output: wrapInHTML(convert(output, pDotWebpageReporterOptions)),
     exitCode: 0,
   };
 }
diff --git a/src/report/dot-webpage/wrap-in-html.mjs b/src/report/dot-webpage/wrap-in-html.mjs
index f0ce8b0f..7edd8ccb 100755
--- a/src/report/dot-webpage/wrap-in-html.mjs
+++ b/src/report/dot-webpage/wrap-in-html.mjs
@@ -45,7 +45,8 @@ export function getFooter(pScript) {
       ${pScript}
     </script>
   </body>
-</html>`;
+</html>
+`;
 }
 
 /**
@@ -53,7 +54,7 @@ export function getFooter(pScript) {
  * @param {string} pSVG
  * @returns {string}
  */
-export function wrappapiccha(pSVG) {
+export function wrapInHTML(pSVG) {
   const lStylesheet = readFileSync(STYLESHEET_FILE, "utf8");
   const lScript = readFileSync(SCRIPT_FILE, "utf8");
 
diff --git a/src/report/error-html/error-html-template.mjs b/src/report/error-html/error-html-template.mjs
index a94ee0bc..babdce13 100644
--- a/src/report/error-html/error-html-template.mjs
+++ b/src/report/error-html/error-html-template.mjs
@@ -209,4 +209,5 @@ export default `<!DOCTYPE html>
       {{runDate}}</p>
   </footer>
 </body>
-</html>`;
+</html>
+`;
diff --git a/src/report/html/html-template.mjs b/src/report/html/html-template.mjs
index 98fb9fd9..264de3c8 100644
--- a/src/report/html/html-template.mjs
+++ b/src/report/html/html-template.mjs
@@ -181,4 +181,5 @@ export default `<!DOCTYPE html>
 <body>
   {{table-here}}
 </body>
-</html>`;
+</html>
+`;
diff --git a/src/report/json.mjs b/src/report/json.mjs
index 09e28c4c..ca77de69 100644
--- a/src/report/json.mjs
+++ b/src/report/json.mjs
@@ -1,3 +1,4 @@
+const EOL = "\n";
 /**
  * Returns the results of a cruise in JSON
  *
@@ -6,7 +7,7 @@
  */
 export default function json(pResults) {
   return {
-    output: JSON.stringify(pResults, null, "  "),
+    output: JSON.stringify(pResults, null, "  ") + EOL,
     exitCode: 0,
   };
 }
diff --git a/src/report/teamcity.mjs b/src/report/teamcity.mjs
index 5997c024..816b4b64 100644
--- a/src/report/teamcity.mjs
+++ b/src/report/teamcity.mjs
@@ -7,6 +7,7 @@ const SEVERITY2TEAMCITY_SEVERITY = new Map([
   ["warn", "WARNING"],
   ["info", "INFO"],
 ]);
+const EOL = "\n";
 
 function severity2teamcitySeverity(pSeverity) {
   return SEVERITY2TEAMCITY_SEVERITY.get(pSeverity) || "INFO";
@@ -159,9 +160,10 @@ export default function teamcity(pResults) {
   const lIgnoredCount = pResults?.summary?.ignore ?? 0;
 
   return {
-    output: reportViolatedRules(lRuleSet, lViolations, lIgnoredCount)
-      .concat(reportViolations(lViolations, lIgnoredCount))
-      .reduce((pAll, pCurrent) => `${pAll}${pCurrent}\n`, ""),
+    output:
+      reportViolatedRules(lRuleSet, lViolations, lIgnoredCount)
+        .concat(reportViolations(lViolations, lIgnoredCount))
+        .reduce((pAll, pCurrent) => `${pAll}${pCurrent}\n`, "") || EOL,
     exitCode: pResults.summary.error,
   };
 }
diff --git a/src/report/text.mjs b/src/report/text.mjs
index 1aec0104..5c200a50 100644
--- a/src/report/text.mjs
+++ b/src/report/text.mjs
@@ -1,10 +1,10 @@
-import { EOL } from "node:os";
 import figures from "figures";
 import chalk from "chalk";
 
 const DEFAULT_OPTIONS = {
   highlightFocused: false,
 };
+const EOL = "\n";
 
 /**
  *
@@ -30,9 +30,9 @@ function toFlatDependencies(pModules, pModulesInFocus, pHighlightFocused) {
   return pModules.reduce(
     (pAll, pModule) =>
       pAll.concat(
-        toFlatModuleDependencies(pModule, pModulesInFocus, pHighlightFocused)
+        toFlatModuleDependencies(pModule, pModulesInFocus, pHighlightFocused),
       ),
-    []
+    [],
   );
 }
 
@@ -55,7 +55,7 @@ function getModulesInFocus(pModules) {
   return new Set(
     pModules
       .filter((pModule) => pModule.matchesFocus || pModule.matchesReaches)
-      .map((pModule) => pModule.source)
+      .map((pModule) => pModule.source),
   );
 }
 
@@ -68,13 +68,15 @@ function getModulesInFocus(pModules) {
 function report(pResults, pOptions) {
   const lOptions = { ...DEFAULT_OPTIONS, ...pOptions };
 
-  return toFlatDependencies(
-    pResults.modules,
-    getModulesInFocus(pResults.modules),
-    lOptions.highlightFocused === true
-  ).reduce(
-    (pAll, pDependency) => pAll.concat(stringify(pDependency)).concat(EOL),
-    ""
+  return (
+    toFlatDependencies(
+      pResults.modules,
+      getModulesInFocus(pResults.modules),
+      lOptions.highlightFocused === true,
+    ).reduce(
+      (pAll, pDependency) => pAll.concat(stringify(pDependency)).concat(EOL),
+      "",
+    ) || EOL
   );
 }
 


Test patch:
diff --git a/test/report/dot-webpage/dot-module.spec.mjs b/test/report/dot-webpage/dot-module.spec.mjs
index 47c97559..90b39f88 100644
--- a/test/report/dot-webpage/dot-module.spec.mjs
+++ b/test/report/dot-webpage/dot-module.spec.mjs
@@ -1,5 +1,4 @@
 import { throws, ok, equal, match } from "node:assert/strict";
-import { spawnSync } from "node:child_process";
 import dotModule from "#report/dot-webpage/dot-module.mjs";
 
 const MINIMAL_CRUISE_RESULT = {
diff --git a/test/report/eol.spec.mjs b/test/report/eol.spec.mjs
new file mode 100644
index 00000000..2de30322
--- /dev/null
+++ b/test/report/eol.spec.mjs
@@ -0,0 +1,66 @@
+import { match, equal } from "node:assert/strict";
+import report from "#report/index.mjs";
+
+const MINIMAL_CRUISE_RESULT = {
+  modules: [],
+  summary: {
+    error: 0,
+    info: 0,
+    warn: 0,
+    ignore: 0,
+    totalCruised: 0,
+    violations: [],
+    optionsUsed: {},
+  },
+};
+
+const lAvailableReporters = report.getAvailableReporters();
+const lReporters = await Promise.all(
+  lAvailableReporters
+    .filter(
+      (pReporterName) =>
+        pReporterName !== "null" && pReporterName !== "x-dot-webpage",
+    )
+    .map(async (pReporter) => {
+      const lReporter = await report.getReporter(pReporter);
+      const lResult = lReporter(MINIMAL_CRUISE_RESULT);
+      return { reporter: pReporter, output: lResult.output };
+    }),
+);
+
+function spawnFunction(pString, pCommandsArray) {
+  if (pCommandsArray[0] === "-V") {
+    return {
+      stderr: "dot - graphviz version 1.2.3.4",
+      status: 0,
+    };
+  } else {
+    return {
+      stdout: "<svg></svg>",
+      status: 0,
+    };
+  }
+}
+
+describe("[I] most reporters' output ends on an EOL", () => {
+  lReporters.forEach(({ reporter, output }) => {
+    it(`the ${reporter} reporter output ends on an EOL`, () => {
+      match(output, /\n$/);
+    });
+  });
+
+  it("the x-dot-webpage reporter output ends on an EOL", async () => {
+    const lXDotWebpageReporter = await report.getReporter("x-dot-webpage");
+
+    const lResult = lXDotWebpageReporter(MINIMAL_CRUISE_RESULT, {
+      spawnFunction,
+    });
+    match(lResult.output, /\n$/);
+  });
+
+  it("the null reporter output DOES NOT end on an EOL", async () => {
+    const lNullReporter = await report.getReporter("null");
+    const lResult = lNullReporter(MINIMAL_CRUISE_RESULT);
+    equal(lResult.output, "");
+  });
+});
diff --git a/test/report/html/__mocks__/cjs-no-dependency-valid.html b/test/report/html/__mocks__/cjs-no-dependency-valid.html
index ffb5ccd6..f9fe7b0a 100644
--- a/test/report/html/__mocks__/cjs-no-dependency-valid.html
+++ b/test/report/html/__mocks__/cjs-no-dependency-valid.html
@@ -298,4 +298,4 @@ two_only_one.js -> sub/dir.js"></td><td class="cell cell-false"></td><td class="
   </table>
 
 </body>
-</html>
\ No newline at end of file
+</html>
diff --git a/test/report/teamcity/__mocks__/everything-fine-teamcity-format.txt b/test/report/teamcity/__mocks__/everything-fine-teamcity-format.txt
index e69de29b..8b137891 100644
--- a/test/report/teamcity/__mocks__/everything-fine-teamcity-format.txt
+++ b/test/report/teamcity/__mocks__/everything-fine-teamcity-format.txt
@@ -0,0 +1,1 @@
+