From b60d88fa80bdd09029610584603734419ef89ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Arod?= Date: Thu, 29 Oct 2015 12:56:13 +0100 Subject: [PATCH 1/5] Allow comments in tsconfig.json issue #4987 --- Jakefile.js | 3 +- src/compiler/commandLineParser.ts | 94 +++++++++++++++++++++++- tests/cases/unittests/tsconfigParsing.ts | 84 +++++++++++++++++++++ 3 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 tests/cases/unittests/tsconfigParsing.ts diff --git a/Jakefile.js b/Jakefile.js index a602a2c459f02..db6b5f5567105 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -145,7 +145,8 @@ var harnessSources = harnessCoreSources.concat([ "transpile.ts", "reuseProgramStructure.ts", "cachingInServerLSHost.ts", - "moduleResolution.ts" + "moduleResolution.ts", + "tsconfigParsing.ts" ].map(function (f) { return path.join(unittestsDirectory, f); })).concat([ diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 84496fdbeb313..e5950db05b66d 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -405,13 +405,105 @@ namespace ts { */ export function parseConfigFileTextToJson(fileName: string, jsonText: string): { config?: any; error?: Diagnostic } { try { - return { config: /\S/.test(jsonText) ? JSON.parse(jsonText) : {} }; + let jsonTextWithoutComments = removeComments(jsonText); + return { config: /\S/.test(jsonTextWithoutComments) ? JSON.parse(jsonTextWithoutComments) : {} }; } catch (e) { return { error: createCompilerDiagnostic(Diagnostics.Failed_to_parse_file_0_Colon_1, fileName, e.message) }; } } + + /** + * Remove the comments from a json like text. + * Comments can be single line comments (starting with # or //) or multiline comments using / * * / + * + * This method replace comment content by whitespace rather than completely remove them to keep positions in json parsing error reporting accurate. + */ + function removeComments(jsonText: string): string { + let result = ""; + let processingString = false; + let processingSingleLineComment = false; + let processingMultiLineComment = false; + for (let i = 0; i < jsonText.length; i++) { + let currentChar = jsonText.charAt(i); + let nextChar = (i + 1 < jsonText.length) ? jsonText.charAt(i + 1) : undefined; + if (processingString) { + if (currentChar === "\\" + && nextChar === "\"") { + // Escaped quote consume the 2 characters + result += currentChar; + result += nextChar; + i += 1; + } + else if (currentChar === "\"") { + // End of string + result += currentChar; + processingString = false; + } + else { + // String content + result += currentChar; + } + } + else if (processingSingleLineComment) { + if (currentChar === "\n") { + // End of single line comment + processingSingleLineComment = false; + // Keep the line breaks to keep line numbers aligned + result += currentChar; + } + else { + // replace comment content by whitespaces + result += " "; + } + } + else if (processingMultiLineComment) { + if (currentChar === "*" && nextChar === "/") { + // End of comment + result += " "; + i += 1; + processingMultiLineComment = false; + } + else if (currentChar === "\n") { + // Keep the line breaks to Keep line aligned + result += currentChar; + } + else { + // replace comment content by whitespaces + result += " "; + } + } + else if (currentChar === "\"") { + // String start + result += currentChar; + processingString = true; + } + else if (currentChar === "#") { + // Start of # comment + result += " "; + processingSingleLineComment = true; + } + else if (currentChar === "/" && nextChar === "/") { + // Start of // comment + result += " "; + i += 1; + processingSingleLineComment = true; + } + else if (currentChar === "/" && nextChar === "*") { + // Start of /**/ comment + result += " "; + i += 1; + processingMultiLineComment = true; + } + else { + // Keep other characters + result += currentChar; + } + } + return result; + } + /** * Parse the contents of a config file (tsconfig.json). * @param json The contents of the config file to parse diff --git a/tests/cases/unittests/tsconfigParsing.ts b/tests/cases/unittests/tsconfigParsing.ts new file mode 100644 index 0000000000000..3da1acb83c3c0 --- /dev/null +++ b/tests/cases/unittests/tsconfigParsing.ts @@ -0,0 +1,84 @@ +/// +/// + +module ts { + describe('parseConfigFileTextToJson', () => { + function assertParseResult(jsonText: string, expectedConfigObject: { config?: any; error?: Diagnostic }) { + let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); + assert.equal(JSON.stringify(parsed), JSON.stringify(expectedConfigObject)); + } + + function assertParseError(jsonText: string) { + let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); + assert.isTrue(undefined === parsed.config); + assert.isTrue(undefined !== parsed.error); + } + + it("returns empty config for file with only whitespaces", () => { + assertParseResult("", { config : {} }); + assertParseResult(" ", { config : {} }); + }); + + it("returns empty config for file with comments only", () => { + assertParseResult("// Comment", { config: {} }); + assertParseResult("# Comment", { config: {} }); + assertParseResult("/* Comment*/", { config: {} }); + }); + + it("returns empty config when config is empty object", () => { + assertParseResult("{}", { config: {} }); + }); + + it("returns config object without comments", () => { + assertParseResult( + `{ // Excluded files + "exclude": [ + // Exclude d.ts + "file.d.ts" + ] + }`, { config: { exclude: ["file.d.ts"] } }); + assertParseResult( + `{ + # Excluded files + "exclude": [ + # Exclude d.ts + "file.d.ts" + ] + }`, { config: { exclude: ["file.d.ts"] } }); + assertParseResult( + `{ + /* Excluded + Files + */ + "exclude": [ + /* multiline comments can be in the middle of a line */"file.d.ts" + ] + }`, { config: { exclude: ["file.d.ts"] } }); + }); + + it("keeps string content untouched", () => { + assertParseResult( + `{ + "exclude": [ + "xx//file.d.ts" + ] + }`, { config: { exclude: ["xx//file.d.ts"] } }); + assertParseResult( + `{ + "exclude": [ + "xx#file.d.ts" + ] + }`, { config: { exclude: ["xx#file.d.ts"] } }); + assertParseResult( + `{ + "exclude": [ + "xx/*file.d.ts*/" + ] + }`, { config: { exclude: ["xx/*file.d.ts*/"] } }); + }); + + it("returns object with error when json is invalid", () => { + assertParseError("invalid"); + }); + }); +} From f5e73ab8bf2d460c4f302f4d3aa286f9e0177f04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Arod?= Date: Thu, 29 Oct 2015 14:55:23 +0100 Subject: [PATCH 2/5] Fix handling of escaped characters in string --- src/compiler/commandLineParser.ts | 5 +++-- tests/cases/unittests/tsconfigParsing.ts | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index e5950db05b66d..d69b91bd5b4cf 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -430,8 +430,9 @@ namespace ts { let nextChar = (i + 1 < jsonText.length) ? jsonText.charAt(i + 1) : undefined; if (processingString) { if (currentChar === "\\" - && nextChar === "\"") { - // Escaped quote consume the 2 characters + && nextChar !== undefined) { + // Found an escaped character + // consume the \ and the escaped char result += currentChar; result += nextChar; i += 1; diff --git a/tests/cases/unittests/tsconfigParsing.ts b/tests/cases/unittests/tsconfigParsing.ts index 3da1acb83c3c0..f48593b316988 100644 --- a/tests/cases/unittests/tsconfigParsing.ts +++ b/tests/cases/unittests/tsconfigParsing.ts @@ -77,6 +77,22 @@ module ts { }`, { config: { exclude: ["xx/*file.d.ts*/"] } }); }); + it("handles escaped characters in strings correctly", () => { + assertParseResult( + `{ + "exclude": [ + "xx\\"//files" + ] + }`, { config: { exclude: ["xx\"//files"] } }); + + assertParseResult( + `{ + "exclude": [ + "xx\\\\" // end of line comment + ] + }`, { config: { exclude: ["xx\\"] } }); + }); + it("returns object with error when json is invalid", () => { assertParseError("invalid"); }); From 5f81ba12aad46c2549d6121d7c8800b43c9f381a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Arod?= Date: Thu, 29 Oct 2015 23:02:32 +0100 Subject: [PATCH 3/5] Use CharactersCodes and isLineBreak in conditions --- src/compiler/commandLineParser.ts | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index d69b91bd5b4cf..885b3af45f804 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -426,18 +426,19 @@ namespace ts { let processingSingleLineComment = false; let processingMultiLineComment = false; for (let i = 0; i < jsonText.length; i++) { + let currentCharCode = jsonText.charCodeAt(i); let currentChar = jsonText.charAt(i); - let nextChar = (i + 1 < jsonText.length) ? jsonText.charAt(i + 1) : undefined; + let nextCharCode = (i + 1 < jsonText.length) ? jsonText.charCodeAt(i + 1) : undefined; if (processingString) { - if (currentChar === "\\" - && nextChar !== undefined) { + if (currentCharCode === CharacterCodes.backslash + && nextCharCode !== undefined) { // Found an escaped character // consume the \ and the escaped char result += currentChar; - result += nextChar; + result += jsonText.charAt(i + 1); i += 1; } - else if (currentChar === "\"") { + else if (currentCharCode === CharacterCodes.doubleQuote) { // End of string result += currentChar; processingString = false; @@ -448,7 +449,7 @@ namespace ts { } } else if (processingSingleLineComment) { - if (currentChar === "\n") { + if (isLineBreak(currentCharCode)) { // End of single line comment processingSingleLineComment = false; // Keep the line breaks to keep line numbers aligned @@ -460,13 +461,13 @@ namespace ts { } } else if (processingMultiLineComment) { - if (currentChar === "*" && nextChar === "/") { - // End of comment + if (currentCharCode === CharacterCodes.asterisk && nextCharCode === CharacterCodes.slash) { + // End of comment */ result += " "; i += 1; processingMultiLineComment = false; } - else if (currentChar === "\n") { + else if (isLineBreak(currentCharCode)) { // Keep the line breaks to Keep line aligned result += currentChar; } @@ -475,23 +476,23 @@ namespace ts { result += " "; } } - else if (currentChar === "\"") { + else if (currentCharCode === CharacterCodes.doubleQuote) { // String start result += currentChar; processingString = true; } - else if (currentChar === "#") { + else if (currentCharCode === CharacterCodes.hash) { // Start of # comment result += " "; processingSingleLineComment = true; } - else if (currentChar === "/" && nextChar === "/") { + else if (currentCharCode === CharacterCodes.slash && nextCharCode === CharacterCodes.slash) { // Start of // comment result += " "; i += 1; processingSingleLineComment = true; } - else if (currentChar === "/" && nextChar === "*") { + else if (currentCharCode === CharacterCodes.slash && nextCharCode === CharacterCodes.asterisk) { // Start of /**/ comment result += " "; i += 1; From 00b389d47765f71bbba9ddd9ebd37e06c29d2009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Arod?= Date: Sat, 31 Oct 2015 23:16:04 +0100 Subject: [PATCH 4/5] New commit using TS scanner. This commit uses TS scanner and replaces comments token text by whitespaces to preserve orginal positions. --- src/compiler/commandLineParser.ts | 112 +++++++---------------- tests/cases/unittests/tsconfigParsing.ts | 24 +---- 2 files changed, 40 insertions(+), 96 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 885b3af45f804..840e2a7e584c3 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -421,91 +421,49 @@ namespace ts { * This method replace comment content by whitespace rather than completely remove them to keep positions in json parsing error reporting accurate. */ function removeComments(jsonText: string): string { - let result = ""; - let processingString = false; - let processingSingleLineComment = false; - let processingMultiLineComment = false; - for (let i = 0; i < jsonText.length; i++) { - let currentCharCode = jsonText.charCodeAt(i); - let currentChar = jsonText.charAt(i); - let nextCharCode = (i + 1 < jsonText.length) ? jsonText.charCodeAt(i + 1) : undefined; - if (processingString) { - if (currentCharCode === CharacterCodes.backslash - && nextCharCode !== undefined) { - // Found an escaped character - // consume the \ and the escaped char - result += currentChar; - result += jsonText.charAt(i + 1); - i += 1; - } - else if (currentCharCode === CharacterCodes.doubleQuote) { - // End of string - result += currentChar; - processingString = false; - } - else { - // String content - result += currentChar; - } - } - else if (processingSingleLineComment) { - if (isLineBreak(currentCharCode)) { - // End of single line comment - processingSingleLineComment = false; - // Keep the line breaks to keep line numbers aligned - result += currentChar; - } - else { - // replace comment content by whitespaces - result += " "; - } + let output = ""; + let scanner = createScanner(ScriptTarget.ES5, /* skipTrivia */ false, LanguageVariant.Standard, jsonText); + let token: SyntaxKind; + while ((token = scanner.scan()) !== SyntaxKind.EndOfFileToken) { + switch (token) { + case SyntaxKind.SingleLineCommentTrivia: + case SyntaxKind.MultiLineCommentTrivia: + // replace comments with whitespaces to preserve original characters position + output += replaceWithWhitespaces(scanner.getTokenText()); + break; + default: + output += scanner.getTokenText(); + break; } - else if (processingMultiLineComment) { - if (currentCharCode === CharacterCodes.asterisk && nextCharCode === CharacterCodes.slash) { - // End of comment */ - result += " "; - i += 1; - processingMultiLineComment = false; - } - else if (isLineBreak(currentCharCode)) { - // Keep the line breaks to Keep line aligned - result += currentChar; + } + return output; + + function replaceWithWhitespaces(commentTokenText: string): string { + let result = ""; + let pos = 0; + let start = 0; + while (pos < commentTokenText.length) { + if (isLineBreak(commentTokenText.charCodeAt(pos))) { + let nbCharToReplace = pos - start; + result += nSpaces(nbCharToReplace); + result += commentTokenText.charAt(pos); + pos += 1; + start = pos; } else { - // replace comment content by whitespaces - result += " "; + pos += 1; } } - else if (currentCharCode === CharacterCodes.doubleQuote) { - // String start - result += currentChar; - processingString = true; - } - else if (currentCharCode === CharacterCodes.hash) { - // Start of # comment - result += " "; - processingSingleLineComment = true; - } - else if (currentCharCode === CharacterCodes.slash && nextCharCode === CharacterCodes.slash) { - // Start of // comment - result += " "; - i += 1; - processingSingleLineComment = true; - } - else if (currentCharCode === CharacterCodes.slash && nextCharCode === CharacterCodes.asterisk) { - // Start of /**/ comment - result += " "; - i += 1; - processingMultiLineComment = true; - } - else { - // Keep other characters - result += currentChar; - } + result += nSpaces(pos - start); + return result; + + function nSpaces(n: number): string { + return new Array(n + 1).join(" "); + }; } - return result; } + /** * Parse the contents of a config file (tsconfig.json). * @param json The contents of the config file to parse diff --git a/tests/cases/unittests/tsconfigParsing.ts b/tests/cases/unittests/tsconfigParsing.ts index f48593b316988..7490b52500085 100644 --- a/tests/cases/unittests/tsconfigParsing.ts +++ b/tests/cases/unittests/tsconfigParsing.ts @@ -21,7 +21,6 @@ module ts { it("returns empty config for file with comments only", () => { assertParseResult("// Comment", { config: {} }); - assertParseResult("# Comment", { config: {} }); assertParseResult("/* Comment*/", { config: {} }); }); @@ -37,19 +36,12 @@ module ts { "file.d.ts" ] }`, { config: { exclude: ["file.d.ts"] } }); - assertParseResult( - `{ - # Excluded files - "exclude": [ - # Exclude d.ts - "file.d.ts" - ] - }`, { config: { exclude: ["file.d.ts"] } }); + assertParseResult( `{ /* Excluded - Files - */ + Files + */ "exclude": [ /* multiline comments can be in the middle of a line */"file.d.ts" ] @@ -60,16 +52,10 @@ module ts { assertParseResult( `{ "exclude": [ - "xx//file.d.ts" + "xx//file.d.ts" ] }`, { config: { exclude: ["xx//file.d.ts"] } }); - assertParseResult( - `{ - "exclude": [ - "xx#file.d.ts" - ] - }`, { config: { exclude: ["xx#file.d.ts"] } }); - assertParseResult( + assertParseResult( `{ "exclude": [ "xx/*file.d.ts*/" From 638e4b758a7f46b4bebae5da3bd89cd5ca3a67e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Arod?= Date: Sun, 1 Nov 2015 15:31:16 +0100 Subject: [PATCH 5/5] Use regex for repacing comments content. Use space for indents --- src/compiler/commandLineParser.ts | 28 +---- tests/cases/unittests/tsconfigParsing.ts | 154 +++++++++++------------ 2 files changed, 79 insertions(+), 103 deletions(-) diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 840e2a7e584c3..aaf3bf624b691 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -428,8 +428,8 @@ namespace ts { switch (token) { case SyntaxKind.SingleLineCommentTrivia: case SyntaxKind.MultiLineCommentTrivia: - // replace comments with whitespaces to preserve original characters position - output += replaceWithWhitespaces(scanner.getTokenText()); + // replace comments with whitespace to preserve original character positions + output += scanner.getTokenText().replace(/\S/g, " "); break; default: output += scanner.getTokenText(); @@ -437,30 +437,6 @@ namespace ts { } } return output; - - function replaceWithWhitespaces(commentTokenText: string): string { - let result = ""; - let pos = 0; - let start = 0; - while (pos < commentTokenText.length) { - if (isLineBreak(commentTokenText.charCodeAt(pos))) { - let nbCharToReplace = pos - start; - result += nSpaces(nbCharToReplace); - result += commentTokenText.charAt(pos); - pos += 1; - start = pos; - } - else { - pos += 1; - } - } - result += nSpaces(pos - start); - return result; - - function nSpaces(n: number): string { - return new Array(n + 1).join(" "); - }; - } } diff --git a/tests/cases/unittests/tsconfigParsing.ts b/tests/cases/unittests/tsconfigParsing.ts index 7490b52500085..3603d22f31457 100644 --- a/tests/cases/unittests/tsconfigParsing.ts +++ b/tests/cases/unittests/tsconfigParsing.ts @@ -1,86 +1,86 @@ /// /// -module ts { - describe('parseConfigFileTextToJson', () => { - function assertParseResult(jsonText: string, expectedConfigObject: { config?: any; error?: Diagnostic }) { - let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); - assert.equal(JSON.stringify(parsed), JSON.stringify(expectedConfigObject)); - } - - function assertParseError(jsonText: string) { - let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); - assert.isTrue(undefined === parsed.config); - assert.isTrue(undefined !== parsed.error); - } +namespace ts { + describe('parseConfigFileTextToJson', () => { + function assertParseResult(jsonText: string, expectedConfigObject: { config?: any; error?: Diagnostic }) { + let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); + assert.equal(JSON.stringify(parsed), JSON.stringify(expectedConfigObject)); + } - it("returns empty config for file with only whitespaces", () => { - assertParseResult("", { config : {} }); - assertParseResult(" ", { config : {} }); - }); - - it("returns empty config for file with comments only", () => { - assertParseResult("// Comment", { config: {} }); - assertParseResult("/* Comment*/", { config: {} }); - }); + function assertParseError(jsonText: string) { + let parsed = ts.parseConfigFileTextToJson("/apath/tsconfig.json", jsonText); + assert.isTrue(undefined === parsed.config); + assert.isTrue(undefined !== parsed.error); + } - it("returns empty config when config is empty object", () => { - assertParseResult("{}", { config: {} }); - }); + it("returns empty config for file with only whitespaces", () => { + assertParseResult("", { config : {} }); + assertParseResult(" ", { config : {} }); + }); - it("returns config object without comments", () => { - assertParseResult( - `{ // Excluded files - "exclude": [ - // Exclude d.ts - "file.d.ts" - ] - }`, { config: { exclude: ["file.d.ts"] } }); - - assertParseResult( - `{ - /* Excluded - Files - */ - "exclude": [ - /* multiline comments can be in the middle of a line */"file.d.ts" - ] - }`, { config: { exclude: ["file.d.ts"] } }); - }); + it("returns empty config for file with comments only", () => { + assertParseResult("// Comment", { config: {} }); + assertParseResult("/* Comment*/", { config: {} }); + }); - it("keeps string content untouched", () => { - assertParseResult( - `{ - "exclude": [ - "xx//file.d.ts" - ] - }`, { config: { exclude: ["xx//file.d.ts"] } }); - assertParseResult( - `{ - "exclude": [ - "xx/*file.d.ts*/" - ] - }`, { config: { exclude: ["xx/*file.d.ts*/"] } }); - }); - - it("handles escaped characters in strings correctly", () => { - assertParseResult( - `{ - "exclude": [ - "xx\\"//files" - ] - }`, { config: { exclude: ["xx\"//files"] } }); - - assertParseResult( - `{ - "exclude": [ - "xx\\\\" // end of line comment - ] - }`, { config: { exclude: ["xx\\"] } }); - }); - - it("returns object with error when json is invalid", () => { - assertParseError("invalid"); + it("returns empty config when config is empty object", () => { + assertParseResult("{}", { config: {} }); + }); + + it("returns config object without comments", () => { + assertParseResult( + `{ // Excluded files + "exclude": [ + // Exclude d.ts + "file.d.ts" + ] + }`, { config: { exclude: ["file.d.ts"] } }); + + assertParseResult( + `{ + /* Excluded + Files + */ + "exclude": [ + /* multiline comments can be in the middle of a line */"file.d.ts" + ] + }`, { config: { exclude: ["file.d.ts"] } }); + }); + + it("keeps string content untouched", () => { + assertParseResult( + `{ + "exclude": [ + "xx//file.d.ts" + ] + }`, { config: { exclude: ["xx//file.d.ts"] } }); + assertParseResult( + `{ + "exclude": [ + "xx/*file.d.ts*/" + ] + }`, { config: { exclude: ["xx/*file.d.ts*/"] } }); + }); + + it("handles escaped characters in strings correctly", () => { + assertParseResult( + `{ + "exclude": [ + "xx\\"//files" + ] + }`, { config: { exclude: ["xx\"//files"] } }); + + assertParseResult( + `{ + "exclude": [ + "xx\\\\" // end of line comment + ] + }`, { config: { exclude: ["xx\\"] } }); + }); + + it("returns object with error when json is invalid", () => { + assertParseError("invalid"); + }); }); - }); }