Skip to content

Commit b645d42

Browse files
octogonzoctogonz
authored
Merge pull request #289 from microsoft/octogonz/tsdoc-config-error-fixes
Improve error reporting for tsdoc.json issues
2 parents fcfc27e + 1459dfd commit b645d42

File tree

15 files changed

+173
-3
lines changed

15 files changed

+173
-3
lines changed

‎.prettierignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,6 @@ dist-prod
104104

105105
# We'll consider enabling this later; Prettier reformats code blocks, which affects end-user content
106106
*.md
107+
108+
# Ignore .json files which may contain intentional syntax errors
109+
**/__tests__/assets/**/*.json

‎common/changes/@microsoft/tsdoc-config/octogonz-tsdoc-config-error-fixes_2021-04-19-20-29.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/tsdoc-config",
5+
"comment": "Improve reporting of error messages when loading tsdoc.json",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/tsdoc-config",
10+
"email": "4673363+octogonz@users.noreply.github.com"
11+
}

‎common/changes/@microsoft/tsdoc/octogonz-tsdoc-config-error-fixes_2021-04-19-20-29.json

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@microsoft/tsdoc",
5+
"comment": "Add a new TSDocMessageId definition used by @microsoft/tsdoc-config",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@microsoft/tsdoc",
10+
"email": "4673363+octogonz@users.noreply.github.com"
11+
}

‎tsdoc-config/src/TSDocConfigFile.ts

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ export class TSDocConfigFile {
180180
const tagDefinition: TSDocTagDefinition = new TSDocTagDefinition(parameters);
181181

182182
if (this._tagDefinitionNames.has(tagDefinition.tagNameWithUpperCase)) {
183-
throw new Error(`A tag defintion was already added with the tag name "${parameters.tagName}"`);
183+
throw new Error(`A tag definition was already added with the tag name "${parameters.tagName}"`);
184184
}
185185
this._tagDefinitionNames.add(tagDefinition.tagName);
186186

@@ -209,7 +209,7 @@ export class TSDocConfigFile {
209209
textRange: TextRange.empty,
210210
});
211211
}
212-
this._tagDefinitionNames.add(tagDefinition.tagName);
212+
this._tagDefinitionNames.add(tagDefinition.tagNameWithUpperCase);
213213

214214
this._tagDefinitions.push(tagDefinition);
215215
}
@@ -381,7 +381,19 @@ export class TSDocConfigFile {
381381
const configJsonContent: string = fs.readFileSync(this._filePath).toString();
382382
this._fileMTime = fs.statSync(this._filePath).mtimeMs;
383383

384-
const configJson: IConfigJson = jju.parse(configJsonContent, { mode: 'cjson' });
384+
let configJson: IConfigJson;
385+
try {
386+
configJson = jju.parse(configJsonContent, { mode: 'cjson' });
387+
} catch (e) {
388+
this.log.addMessage(
389+
new ParserMessage({
390+
messageId: TSDocMessageId.ConfigInvalidJson,
391+
messageText: 'Error parsing JSON input: ' + e.message,
392+
textRange: TextRange.empty,
393+
})
394+
);
395+
return;
396+
}
385397

386398
this._loadJsonObject(configJson);
387399

@@ -446,6 +458,12 @@ export class TSDocConfigFile {
446458
/**
447459
* Calls `TSDocConfigFile.findConfigPathForFolder()` to find the relevant tsdoc.json config file, if one exists.
448460
* Then calls `TSDocConfigFile.findConfigPathForFolder()` to return the loaded result.
461+
*
462+
* @remarks
463+
* This API does not report loading errors by throwing exceptions. Instead, the caller is expected to check
464+
* for errors using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log},
465+
* or {@link TSDocConfigFile.getErrorSummary}.
466+
*
449467
* @param folderPath - the path to a folder where the search should start
450468
*/
451469
public static loadForFolder(folderPath: string): TSDocConfigFile {
@@ -455,6 +473,12 @@ export class TSDocConfigFile {
455473

456474
/**
457475
* Loads the specified tsdoc.json and any base files that it refers to using the "extends" option.
476+
*
477+
* @remarks
478+
* This API does not report loading errors by throwing exceptions. Instead, the caller is expected to check
479+
* for errors using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log},
480+
* or {@link TSDocConfigFile.getErrorSummary}.
481+
*
458482
* @param tsdocJsonFilePath - the path to the tsdoc.json config file
459483
*/
460484
public static loadFile(tsdocJsonFilePath: string): TSDocConfigFile {
@@ -469,6 +493,10 @@ export class TSDocConfigFile {
469493
*
470494
* @remarks
471495
* The serialized object has the same structure as `tsdoc.json`; however the `"extends"` field is not allowed.
496+
*
497+
* This API does not report loading errors by throwing exceptions. Instead, the caller is expected to check
498+
* for errors using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log},
499+
* or {@link TSDocConfigFile.getErrorSummary}.
472500
*/
473501
public static loadFromObject(jsonObject: unknown): TSDocConfigFile {
474502
const configFile: TSDocConfigFile = new TSDocConfigFile();
@@ -484,6 +512,11 @@ export class TSDocConfigFile {
484512

485513
/**
486514
* Initializes a TSDocConfigFile object using the state from the provided `TSDocConfiguration` object.
515+
*
516+
* @remarks
517+
* This API does not report loading errors by throwing exceptions. Instead, the caller is expected to check
518+
* for errors using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log},
519+
* or {@link TSDocConfigFile.getErrorSummary}.
487520
*/
488521
public static loadFromParser(configuration: TSDocConfiguration): TSDocConfigFile {
489522
const configFile: TSDocConfigFile = new TSDocConfigFile();
@@ -607,6 +640,10 @@ export class TSDocConfigFile {
607640
/**
608641
* Applies the settings from this config file to a TSDoc parser configuration.
609642
* Any `extendsFile` settings will also applied.
643+
*
644+
* @remarks
645+
* Additional validation is performed during this operation. The caller is expected to check for errors
646+
* using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log}, or {@link TSDocConfigFile.getErrorSummary}.
610647
*/
611648
public configureParser(configuration: TSDocConfiguration): void {
612649
if (this._getNoStandardTagsWithExtends()) {
@@ -622,6 +659,10 @@ export class TSDocConfigFile {
622659

623660
/**
624661
* This is the same as {@link configureParser}, but it preserves any previous state.
662+
*
663+
* @remarks
664+
* Additional validation is performed during this operation. The caller is expected to check for errors
665+
* using {@link TSDocConfigFile.hasErrors}, {@link TSDocConfigFile.log}, or {@link TSDocConfigFile.getErrorSummary}.
625666
*/
626667
public updateParser(configuration: TSDocConfiguration): void {
627668
// First apply the base config files

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

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import * as path from 'path';
2+
3+
import { TSDocConfigFile } from '../TSDocConfigFile';
4+
5+
function testLoadingFolder(assetPath: string): unknown {
6+
const configFile: TSDocConfigFile = TSDocConfigFile.loadForFolder(path.join(__dirname, assetPath));
7+
8+
return {
9+
fileNotFound: configFile.fileNotFound,
10+
hasErrors: configFile.hasErrors,
11+
log: configFile.log.messages.map((x) => `[${x.messageId}] ${x.text}`),
12+
};
13+
}
14+
15+
test('Load e1', () => {
16+
expect(testLoadingFolder('assets/e1')).toMatchInlineSnapshot(`
17+
Object {
18+
"fileNotFound": false,
19+
"hasErrors": true,
20+
"log": Array [
21+
"[tsdoc-config-schema-error] Error loading config file: data should NOT have additional properties",
22+
],
23+
}
24+
`);
25+
});
26+
27+
test('Load e2', () => {
28+
expect(testLoadingFolder('assets/e2')).toMatchInlineSnapshot(`
29+
Object {
30+
"fileNotFound": true,
31+
"hasErrors": false,
32+
"log": Array [
33+
"[tsdoc-config-invalid-json] Error parsing JSON input: Unexpected token '\\\\n' at 3:12
34+
\\"invalid
35+
^",
36+
],
37+
}
38+
`);
39+
});
40+
41+
test('Load e3', () => {
42+
expect(testLoadingFolder('assets/e3')).toMatchInlineSnapshot(`
43+
Object {
44+
"fileNotFound": false,
45+
"hasErrors": true,
46+
"log": Array [
47+
"[tsdoc-config-unsupported-schema] Unsupported JSON \\"$schema\\" value; expecting \\"https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json\\"",
48+
],
49+
}
50+
`);
51+
});
52+
53+
test('Load e4', () => {
54+
expect(testLoadingFolder('assets/e4')).toMatchInlineSnapshot(`
55+
Object {
56+
"fileNotFound": false,
57+
"hasErrors": true,
58+
"log": Array [
59+
"[tsdoc-config-duplicate-tag-name] The \\"tagDefinitions\\" field specifies more than one tag with the name \\"@dupe\\"",
60+
],
61+
}
62+
`);
63+
});

‎tsdoc-config/src/__tests__/assets/e1/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎tsdoc-config/src/__tests__/assets/e1/tsdoc.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
3+
"invalidSetting": false
4+
}

‎tsdoc-config/src/__tests__/assets/e2/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎tsdoc-config/src/__tests__/assets/e2/tsdoc.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
3+
"invalid

‎tsdoc-config/src/__tests__/assets/e3/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

0 commit comments

Comments
 (0)