Mention file names in script parse failures (#4397)

Fixes #4381
palantir · Jan 10, 2019 · ba5ffe8 · ba5ffe8
1 parent 65b3e63
commit ba5ffe8
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 13 deletions.
diff --git a/src/test.ts b/src/test.ts
@@ -205,8 +205,16 @@ export function runTest(testDirectory: string, rulesDirectory?: string | string[
             errorsFromMarkup,
             fixesFromLinter: newFileText,
             fixesFromMarkup: fixedFileText,
-            markupFromLinter: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromMarkup),
-            markupFromMarkup: parse.createMarkupFromErrors(fileTextWithoutMarkup, errorsFromLinter),
+            markupFromLinter: parse.createMarkupFromErrors(
+                fileToLint,
+                fileTextWithoutMarkup,
+                errorsFromMarkup,
+            ),
+            markupFromMarkup: parse.createMarkupFromErrors(
+                fileToLint,
+                fileTextWithoutMarkup,
+                errorsFromLinter,
+            ),
             skipped: false,
         };
     }

diff --git a/src/verify/lines.ts b/src/verify/lines.ts
@@ -87,14 +87,17 @@ export function parseLine(text: string): Line {
  * Maps a Line object to a matching line of text that could be in a .lint file.
  * This is almost the inverse of parseLine.
  * If you ran `printLine(parseLine(someText), code)`, the whitespace in the result may be different than in someText
+ * @param fileName - File name containing the line and code.
  * @param line - A Line object to convert to text
  * @param code - If line represents error markup, this is the line of code preceding the markup.
  *               Otherwise, this parameter is not required.
  */
-export function printLine(line: Line, code?: string): string | undefined {
+export function printLine(fileName: string, line: Line, code?: string): string | undefined {
     if (line instanceof ErrorLine) {
         if (code === undefined) {
-            throw new Error("Must supply argument for code parameter when line is an ErrorLine");
+            throw new Error(
+                `${fileName}: Must supply argument for code parameter when line is an ErrorLine`,
+            );
         }
 
         const leadingSpaces = " ".repeat(line.startCol);
@@ -111,7 +114,7 @@ export function printLine(line: Line, code?: string): string | undefined {
             let tildes = "~".repeat(line.endCol - line.startCol);
             if (code.length < line.endCol) {
                 // Better than crashing in String.repeat
-                throw new Error(`Bad error marker at ${JSON.stringify(line)}`);
+                throw new Error(`Bad error marker in ${fileName} at ${JSON.stringify(line)}`);
             }
             let endSpaces = " ".repeat(code.length - line.endCol);
             if (tildes.length === 0) {

diff --git a/src/verify/parse.ts b/src/verify/parse.ts
@@ -255,7 +255,7 @@ function parseFormatArguments(text: string): string[] | undefined {
     return result.length === 0 ? undefined : result;
 }
 
-export function createMarkupFromErrors(code: string, lintErrors: LintError[]) {
+export function createMarkupFromErrors(fileName: string, code: string, lintErrors: LintError[]) {
     lintErrors.sort(errorComparator);
 
     const codeText = code.split("\n");
@@ -281,7 +281,7 @@ export function createMarkupFromErrors(code: string, lintErrors: LintError[]) {
 
     return flatMap(codeText, (line, i) => [
         line,
-        ...mapDefined(errorLinesForCodeText[i], err => printLine(err, line)),
+        ...mapDefined(errorLinesForCodeText[i], err => printLine(fileName, err, line)),
     ]).join("\n");
 }
 /* tslint:enable:object-literal-sort-keys */

diff --git a/test/rule-tester/linesTests.ts b/test/rule-tester/linesTests.ts
@@ -24,24 +24,43 @@ describe("Rule Test Lines", () => {
             const code1 = "this is a line of code";
             const errorLine1 = new lines.MultilineErrorLine(2);
             const errorMarkup1 = "  ~~~~~~~~~~~~~~~~~~~~";
-            assert.strictEqual(lines.printLine(errorLine1, code1), errorMarkup1);
+            assert.strictEqual(lines.printLine("fileName.ts", errorLine1, code1), errorMarkup1);
 
             const code2 = "another line of code here";
             const errorLine2 = new lines.EndErrorLine(0, code2.length, "foo");
             const errorMarkup2 = "~~~~~~~~~~~~~~~~~~~~~~~~~ [foo]";
-            assert.strictEqual(lines.printLine(errorLine2, code2), errorMarkup2);
+            assert.strictEqual(lines.printLine("fileName.ts", errorLine2, code2), errorMarkup2);
         });
 
         it("should correctly create strings with empty lines of code", () => {
             const code1 = "";
             const errorLine1 = new lines.MultilineErrorLine(0);
             const errorMarkup1 = lines.ZERO_LENGTH_ERROR;
-            assert.strictEqual(lines.printLine(errorLine1, code1), errorMarkup1);
+            assert.strictEqual(lines.printLine("fileName.ts", errorLine1, code1), errorMarkup1);
 
             const code2 = "";
             const errorLine2 = new lines.EndErrorLine(0, 0, "foo");
             const errorMarkup2 = `${lines.ZERO_LENGTH_ERROR} [foo]`;
-            assert.strictEqual(lines.printLine(errorLine2, code2), errorMarkup2);
+            assert.strictEqual(lines.printLine("fileName.ts", errorLine2, code2), errorMarkup2);
+        });
+
+        it("should correctly throw an error when code is not supplied", () => {
+            const errorLine = new lines.EndErrorLine(0, 0, "foo");
+            assert.throws(
+                () => lines.printLine("fileName.ts", errorLine),
+                Error,
+                "fileName.ts: Must supply argument for code parameter when line is an ErrorLine",
+            );
+        });
+
+        it("should correctly throw an when the error marker is off", () => {
+            const code = "";
+            const errorLine = new lines.EndErrorLine(0, 2, "foo");
+            assert.throws(
+                () => lines.printLine("fileName.ts", errorLine, code),
+                Error,
+                `Bad error marker in fileName.ts at {"startCol":0,"endCol":2,"message":"foo"}`,
+            );
         });
     });
 });
diff --git a/test/rule-tester/parseTests.ts b/test/rule-tester/parseTests.ts
@@ -67,14 +67,22 @@ describe("Rule Test Parse", () => {
     describe("createMarkupFromErrors", () => {
         it("should generate correct markup", () => {
             assert.strictEqual(
-                parse.createMarkupFromErrors(testData.codeStr5, testData.resultErrs5),
+                parse.createMarkupFromErrors(
+                    "fileName.ts",
+                    testData.codeStr5,
+                    testData.resultErrs5,
+                ),
                 testData.lintStr5,
             );
         });
 
         it("should generate correct markup with nil-length errors", () => {
             assert.strictEqual(
-                parse.createMarkupFromErrors(testData.codeStr7, testData.resultErrs7),
+                parse.createMarkupFromErrors(
+                    "fileName.ts",
+                    testData.codeStr7,
+                    testData.resultErrs7,
+                ),
                 testData.lintStr7,
             );
         });