Skip to content

Commit 24b02c7

Browse files
committed
Switch from strip-json-comments to jsonc-parser for better handling of JSONC (e.g., trailing commas) (fixes #267).
1 parent 551e6ba commit 24b02c7

14 files changed

+145
-43
lines changed

markdownlint-cli2.js

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,18 @@ const utf8 = "utf8";
3535
// No-op function
3636
const noop = () => null;
3737

38-
// Gets a synchronous function to parse JSONC text
39-
const getJsoncParse = async () => {
40-
const { "default": stripJsonComments } =
41-
// eslint-disable-next-line no-inline-comments
42-
await import(/* webpackMode: "eager" */ "strip-json-comments");
43-
return (text) => JSON.parse(stripJsonComments(text));
38+
// Synchronous function to parse JSONC text
39+
const jsoncParse = (text) => {
40+
const { parse, printParseErrorCode } = require("jsonc-parser");
41+
const errors = [];
42+
const result = parse(text, errors, { "allowTrailingComma": true });
43+
if (errors.length > 0) {
44+
const aggregate = errors.map(
45+
(err) => `${printParseErrorCode(err.error)} (offset ${err.offset}, length ${err.length})`
46+
).join(", ");
47+
throw new Error(`Unable to parse JSON(C) content, ${aggregate}`);
48+
}
49+
return result;
4450
};
4551

4652
// Synchronous function to parse YAML text
@@ -67,12 +73,10 @@ const readConfig = (fs, dir, name, otherwise) => {
6773
const file = pathPosix.join(dir, name);
6874
return () => fs.promises.access(file).
6975
then(
70-
() => getJsoncParse().then(
71-
(jsoncParse) => markdownlintReadConfig(
72-
file,
73-
[ jsoncParse, yamlParse ],
74-
fs
75-
)
76+
() => markdownlintReadConfig(
77+
file,
78+
[ jsoncParse, yamlParse ],
79+
fs
7680
),
7781
otherwise
7882
);
@@ -139,9 +143,8 @@ const importOrRequireConfig = (fs, dir, name, noRequire, otherwise) => {
139143
};
140144

141145
// Extend a config object if it has 'extends' property
142-
const getExtendedConfig = async (config, configPath, fs) => {
146+
const getExtendedConfig = (config, configPath, fs) => {
143147
if (config.extends) {
144-
const jsoncParse = await getJsoncParse();
145148
return markdownlintExtendConfig(
146149
config,
147150
configPath,
@@ -150,7 +153,7 @@ const getExtendedConfig = async (config, configPath, fs) => {
150153
);
151154
}
152155

153-
return config;
156+
return Promise.resolve(config);
154157
};
155158

156159
// Read an options or config file in any format and return the object
@@ -160,7 +163,6 @@ const readOptionsOrConfig = async (configPath, fs, noRequire) => {
160163
let options = null;
161164
let config = null;
162165
if (basename.endsWith(".markdownlint-cli2.jsonc")) {
163-
const jsoncParse = await getJsoncParse();
164166
options = jsoncParse(await fs.promises.readFile(configPath, utf8));
165167
} else if (basename.endsWith(".markdownlint-cli2.yaml")) {
166168
options = yamlParse(await fs.promises.readFile(configPath, utf8));
@@ -177,7 +179,6 @@ const readOptionsOrConfig = async (configPath, fs, noRequire) => {
177179
basename.endsWith(".markdownlint.yaml") ||
178180
basename.endsWith(".markdownlint.yml")
179181
) {
180-
const jsoncParse = await getJsoncParse();
181182
config =
182183
await markdownlintReadConfig(configPath, [ jsoncParse, yamlParse ], fs);
183184
} else if (
@@ -319,10 +320,7 @@ const getAndProcessDirInfo = (
319320
then(
320321
() => fs.promises.
321322
readFile(markdownlintCli2Jsonc, utf8).
322-
then(
323-
(content) => getJsoncParse().
324-
then((jsoncParse) => jsoncParse(content))
325-
),
323+
then(jsoncParse),
326324
() => fs.promises.access(markdownlintCli2Yaml).
327325
then(
328326
() => fs.promises.
@@ -346,11 +344,8 @@ const getAndProcessDirInfo = (
346344
then(
347345
() => fs.promises.
348346
readFile(packageJson, utf8).
349-
then(
350-
(content) => getJsoncParse().
351-
then((jsoncParse) => jsoncParse(content)).
352-
then((obj) => obj[packageName])
353-
),
347+
then(jsoncParse).
348+
then((obj) => obj[packageName]),
354349
noop
355350
)
356351
)
@@ -743,8 +738,7 @@ const createDirInfos = async (
743738
};
744739

745740
// Lint files in groups by shared configuration
746-
const lintFiles = async (fs, dirInfos, fileContents) => {
747-
const jsoncParse = await getJsoncParse();
741+
const lintFiles = (fs, dirInfos, fileContents) => {
748742
const tasks = [];
749743
// For each dirInfo
750744
for (const dirInfo of dirInfos) {

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,10 +63,10 @@
6363
],
6464
"dependencies": {
6565
"globby": "14.0.0",
66+
"jsonc-parser": "3.2.0",
6667
"markdownlint": "0.33.0",
6768
"markdownlint-cli2-formatter-default": "0.0.4",
6869
"micromatch": "4.0.5",
69-
"strip-json-comments": "5.0.1",
7070
"yaml": "2.3.4"
7171
},
7272
"devDependencies": {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
// Comment
3+
"config": {
4+
"MD032": false,
5+
"no-multiple-blanks": false,
6+
}
7+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"first-line-heading": false,
3+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
## Information
2+
Text ` code1` text `code2 ` text
3+
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# Title
2+
3+
> Tagline
4+
5+
6+
# Description
7+
8+
Text text text
9+
Text text text
10+
Text text text
11+
12+
## Summary
13+
14+
Text text text

test/markdownlint-cli2-test-cases.js

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ const testCases = ({
346346
"name": "markdownlint-json-invalid",
347347
"args": [ ".*" ],
348348
"exitCode": 2,
349-
"stderrRe": /(?:Unexpected end)|(?:Expected property name)/u
349+
"stderrRe": /Unable to parse JSON\(C\) content/u
350350
});
351351

352352
testCase({
@@ -388,7 +388,7 @@ const testCases = ({
388388
"name": "markdownlint-cli2-jsonc-mismatch",
389389
"args": [ "viewme.md" ],
390390
"exitCode": 2,
391-
"stderrRe": /Unexpected token/u
391+
"stderrRe": /Unable to parse JSON\(C\) content/u
392392
});
393393

394394
testCase({
@@ -415,7 +415,7 @@ const testCases = ({
415415
"name": "markdownlint-cli2-jsonc-mismatch-config",
416416
"args": [ "--config", "../markdownlint-cli2-jsonc-mismatch/.markdownlint-cli2.jsonc", "viewme.md" ],
417417
"exitCode": 2,
418-
"stderrRe": /Unexpected token/u,
418+
"stderrRe": /Unable to parse JSON\(C\) content/u,
419419
"cwd": "no-config",
420420
});
421421

@@ -446,7 +446,7 @@ const testCases = ({
446446
"name": "markdownlint-cli2-jsonc-invalid",
447447
"args": [ ".*" ],
448448
"exitCode": 2,
449-
"stderrRe": /(?:Unexpected end)|(?:Expected property name)/u
449+
"stderrRe": /Unable to parse JSON\(C\) content/u
450450
});
451451

452452
testCase({
@@ -665,8 +665,7 @@ const testCases = ({
665665
});
666666
}
667667

668-
const unexpectedJsonRe =
669-
/(?:Unexpected end of JSON input)|(?:Expected property name)/u;
668+
const unexpectedJsonRe = /Unable to parse JSON\(C\) content/u;
670669
const unableToRequireRe = /Unable to require or import module/u;
671670
const unableToParseRe = /Unable to parse/u;
672671
const invalidConfigFiles = [
@@ -775,7 +774,7 @@ const testCases = ({
775774
"name": "package-json-invalid",
776775
"args": [ "**/*.md" ],
777776
"exitCode": 2,
778-
"stderrRe": /(?:Unexpected end)|(?:Expected property name)/u
777+
"stderrRe": /Unable to parse JSON\(C\) content/u
779778
});
780779

781780
testCase({
@@ -1134,6 +1133,12 @@ const testCases = ({
11341133
"usesRequire": true
11351134
});
11361135

1136+
testCase({
1137+
"name": "jsonc-trailing-comma",
1138+
"args": [ "**/*.md" ],
1139+
"exitCode": 1
1140+
});
1141+
11371142
};
11381143

11391144
module.exports = testCases;

test/markdownlint-cli2-test.js

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ const fs = require("node:fs/promises");
66
const path = require("node:path");
77
const Ajv = require("ajv");
88
const test = require("ava").default;
9+
const jsoncParser = require("jsonc-parser");
910
const { "main": markdownlintCli2 } = require("../markdownlint-cli2.js");
1011
const FsMock = require("./fs-mock");
1112

@@ -63,7 +64,7 @@ test("README files", (t) => {
6364
});
6465

6566
test("validateMarkdownlintConfigSchema", async (t) => {
66-
t.plan(25);
67+
t.plan(26);
6768

6869
// Validate schema
6970
// @ts-ignore
@@ -81,8 +82,6 @@ test("validateMarkdownlintConfigSchema", async (t) => {
8182
);
8283

8384
// Validate instances
84-
// @ts-ignore
85-
const { "default": stripJsonComments } = await import("strip-json-comments");
8685
const { globby } = await import("globby");
8786
const files = await globby(
8887
[
@@ -99,7 +98,7 @@ test("validateMarkdownlintConfigSchema", async (t) => {
9998
);
10099
return Promise.all(files.map(async (file) => {
101100
const content = await fs.readFile(file, "utf8");
102-
const json = JSON.parse(stripJsonComments(content));
101+
const json = jsoncParser.parse(content);
103102
const instanceResult = validateConfigSchema(json);
104103
t.truthy(
105104
instanceResult,
@@ -109,7 +108,7 @@ test("validateMarkdownlintConfigSchema", async (t) => {
109108
});
110109

111110
test("validateMarkdownlintCli2ConfigSchema", async (t) => {
112-
t.plan(87);
111+
t.plan(88);
113112

114113
// Validate schema
115114
// @ts-ignore
@@ -129,7 +128,6 @@ test("validateMarkdownlintCli2ConfigSchema", async (t) => {
129128
);
130129

131130
// Validate instances
132-
const { "default": stripJsonComments } = await import("strip-json-comments");
133131
const { globby } = await import("globby");
134132
const files = await globby(
135133
[
@@ -147,7 +145,7 @@ test("validateMarkdownlintCli2ConfigSchema", async (t) => {
147145
);
148146
return Promise.all(files.map(async (file) => {
149147
const content = await fs.readFile(file, "utf8");
150-
const json = JSON.parse(stripJsonComments(content));
148+
const json = jsoncParser.parse(content);
151149
const instanceResult = validateConfigSchema(json);
152150
t.truthy(
153151
instanceResult,

test/snapshots/markdownlint-cli2-test-exec.js.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6350,3 +6350,29 @@ Generated by [AVA](https://avajs.dev).
63506350
Summary: 14 error(s)␊
63516351
`,
63526352
}
6353+
6354+
## jsonc-trailing-comma (exec)
6355+
6356+
> Snapshot 1
6357+
6358+
{
6359+
exitCode: 1,
6360+
formatterCodeQuality: '',
6361+
formatterJson: '',
6362+
formatterJunit: '',
6363+
formatterSarif: '',
6364+
stderr: `dir/info.md:1 MD022/blanks-around-headings Headings should be surrounded by blank lines [Expected: 1; Actual: 0; Below] [Context: "## Information"]␊
6365+
dir/info.md:2:6 MD038/no-space-in-code Spaces inside code span elements [Context: "\` code1\`"]␊
6366+
dir/info.md:2:20 MD038/no-space-in-code Spaces inside code span elements [Context: "\`code2 \`"]␊
6367+
dir/info.md:4 MD012/no-multiple-blanks Multiple consecutive blank lines [Expected: 1; Actual: 2]␊
6368+
viewme.md:3:10 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]␊
6369+
viewme.md:6 MD025/single-title/single-h1 Multiple top-level headings in the same document [Context: "# Description"]␊
6370+
viewme.md:12:1 MD019/no-multiple-space-atx Multiple spaces after hash on atx style heading [Context: "## Summary"]␊
6371+
viewme.md:14:14 MD047/single-trailing-newline Files should end with a single newline character␊
6372+
`,
6373+
stdout: `markdownlint-cli2 vX.Y.Z (markdownlint vX.Y.Z)␊
6374+
Finding: **/*.md␊
6375+
Linting: 2 file(s)␊
6376+
Summary: 8 error(s)␊
6377+
`,
6378+
}
41 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)