Skip to content

Commit f01702c

Browse files
Closure Teamcopybara-github
Closure Team
authored andcommitted
Update TestErrorReporter to deduplicate identical errors
PiperOrigin-RevId: 722805984
1 parent c2052d3 commit f01702c

File tree

4 files changed

+30
-10
lines changed

4 files changed

+30
-10
lines changed

src/com/google/javascript/rhino/testing/TestErrorReporter.java

+14-6
Original file line numberDiff line numberDiff line change
@@ -44,24 +44,28 @@
4444
import com.google.javascript.rhino.ErrorReporter;
4545
import java.util.ArrayList;
4646
import java.util.Collections;
47+
import java.util.LinkedHashMap;
4748

4849
/**
4950
* An error reporter for testing that verifies that messages reported to the reporter are expected.
5051
*/
5152
public final class TestErrorReporter implements ErrorReporter {
5253
private final ArrayList<String> expectedErrors = new ArrayList<>();
5354
private final ArrayList<String> expectedWarnings = new ArrayList<>();
54-
private final ArrayList<String> seenErrors = new ArrayList<>();
55-
private final ArrayList<String> seenWarnings = new ArrayList<>();
55+
56+
// Seen diagnostics are stored in a LinkedHashMap to deduplicate identical diagnostics.
57+
// The default com.google.javascript.jscomp.SortingErrorManager behaves in this way.
58+
private final LinkedHashMap<String, String> seenErrors = new LinkedHashMap<>();
59+
private final LinkedHashMap<String, String> seenWarnings = new LinkedHashMap<>();
5660

5761
@Override
5862
public void error(String message, String sourceName, int line, int lineOffset) {
59-
this.seenErrors.add(message);
63+
seenErrors.put(fmtDiagnosticKey(message, sourceName, line, lineOffset), message);
6064
}
6165

6266
@Override
6367
public void warning(String message, String sourceName, int line, int lineOffset) {
64-
this.seenWarnings.add(message);
68+
seenWarnings.put(fmtDiagnosticKey(message, sourceName, line, lineOffset), message);
6569
}
6670

6771
public TestErrorReporter expectAllErrors(String... errors) {
@@ -75,7 +79,11 @@ public TestErrorReporter expectAllWarnings(String... warnings) {
7579
}
7680

7781
public void verifyHasEncounteredAllWarningsAndErrors() {
78-
assertThat(seenWarnings).containsExactlyElementsIn(expectedWarnings).inOrder();
79-
assertThat(seenErrors).containsExactlyElementsIn(expectedErrors).inOrder();
82+
assertThat(seenWarnings.values()).containsExactlyElementsIn(expectedWarnings).inOrder();
83+
assertThat(seenErrors.values()).containsExactlyElementsIn(expectedErrors).inOrder();
84+
}
85+
86+
private String fmtDiagnosticKey(String message, String sourceName, int line, int lineOffset) {
87+
return message + ":" + sourceName + ":" + line + ":" + lineOffset;
8088
}
8189
}

test/com/google/javascript/jscomp/parsing/ParserTest.java

+15-1
Original file line numberDiff line numberDiff line change
@@ -4121,6 +4121,21 @@ public void testStringContinuationIssue3492() {
41214121
expectFeatures(Feature.STRING_CONTINUATION);
41224122
strictMode = SLOPPY;
41234123

4124+
// This test runs on this input code, which technically has STRING_CONTINUATIONS_WARNINGs at 2
4125+
// places:
4126+
// ```
4127+
// function x() {
4128+
// a = "\ <--- STRING_CONTINUATIONS_WARNING
4129+
// \ \ <--- STRING_CONTINUATIONS_WARNING
4130+
// ";
4131+
// };
4132+
// ```
4133+
// We deduplicate based on the key `sourceName:line_no:col_no:warning` being the same. Here,
4134+
// even though the `STRING_CONTINUATIONS_WARNING` happens at 2 places, JSCompiler reports the
4135+
// warnings within a string with a location pointing to the start of the string token. This
4136+
// happens here `charno(token.location.start)`:
4137+
// google3/third_party/java_src/jscomp/java/com/google/javascript/jscomp/parsing/IRFactory.java?rcl=719034735&l=3482
4138+
// Hence, after deduplication, we only get one error.
41244139
parseWarning(
41254140
lines(
41264141
"function x() {", //
@@ -4129,7 +4144,6 @@ public void testStringContinuationIssue3492() {
41294144
" \";",
41304145
"};"),
41314146
"Unnecessary escape: '\\ ' is equivalent to just ' '",
4132-
STRING_CONTINUATIONS_WARNING,
41334147
STRING_CONTINUATIONS_WARNING);
41344148
}
41354149

test/com/google/javascript/rhino/jstype/JSTypeTest.java

+1-2
Original file line numberDiff line numberDiff line change
@@ -4728,8 +4728,7 @@ public void testTypeOfThisIsProxied() {
47284728
public void testNamedTypeEquals() {
47294729
try (JSTypeResolver.Closer closer = registry.getResolver().openForDefinition()) {
47304730
// test == if references are equal
4731-
errorReporter.expectAllWarnings(
4732-
"Bad type annotation. Unknown type type1", "Bad type annotation. Unknown type type1");
4731+
errorReporter.expectAllWarnings("Bad type annotation. Unknown type type1");
47334732
NamedType a = registry.createNamedType(EMPTY_SCOPE, "type1", "source", 1, 0);
47344733
NamedType b = registry.createNamedType(EMPTY_SCOPE, "type1", "source", 1, 0);
47354734
assertThat(a.equals(b)).isTrue();

test/com/google/javascript/rhino/jstype/UnionTypeBuilderTest.java

-1
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,6 @@ public void testUnknownTypes() {
157157
@Test
158158
public void testUnresolvedNamedTypes() {
159159
errorReporter.expectAllWarnings(
160-
"Bad type annotation. Unknown type not.resolved.A",
161160
"Bad type annotation. Unknown type not.resolved.A",
162161
"Bad type annotation. Unknown type not.resolved.B");
163162

0 commit comments

Comments
 (0)