Fix some issues with the logic for TSDocConfigFile.hasErrors and TSDocConfigFile.fileNotFound

Author: octogonz

tsdoc-config/src/TSDocConfigFile.ts

@@ -75,7 +75,7 @@ export class TSDocConfigFile {
 
     this._extendsFiles = [];
     this._filePath = '';
-    this._fileNotFound = true;
+    this._fileNotFound = false;
     this._hasErrors = false;
     this._fileMTime = 0;
     this._tsdocSchema = '';
@@ -102,8 +102,11 @@ export class TSDocConfigFile {
   }
 
   /**
-   * If true, then the TSDocConfigFile object contains an empty state, because the `tsdoc.json` file could
-   * not be found by the loader.
+   * If true, then the TSDocConfigFile object contains an empty state, because the `tsdoc.json` file
+   * was not found by the loader.
+   *
+   * @remarks
+   * A missing "tsdoc.json" file is not considered an error.  It simply means that the defaults will be used.
    */
   public get fileNotFound(): boolean {
     return this._fileNotFound;
@@ -278,8 +281,6 @@ export class TSDocConfigFile {
   }
 
   private _loadJsonObject(configJson: IConfigJson): void {
-    this._fileNotFound = false;
-
     if (configJson.$schema !== TSDocConfigFile.CURRENT_SCHEMA_URL) {
       this._reportError({
         messageId: TSDocMessageId.ConfigFileUnsupportedSchema,
@@ -347,6 +348,10 @@ export class TSDocConfigFile {
     referencingConfigFile: TSDocConfigFile | undefined,
     alreadyVisitedPaths: Set<string>
   ): void {
+    // In case an exception is thrown, start by assuming that the file was not found; we'll revise
+    // this later upon success
+    this._fileNotFound = true;
+
     if (!configFilePath) {
       this._reportError({
         messageId: TSDocMessageId.ConfigFileNotFound,
@@ -367,6 +372,11 @@ export class TSDocConfigFile {
       return;
     }
 
+    const configJsonContent: string = fs.readFileSync(this._filePath).toString();
+    this._fileMTime = fs.statSync(this._filePath).mtimeMs;
+
+    this._fileNotFound = false;
+
     const hashKey: string = fs.realpathSync(this._filePath);
     if (referencingConfigFile && alreadyVisitedPaths.has(hashKey)) {
       this._reportError({
@@ -378,20 +388,15 @@ export class TSDocConfigFile {
     }
     alreadyVisitedPaths.add(hashKey);
 
-    const configJsonContent: string = fs.readFileSync(this._filePath).toString();
-    this._fileMTime = fs.statSync(this._filePath).mtimeMs;
-
     let configJson: IConfigJson;
     try {
       configJson = jju.parse(configJsonContent, { mode: 'cjson' });
     } catch (e) {
-      this.log.addMessage(
-        new ParserMessage({
-          messageId: TSDocMessageId.ConfigInvalidJson,
-          messageText: 'Error parsing JSON input: ' + e.message,
-          textRange: TextRange.empty,
-        })
-      );
+      this._reportError({
+        messageId: TSDocMessageId.ConfigInvalidJson,
+        messageText: 'Error parsing JSON input: ' + e.message,
+        textRange: TextRange.empty,
+      });
       return;
     }
 
@@ -616,20 +621,26 @@ export class TSDocConfigFile {
       return 'No errors.';
     }
 
-    let result: string;
+    let result: string = '';
 
-    if (this.filePath) {
-      result = `Errors encountered for ${this.filePath}:\n`;
-    } else {
-      result = `Errors encountered when loading TSDoc configuration:\n`;
-    }
+    if (this.log.messages.length > 0) {
+      const errorNoun: string = this.log.messages.length > 1 ? 'Errors' : 'Error';
+      if (this.filePath) {
+        result += `${errorNoun} encountered for ${this.filePath}:\n`;
+      } else {
+        result += `${errorNoun} encountered when loading TSDoc configuration:\n`;
+      }
 
-    for (const message of this.log.messages) {
-      result += `  ${message.text}\n`;
+      for (const message of this.log.messages) {
+        result += `  ${message.text}\n`;
+      }
     }
 
     for (const extendsFile of this.extendsFiles) {
       if (extendsFile.hasErrors) {
+        if (result !== '') {
+          result += '\n';
+        }
         result += extendsFile.getErrorSummary();
       }
     }
@@ -681,6 +692,8 @@ export class TSDocConfigFile {
         // Note that setSupportForTag() automatically enables configuration.validation.reportUnsupportedTags
         configuration.setSupportForTag(tagDefinition, supported);
       } else {
+        // Note that this validation may depend partially on the preexisting state of the TSDocConfiguration
+        // object, so it cannot be performed during the TSConfigFile.loadFile() stage.
         this._reportError({
           messageId: TSDocMessageId.ConfigFileUndefinedTag,
           messageText: `The "supportForTags" field refers to an undefined tag ${JSON.stringify(tagName)}.`,

tsdoc-config/src/__tests__/ErrorHandling.test.ts

@@ -2,62 +2,199 @@ import * as path from 'path';
 
 import { TSDocConfigFile } from '../TSDocConfigFile';
 
-function testLoadingFolder(assetPath: string): unknown {
-  const configFile: TSDocConfigFile = TSDocConfigFile.loadForFolder(path.join(__dirname, assetPath));
+interface ISnapshot {
+  _0_filePath: string;
+  _1_fileNotFound: boolean;
+  _2_hasErrors: boolean;
+  _3_errorSummary: string;
+  _4_log: string[];
+  _5_extends: ISnapshot[];
+}
+
+// To make the unit tests deterministic, we need to replace all OS-dependent absolute paths
+// with OS-independent paths that are relative to the unit test folder.
+function makeStablePath(testPath: string): string {
+  if (testPath.length === 0) {
+    return '';
+  }
+  return '.../' + path.relative(__dirname, testPath).split('\\').join('/');
+}
+
+// Build a map from absolute path --> stable path, for each TSDocConfigFile.filePath value
+function buildStablePathMap(stablePathMap: Map<string, string>, configFile: TSDocConfigFile): void {
+  if (!stablePathMap.has(configFile.filePath)) {
+    stablePathMap.set(configFile.filePath, makeStablePath(configFile.filePath));
+  }
+  for (const extendsFile of configFile.extendsFiles) {
+    buildStablePathMap(stablePathMap, extendsFile);
+  }
+}
 
+// Search and replace all absolute paths with the corresponding stable path.
+// For example, "Found C:\A\B\C.txt in C:\A\D\E.txt" becomes "Found .../B/C.txt in .../D/E.txt".
+function convertToStablePaths(text: string, stablePathMap: Map<string, string>): string {
+  for (const pair of Array.from(stablePathMap.entries())) {
+    text = text.split(pair[0]).join(pair[1]);
+  }
+  return text;
+}
+
+function createSnapshot(configFile: TSDocConfigFile, pathFixupMap: Map<string, string>): ISnapshot {
   return {
-    fileNotFound: configFile.fileNotFound,
-    hasErrors: configFile.hasErrors,
-    log: configFile.log.messages.map((x) => `[${x.messageId}] ${x.text}`),
+    _0_filePath: convertToStablePaths(configFile.filePath, pathFixupMap),
+    _1_fileNotFound: configFile.fileNotFound,
+    _2_hasErrors: configFile.hasErrors,
+    _4_log: configFile.log.messages.map((x) => `[${x.messageId}] ${convertToStablePaths(x.text, pathFixupMap)}`),
+    _5_extends: configFile.extendsFiles.map((x) => createSnapshot(x, pathFixupMap)),
+    _3_errorSummary: convertToStablePaths(configFile.getErrorSummary(), pathFixupMap),
   };
 }
 
+function testLoadingFolder(assetPath: string): ISnapshot {
+  const configFile: TSDocConfigFile = TSDocConfigFile.loadForFolder(path.join(__dirname, assetPath));
+
+  const pathFixupMap: Map<string, string> = new Map();
+  buildStablePathMap(pathFixupMap, configFile);
+
+  return createSnapshot(configFile, pathFixupMap);
+}
+
 test('Load e1', () => {
   expect(testLoadingFolder('assets/e1')).toMatchInlineSnapshot(`
     Object {
-      "fileNotFound": false,
-      "hasErrors": true,
-      "log": Array [
+      "_0_filePath": ".../assets/e1/tsdoc.json",
+      "_1_fileNotFound": false,
+      "_2_hasErrors": true,
+      "_3_errorSummary": "Error encountered for .../assets/e1/tsdoc.json:
+      Error loading config file: data should NOT have additional properties
+    ",
+      "_4_log": Array [
         "[tsdoc-config-schema-error] Error loading config file: data should NOT have additional properties",
       ],
+      "_5_extends": Array [],
     }
   `);
 });
 
 test('Load e2', () => {
   expect(testLoadingFolder('assets/e2')).toMatchInlineSnapshot(`
     Object {
-      "fileNotFound": true,
-      "hasErrors": false,
-      "log": Array [
+      "_0_filePath": ".../assets/e2/tsdoc.json",
+      "_1_fileNotFound": false,
+      "_2_hasErrors": true,
+      "_3_errorSummary": "Error encountered for .../assets/e2/tsdoc.json:
+      Error parsing JSON input: Unexpected token '\\\\n' at 3:12
+      \\"invalid
+              ^
+    ",
+      "_4_log": Array [
         "[tsdoc-config-invalid-json] Error parsing JSON input: Unexpected token '\\\\n' at 3:12
       \\"invalid
               ^",
       ],
+      "_5_extends": Array [],
     }
   `);
 });
 
 test('Load e3', () => {
   expect(testLoadingFolder('assets/e3')).toMatchInlineSnapshot(`
     Object {
-      "fileNotFound": false,
-      "hasErrors": true,
-      "log": Array [
+      "_0_filePath": ".../assets/e3/tsdoc.json",
+      "_1_fileNotFound": false,
+      "_2_hasErrors": true,
+      "_3_errorSummary": "Error encountered for .../assets/e3/tsdoc.json:
+      Unsupported JSON \\"$schema\\" value; expecting \\"https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json\\"
+    ",
+      "_4_log": Array [
         "[tsdoc-config-unsupported-schema] Unsupported JSON \\"$schema\\" value; expecting \\"https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json\\"",
       ],
+      "_5_extends": Array [],
     }
   `);
 });
 
 test('Load e4', () => {
   expect(testLoadingFolder('assets/e4')).toMatchInlineSnapshot(`
     Object {
-      "fileNotFound": false,
-      "hasErrors": true,
-      "log": Array [
+      "_0_filePath": ".../assets/e4/tsdoc.json",
+      "_1_fileNotFound": false,
+      "_2_hasErrors": true,
+      "_3_errorSummary": "Error encountered for .../assets/e4/tsdoc.json:
+      The \\"tagDefinitions\\" field specifies more than one tag with the name \\"@dupe\\"
+    ",
+      "_4_log": Array [
         "[tsdoc-config-duplicate-tag-name] The \\"tagDefinitions\\" field specifies more than one tag with the name \\"@dupe\\"",
       ],
+      "_5_extends": Array [],
+    }
+  `);
+});
+
+test('Load e5', () => {
+  expect(testLoadingFolder('assets/e5')).toMatchInlineSnapshot(`
+    Object {
+      "_0_filePath": ".../assets/e5/tsdoc.json",
+      "_1_fileNotFound": false,
+      "_2_hasErrors": true,
+      "_3_errorSummary": "Error encountered for .../assets/e5/tsdoc-a.json:
+      Circular reference encountered for \\"extends\\" field of \\".../assets/e5/tsdoc-b.json\\"
+
+    Error encountered for .../assets/e5/tsdoc-c.json:
+      Error loading config file: data should NOT have additional properties
+    ",
+      "_4_log": Array [],
+      "_5_extends": Array [
+        Object {
+          "_0_filePath": ".../assets/e5/tsdoc-a.json",
+          "_1_fileNotFound": false,
+          "_2_hasErrors": true,
+          "_3_errorSummary": "Error encountered for .../assets/e5/tsdoc-a.json:
+      Circular reference encountered for \\"extends\\" field of \\".../assets/e5/tsdoc-b.json\\"
+
+    Error encountered for .../assets/e5/tsdoc-c.json:
+      Error loading config file: data should NOT have additional properties
+    ",
+          "_4_log": Array [],
+          "_5_extends": Array [
+            Object {
+              "_0_filePath": ".../assets/e5/tsdoc-b.json",
+              "_1_fileNotFound": false,
+              "_2_hasErrors": true,
+              "_3_errorSummary": "Error encountered for .../assets/e5/tsdoc-a.json:
+      Circular reference encountered for \\"extends\\" field of \\".../assets/e5/tsdoc-b.json\\"
+    ",
+              "_4_log": Array [],
+              "_5_extends": Array [
+                Object {
+                  "_0_filePath": ".../assets/e5/tsdoc-a.json",
+                  "_1_fileNotFound": false,
+                  "_2_hasErrors": true,
+                  "_3_errorSummary": "Error encountered for .../assets/e5/tsdoc-a.json:
+      Circular reference encountered for \\"extends\\" field of \\".../assets/e5/tsdoc-b.json\\"
+    ",
+                  "_4_log": Array [
+                    "[tsdoc-config-cyclic-extends] Circular reference encountered for \\"extends\\" field of \\".../assets/e5/tsdoc-b.json\\"",
+                  ],
+                  "_5_extends": Array [],
+                },
+              ],
+            },
+            Object {
+              "_0_filePath": ".../assets/e5/tsdoc-c.json",
+              "_1_fileNotFound": false,
+              "_2_hasErrors": true,
+              "_3_errorSummary": "Error encountered for .../assets/e5/tsdoc-c.json:
+      Error loading config file: data should NOT have additional properties
+    ",
+              "_4_log": Array [
+                "[tsdoc-config-schema-error] Error loading config file: data should NOT have additional properties",
+              ],
+              "_5_extends": Array [],
+            },
+          ],
+        },
+      ],
     }
   `);
 });

tsdoc-config/src/__tests__/assets/e5/package.json

@@ -0,0 +1 @@
+{}

tsdoc-config/src/__tests__/assets/e5/tsdoc-a.json

@@ -0,0 +1,4 @@
+{
+  "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
+  "extends": [ "./tsdoc-b.json", "./tsdoc-c.json" ]
+}

tsdoc-config/src/__tests__/assets/e5/tsdoc-b.json

@@ -0,0 +1,4 @@
+{
+  "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
+  "extends": [ "./tsdoc-a.json" ]
+}

tsdoc-config/src/__tests__/assets/e5/tsdoc-c.json

@@ -0,0 +1,4 @@
+{
+  "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
+  "unknownSetting": 123
+}

tsdoc-config/src/__tests__/assets/e5/tsdoc.json

@@ -0,0 +1,5 @@
+{
+  "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
+  "extends": [ "./tsdoc-a.json" ]
+
+}