Skip to content

Commit 54f5f15

Browse files
committed
Update the project runner to emit errors in more readable way
1 parent 2264322 commit 54f5f15

File tree

31 files changed

+868
-88
lines changed

31 files changed

+868
-88
lines changed

src/harness/compilerRunner.ts

Lines changed: 1 addition & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -122,93 +122,7 @@ class CompilerBaselineRunner extends RunnerBase {
122122
otherFiles: { unitName: string; content: string }[],
123123
result: Harness.Compiler.CompilerResult
124124
) {
125-
126-
var outputLines: string[] = [];
127-
// Count up all the errors we find so we don't miss any
128-
var totalErrorsReported = 0;
129-
130-
function outputErrorText(error: Harness.Compiler.MinimalDiagnostic) {
131-
var errLines = RunnerBase.removeFullPaths(error.message)
132-
.split('\n')
133-
.map(s => s.length > 0 && s.charAt(s.length - 1) === '\r' ? s.substr(0, s.length - 1) : s)
134-
.filter(s => s.length > 0)
135-
.map(s => '!!! ' + s);
136-
errLines.forEach(e => outputLines.push(e));
137-
138-
totalErrorsReported++;
139-
}
140-
141-
// Report glovbal errors:
142-
var globalErrors = result.errors.filter(err => !err.filename);
143-
globalErrors.forEach(err => outputErrorText(err));
144-
145-
// 'merge' the lines of each input file with any errors associated with it
146-
toBeCompiled.concat(otherFiles).forEach(inputFile => {
147-
// Filter down to the errors in the file
148-
var fileErrors = result.errors.filter(e => {
149-
var errFn = e.filename;
150-
return errFn && errFn === inputFile.unitName;
151-
});
152-
153-
154-
// Header
155-
outputLines.push('==== ' + inputFile.unitName + ' (' + fileErrors.length + ' errors) ====');
156-
157-
// Make sure we emit something for every error
158-
var markedErrorCount = 0;
159-
// For each line, emit the line followed by any error squiggles matching this line
160-
// Note: IE JS engine incorrectly handles consecutive delimiters here when using RegExp split, so
161-
// we have to string-based splitting instead and try to figure out the delimiting chars
162-
163-
var lineStarts = ts.getLineStarts(inputFile.content);
164-
var lines = inputFile.content.split('\n');
165-
lines.forEach((line, lineIndex) => {
166-
if (line.length > 0 && line.charAt(line.length - 1) === '\r') {
167-
line = line.substr(0, line.length - 1);
168-
}
169-
170-
var thisLineStart = lineStarts[lineIndex];
171-
var nextLineStart: number;
172-
// On the last line of the file, fake the next line start number so that we handle errors on the last character of the file correctly
173-
if (lineIndex === lines.length - 1) {
174-
nextLineStart = inputFile.content.length;
175-
} else {
176-
nextLineStart = lineStarts[lineIndex + 1];
177-
}
178-
// Emit this line from the original file
179-
outputLines.push(' ' + line);
180-
fileErrors.forEach(err => {
181-
// Does any error start or continue on to this line? Emit squiggles
182-
if ((err.end >= thisLineStart) && ((err.start < nextLineStart) || (lineIndex === lines.length - 1))) {
183-
// How many characters from the start of this line the error starts at (could be positive or negative)
184-
var relativeOffset = err.start - thisLineStart;
185-
// How many characters of the error are on this line (might be longer than this line in reality)
186-
var length = (err.end - err.start) - Math.max(0, thisLineStart - err.start);
187-
// Calculate the start of the squiggle
188-
var squiggleStart = Math.max(0, relativeOffset);
189-
// TODO/REVIEW: this doesn't work quite right in the browser if a multi file test has files whose names are just the right length relative to one another
190-
outputLines.push(' ' + line.substr(0, squiggleStart).replace(/[^\s]/g, ' ') + new Array(Math.min(length, line.length - squiggleStart) + 1).join('~'));
191-
192-
// If the error ended here, or we're at the end of the file, emit its message
193-
if ((lineIndex === lines.length - 1) || nextLineStart > err.end) {
194-
// Just like above, we need to do a split on a string instead of on a regex
195-
// because the JS engine does regexes wrong
196-
197-
outputErrorText(err);
198-
markedErrorCount++;
199-
}
200-
}
201-
});
202-
});
203-
204-
// Verify we didn't miss any errors in this file
205-
assert.equal(markedErrorCount, fileErrors.length, 'count of errors in ' + inputFile.unitName);
206-
});
207-
208-
// Verify we didn't miss any errors in total
209-
assert.equal(totalErrorsReported, result.errors.length, 'total number of errors');
210-
211-
return outputLines.join('\r\n');
125+
return Harness.Compiler.getErrorBaseline(toBeCompiled.concat(otherFiles), result.errors);
212126
}
213127

214128
// check errors

src/harness/harness.ts

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,7 @@ module Harness {
747747
var errors: MinimalDiagnostic[] = [];
748748
program.getDiagnostics().concat(checker.getDiagnostics()).concat(emitResult ? emitResult.errors : []).forEach(err => {
749749
// TODO: new compiler formats errors after this point to add . and newlines so we'll just do it manually for now
750-
errors.push({ filename: err.file && err.file.filename, start: err.start, end: err.start + err.length, line: 0, character: 0, message: err.messageText });
750+
errors.push(getMinimalDiagnostic(err));
751751
});
752752
this.lastErrors = errors;
753753

@@ -762,6 +762,102 @@ module Harness {
762762
}
763763
}
764764

765+
export function getMinimalDiagnostic(err: ts.Diagnostic): MinimalDiagnostic {
766+
return { filename: err.file && err.file.filename, start: err.start, end: err.start + err.length, line: 0, character: 0, message: err.messageText };
767+
}
768+
769+
export function getErrorBaseline(inputFiles: { unitName: string; content: string }[],
770+
diagnostics: MinimalDiagnostic[]
771+
) {
772+
773+
var outputLines: string[] = [];
774+
// Count up all the errors we find so we don't miss any
775+
var totalErrorsReported = 0;
776+
777+
function outputErrorText(error: Harness.Compiler.MinimalDiagnostic) {
778+
var errLines = RunnerBase.removeFullPaths(error.message)
779+
.split('\n')
780+
.map(s => s.length > 0 && s.charAt(s.length - 1) === '\r' ? s.substr(0, s.length - 1) : s)
781+
.filter(s => s.length > 0)
782+
.map(s => '!!! ' + s);
783+
errLines.forEach(e => outputLines.push(e));
784+
785+
totalErrorsReported++;
786+
}
787+
788+
// Report glovbal errors:
789+
var globalErrors = diagnostics.filter(err => !err.filename);
790+
globalErrors.forEach(err => outputErrorText(err));
791+
792+
// 'merge' the lines of each input file with any errors associated with it
793+
inputFiles.forEach(inputFile => {
794+
// Filter down to the errors in the file
795+
var fileErrors = diagnostics.filter(e => {
796+
var errFn = e.filename;
797+
return errFn && errFn === inputFile.unitName;
798+
});
799+
800+
801+
// Header
802+
outputLines.push('==== ' + inputFile.unitName + ' (' + fileErrors.length + ' errors) ====');
803+
804+
// Make sure we emit something for every error
805+
var markedErrorCount = 0;
806+
// For each line, emit the line followed by any error squiggles matching this line
807+
// Note: IE JS engine incorrectly handles consecutive delimiters here when using RegExp split, so
808+
// we have to string-based splitting instead and try to figure out the delimiting chars
809+
810+
var lineStarts = ts.getLineStarts(inputFile.content);
811+
var lines = inputFile.content.split('\n');
812+
lines.forEach((line, lineIndex) => {
813+
if (line.length > 0 && line.charAt(line.length - 1) === '\r') {
814+
line = line.substr(0, line.length - 1);
815+
}
816+
817+
var thisLineStart = lineStarts[lineIndex];
818+
var nextLineStart: number;
819+
// On the last line of the file, fake the next line start number so that we handle errors on the last character of the file correctly
820+
if (lineIndex === lines.length - 1) {
821+
nextLineStart = inputFile.content.length;
822+
} else {
823+
nextLineStart = lineStarts[lineIndex + 1];
824+
}
825+
// Emit this line from the original file
826+
outputLines.push(' ' + line);
827+
fileErrors.forEach(err => {
828+
// Does any error start or continue on to this line? Emit squiggles
829+
if ((err.end >= thisLineStart) && ((err.start < nextLineStart) || (lineIndex === lines.length - 1))) {
830+
// How many characters from the start of this line the error starts at (could be positive or negative)
831+
var relativeOffset = err.start - thisLineStart;
832+
// How many characters of the error are on this line (might be longer than this line in reality)
833+
var length = (err.end - err.start) - Math.max(0, thisLineStart - err.start);
834+
// Calculate the start of the squiggle
835+
var squiggleStart = Math.max(0, relativeOffset);
836+
// TODO/REVIEW: this doesn't work quite right in the browser if a multi file test has files whose names are just the right length relative to one another
837+
outputLines.push(' ' + line.substr(0, squiggleStart).replace(/[^\s]/g, ' ') + new Array(Math.min(length, line.length - squiggleStart) + 1).join('~'));
838+
839+
// If the error ended here, or we're at the end of the file, emit its message
840+
if ((lineIndex === lines.length - 1) || nextLineStart > err.end) {
841+
// Just like above, we need to do a split on a string instead of on a regex
842+
// because the JS engine does regexes wrong
843+
844+
outputErrorText(err);
845+
markedErrorCount++;
846+
}
847+
}
848+
});
849+
});
850+
851+
// Verify we didn't miss any errors in this file
852+
assert.equal(markedErrorCount, fileErrors.length, 'count of errors in ' + inputFile.unitName);
853+
});
854+
855+
// Verify we didn't miss any errors in total
856+
assert.equal(totalErrorsReported, diagnostics.length, 'total number of errors');
857+
858+
return outputLines.join('\r\n');
859+
}
860+
765861
/* TODO: Delete?
766862
export function makeDefaultCompilerSettings(options?: { useMinimalDefaultLib: boolean; noImplicitAny: boolean; }) {
767863
var useMinimalDefaultLib = options ? options.useMinimalDefaultLib : true;

src/harness/projectsRunner.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,14 @@ class ProjectRunner extends RunnerBase {
310310
}
311311
}
312312

313+
var inputFiles = ts.map(ts.filter(compilerResult.program.getSourceFiles(),
314+
sourceFile => sourceFile.filename !== "lib.d.ts"),
315+
sourceFile => {
316+
return { unitName: sourceFile.filename, content: sourceFile.text };
317+
});
318+
var diagnostics = ts.map(compilerResult.errors, error => Harness.Compiler.getMinimalDiagnostic(error));
319+
errors += sys.newLine + sys.newLine + Harness.Compiler.getErrorBaseline(inputFiles, diagnostics);
320+
313321
return errors;
314322
}
315323

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,23 @@
11
decl.ts(1,26): Cannot find external module './foo/bar.js'.
22
decl.ts(2,26): Cannot find external module 'baz'.
33
decl.ts(3,26): Cannot find external module './baz'.
4+
5+
6+
==== decl.ts (3 errors) ====
7+
import modErr = require("./foo/bar.js");
8+
~~~~~~~~~~~~~~
9+
!!! Cannot find external module './foo/bar.js'.
10+
import modErr1 = require("baz");
11+
~~~~~
12+
!!! Cannot find external module 'baz'.
13+
import modErr2 = require("./baz");
14+
~~~~~~~
15+
!!! Cannot find external module './baz'.
16+
17+
//import modErr1 = require("\bar");
18+
19+
//import mod = require("./foo/bar");
20+
//import mod1 = require("../module paths/foo/bar");
21+
//import mod2 = require("foo/bar");
22+
23+
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,23 @@
11
decl.ts(1,26): Cannot find external module './foo/bar.js'.
22
decl.ts(2,26): Cannot find external module 'baz'.
33
decl.ts(3,26): Cannot find external module './baz'.
4+
5+
6+
==== decl.ts (3 errors) ====
7+
import modErr = require("./foo/bar.js");
8+
~~~~~~~~~~~~~~
9+
!!! Cannot find external module './foo/bar.js'.
10+
import modErr1 = require("baz");
11+
~~~~~
12+
!!! Cannot find external module 'baz'.
13+
import modErr2 = require("./baz");
14+
~~~~~~~
15+
!!! Cannot find external module './baz'.
16+
17+
//import modErr1 = require("\bar");
18+
19+
//import mod = require("./foo/bar");
20+
//import mod1 = require("../module paths/foo/bar");
21+
//import mod2 = require("foo/bar");
22+
23+
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,22 @@
11
in2.d.ts(1,8): Duplicate identifier 'a'.
2+
3+
4+
==== decl.d.ts (0 errors) ====
5+
// bug 535531: duplicate identifier error reported for "import" declarations in separate files
6+
7+
declare module A
8+
{
9+
10+
class MyRoot { }
11+
12+
export module B
13+
{
14+
class MyClass{ }
15+
}
16+
}
17+
==== in1.d.ts (0 errors) ====
18+
import a = A;
19+
==== in2.d.ts (1 errors) ====
20+
import a = A;
21+
~
22+
!!! Duplicate identifier 'a'.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,22 @@
11
in2.d.ts(1,8): Duplicate identifier 'a'.
2+
3+
4+
==== decl.d.ts (0 errors) ====
5+
// bug 535531: duplicate identifier error reported for "import" declarations in separate files
6+
7+
declare module A
8+
{
9+
10+
class MyRoot { }
11+
12+
export module B
13+
{
14+
class MyClass{ }
15+
}
16+
}
17+
==== in1.d.ts (0 errors) ====
18+
import a = A;
19+
==== in2.d.ts (1 errors) ====
20+
import a = A;
21+
~
22+
!!! Duplicate identifier 'a'.
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
11
internal2.ts(2,2): Import declarations in an internal module cannot reference an external module.
2+
3+
4+
==== internal2.ts (1 errors) ====
5+
module outer {
6+
import g = require("external2")
7+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8+
!!! Import declarations in an internal module cannot reference an external module.
9+
export var a = g.square(5);
10+
export var b = "foo";
11+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
11
internal2.ts(2,2): Import declarations in an internal module cannot reference an external module.
2+
3+
4+
==== internal2.ts (1 errors) ====
5+
module outer {
6+
import g = require("external2")
7+
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8+
!!! Import declarations in an internal module cannot reference an external module.
9+
export var a = g.square(5);
10+
export var b = "foo";
11+
}
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,27 @@
11
Option mapRoot cannot be specified without specifying sourcemap option.
22
Option sourceRoot cannot be specified without specifying sourcemap option.
3+
4+
5+
!!! Option mapRoot cannot be specified without specifying sourcemap option.
6+
!!! Option sourceRoot cannot be specified without specifying sourcemap option.
7+
==== m1.ts (0 errors) ====
8+
var m1_a1 = 10;
9+
class m1_c1 {
10+
public m1_c1_p1: number;
11+
}
12+
13+
var m1_instance1 = new m1_c1();
14+
function m1_f1() {
15+
return m1_instance1;
16+
}
17+
==== test.ts (0 errors) ====
18+
/// <reference path='m1.ts'/>
19+
var a1 = 10;
20+
class c1 {
21+
public p1: number;
22+
}
23+
24+
var instance1 = new c1();
25+
function f1() {
26+
return instance1;
27+
}

0 commit comments

Comments
 (0)