Skip to content

Commit

Permalink
Abstract out the error printing strategy as a new interface
Browse files Browse the repository at this point in the history 
This makes it so that one can create custom error printing set-ups by passing
a strategy to the standard error manager, rather than always needing to create
a subclass.  As an example, converted the JSON stream report generator from
a subclass of the standard error manger to an instance of the new interface.

The end goal is to support multiple different ErrorReportGenerators to run
within a single compiler, like for example to print out normally formatted
messages to the console and at the same time also print JSON formatted messages
to an extra output file.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=211713942
  • Loading branch information
@blickly @lauraharker
blickly authored and lauraharker committed Sep 6, 2018
1 parent 593f505 commit 35a2aea
Show file tree
Hide file tree
Showing 7 changed files with 283 additions and 215 deletions.
7 changes: 4 additions & 3 deletions src/com/google/javascript/jscomp/AbstractCommandLineRunner.java
View file
Expand Up @@ -30,6 +30,7 @@
import com.google.common.base.Supplier; import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.io.ByteStreams; import com.google.common.io.ByteStreams;
Expand Down Expand Up @@ -475,9 +476,9 @@ protected void setRunOptions(CompilerOptions options) throws IOException {
options.instrumentationTemplateFile = config.instrumentationTemplateFile; options.instrumentationTemplateFile = config.instrumentationTemplateFile;


if (config.errorFormat == CommandLineConfig.ErrorFormatOption.JSON) { if (config.errorFormat == CommandLineConfig.ErrorFormatOption.JSON) {
PrintStreamJSONErrorManager printer = JsonErrorReportGenerator errorGenerator =
new PrintStreamJSONErrorManager(getErrorPrintStream(), compiler); new JsonErrorReportGenerator(getErrorPrintStream(), compiler);
compiler.setErrorManager(printer); compiler.setErrorManager(new SortingErrorManager(ImmutableSet.of(errorGenerator)));
} }
} }


Expand Down
180 changes: 10 additions & 170 deletions src/com/google/javascript/jscomp/BasicErrorManager.java
View file
Expand Up @@ -16,48 +16,23 @@


package com.google.javascript.jscomp; package com.google.javascript.jscomp;


import com.google.common.base.Objects; import com.google.common.collect.ImmutableSet;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.PriorityQueue;
import java.util.Set;


/** /**
* <p>A basic error manager that sorts all errors and warnings reported to it to * An error manager that generates a sorted report when the {@link #generateReport()} method is
* generate a sorted report when the {@link #generateReport()} method * called.
* is called.</p>
*
* <p>This error manager does not produce any output, but subclasses can
* override the {@link #println(CheckLevel, JSError)} method to generate custom
* output.</p>
* *
* <p>This error manager does not produce any output, but subclasses can override the {@link
* #println(CheckLevel, JSError)} method to generate custom output. Consider using the
* SortingErrorManager with a custom {@link SortingErrorManager.ErrorReportGenerator} instead.
*/ */
public abstract class BasicErrorManager implements ErrorManager { @Deprecated
final PriorityQueue<ErrorWithLevel> messages = public abstract class BasicErrorManager extends SortingErrorManager {
new PriorityQueue<>(1, new LeveledJSErrorComparator());
private final Set<ErrorWithLevel> alreadyAdded = new HashSet<>();
private int originalErrorCount = 0;
private int promotedErrorCount = 0;
private int warningCount = 0;
private double typedPercent = 0.0;


@Override public BasicErrorManager() {
public void report(CheckLevel level, JSError error) { super(ImmutableSet.of());
ErrorWithLevel e = new ErrorWithLevel(error, level);
if (alreadyAdded.add(e)) {
messages.add(e);
if (level == CheckLevel.ERROR) {
if (error.getType().level == CheckLevel.ERROR) {
originalErrorCount++;
} else {
promotedErrorCount++;
}
} else if (level == CheckLevel.WARNING) {
warningCount++;
}
}
} }


@Override @Override
Expand All @@ -82,139 +57,4 @@ public void generateReport() {
* Print the summary of the compilation - number of errors and warnings. * Print the summary of the compilation - number of errors and warnings.
*/ */
protected abstract void printSummary(); protected abstract void printSummary();

@Override
public boolean hasHaltingErrors() {
return originalErrorCount != 0;
}

@Override
public int getErrorCount() {
return originalErrorCount + promotedErrorCount;
}

@Override
public int getWarningCount() {
return warningCount;
}

@Override
public JSError[] getErrors() {
return toArray(CheckLevel.ERROR);
}

@Override
public JSError[] getWarnings() {
return toArray(CheckLevel.WARNING);
}

@Override
public void setTypedPercent(double typedPercent) {
this.typedPercent = typedPercent;
}

@Override
public double getTypedPercent() {
return typedPercent;
}

private JSError[] toArray(CheckLevel level) {
List<JSError> errors = new ArrayList<>(messages.size());
for (ErrorWithLevel p : messages) {
if (p.level == level) {
errors.add(p.error);
}
}
return errors.toArray(new JSError[0]);
}

/**
* Comparator of {@link JSError} with an associated {@link CheckLevel}. The ordering is the
* standard lexical ordering on the quintuple (file name, line number, {@link CheckLevel},
* character number, description).
*
* <p>Note: this comparator imposes orderings that are inconsistent with {@link
* JSError#equals(Object)}.
*/
static final class LeveledJSErrorComparator implements Comparator<ErrorWithLevel> {
private static final int P1_LT_P2 = -1;
private static final int P1_GT_P2 = 1;

@Override
public int compare(ErrorWithLevel p1, ErrorWithLevel p2) {
// null is the smallest value
if (p2 == null) {
if (p1 == null) {
return 0;
} else {
return P1_GT_P2;
}
}

// check level
if (p1.level != p2.level) {
return p2.level.compareTo(p1.level);
}

// sourceName comparison
String source1 = p1.error.sourceName;
String source2 = p2.error.sourceName;
if (source1 != null && source2 != null) {
int sourceCompare = source1.compareTo(source2);
if (sourceCompare != 0) {
return sourceCompare;
}
} else if (source1 == null && source2 != null) {
return P1_LT_P2;
} else if (source1 != null && source2 == null) {
return P1_GT_P2;
}
// lineno comparison
int lineno1 = p1.error.lineNumber;
int lineno2 = p2.error.lineNumber;
if (lineno1 != lineno2) {
return lineno1 - lineno2;
} else if (lineno1 < 0 && 0 <= lineno2) {
return P1_LT_P2;
} else if (0 <= lineno1 && lineno2 < 0) {
return P1_GT_P2;
}
// charno comparison
int charno1 = p1.error.getCharno();
int charno2 = p2.error.getCharno();
if (charno1 != charno2) {
return charno1 - charno2;
} else if (charno1 < 0 && 0 <= charno2) {
return P1_LT_P2;
} else if (0 <= charno1 && charno2 < 0) {
return P1_GT_P2;
}
// description
return p1.error.description.compareTo(p2.error.description);
}
}

static class ErrorWithLevel {
final JSError error;
final CheckLevel level;

ErrorWithLevel(JSError error, CheckLevel level) {
this.error = error;
this.level = level;
}

@Override
public int hashCode() {
return Objects.hashCode(
level, error.description, error.sourceName, error.lineNumber, error.getCharno());
}

@Override
public boolean equals(Object obj) {
if (obj == null) {
return false;
}
return obj.hashCode() == this.hashCode();
}
}
} }
12 changes: 6 additions & 6 deletions src/com/google/javascript/jscomp/BlackHoleErrorManager.java
View file
Expand Up @@ -15,11 +15,11 @@
*/ */
package com.google.javascript.jscomp; package com.google.javascript.jscomp;


/** An ErrorManager that silently swallows all messages. */ import com.google.common.collect.ImmutableSet;
public final class BlackHoleErrorManager extends BasicErrorManager {
@Override
public void println(CheckLevel level, JSError error) { /* no-op */ }


@Override /** An ErrorManager that silently swallows all messages. */
public void printSummary() { /* no-op */ } public final class BlackHoleErrorManager extends SortingErrorManager {
public BlackHoleErrorManager() {
super(ImmutableSet.of());
}
} }
44 changes: 12 additions & 32 deletions ...t/jscomp/PrintStreamJSONErrorManager.java → ...ript/jscomp/JsonErrorReportGenerator.java
View file
Expand Up @@ -20,21 +20,21 @@
import com.google.debugging.sourcemap.proto.Mapping.OriginalMapping; import com.google.debugging.sourcemap.proto.Mapping.OriginalMapping;
import com.google.gson.stream.JsonWriter; import com.google.gson.stream.JsonWriter;
import com.google.javascript.jscomp.LightweightMessageFormatter.LineNumberingFormatter; import com.google.javascript.jscomp.LightweightMessageFormatter.LineNumberingFormatter;
import com.google.javascript.jscomp.SortingErrorManager.ErrorReportGenerator;
import com.google.javascript.jscomp.SortingErrorManager.ErrorWithLevel;
import com.google.javascript.jscomp.SourceExcerptProvider.SourceExcerpt; import com.google.javascript.jscomp.SourceExcerptProvider.SourceExcerpt;
import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat; import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat;
import com.google.javascript.rhino.TokenUtil; import com.google.javascript.rhino.TokenUtil;
import java.io.ByteArrayOutputStream; import java.io.ByteArrayOutputStream;
import java.io.IOException; import java.io.IOException;
import java.io.OutputStreamWriter; import java.io.OutputStreamWriter;
import java.io.PrintStream; import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;


/** /**
* An error manager that prints error and warning data to the print stream as an array of JSON * An error report generator that prints error and warning data to the print stream as an array of
* objects provided in addition to the functionality of the {@link BasicErrorManager}. * JSON objects.
*/ */
public class PrintStreamJSONErrorManager extends BasicErrorManager { public class JsonErrorReportGenerator implements ErrorReportGenerator {
private final PrintStream stream; private final PrintStream stream;
private final SourceExcerptProvider sourceExcerptProvider; private final SourceExcerptProvider sourceExcerptProvider;
private static final LineNumberingFormatter excerptFormatter = new LineNumberingFormatter(); private static final LineNumberingFormatter excerptFormatter = new LineNumberingFormatter();
Expand All @@ -46,20 +46,18 @@ public class PrintStreamJSONErrorManager extends BasicErrorManager {
* not close the stream * not close the stream
* @param sourceExcerptProvider used to retrieve the source context which generated the error * @param sourceExcerptProvider used to retrieve the source context which generated the error
*/ */
public PrintStreamJSONErrorManager( public JsonErrorReportGenerator(PrintStream stream, SourceExcerptProvider sourceExcerptProvider) {
PrintStream stream, SourceExcerptProvider sourceExcerptProvider) {
this.stream = stream; this.stream = stream;
this.sourceExcerptProvider = sourceExcerptProvider; this.sourceExcerptProvider = sourceExcerptProvider;
} }


@Override @Override
@GwtIncompatible @GwtIncompatible
public void generateReport() { public void generateReport(SortingErrorManager manager) {
ByteArrayOutputStream bufferedStream = new ByteArrayOutputStream(); ByteArrayOutputStream bufferedStream = new ByteArrayOutputStream();
List<ErrorWithLevel> list = new ArrayList<>();
try (JsonWriter jsonWriter = new JsonWriter(new OutputStreamWriter(bufferedStream, "UTF-8"))) { try (JsonWriter jsonWriter = new JsonWriter(new OutputStreamWriter(bufferedStream, "UTF-8"))) {
jsonWriter.beginArray(); jsonWriter.beginArray();
for (ErrorWithLevel message = messages.poll(); message != null; message = messages.poll()) { for (ErrorWithLevel message : manager.getSortedDiagnostics()) {
String sourceName = message.error.sourceName; String sourceName = message.error.sourceName;
int lineNumber = message.error.getLineNumber(); int lineNumber = message.error.getLineNumber();
int charno = message.error.getCharno(); int charno = message.error.getCharno();
Expand Down Expand Up @@ -119,18 +117,18 @@ public void generateReport() {
} }


jsonWriter.endObject(); jsonWriter.endObject();
list.add(message);
} }


StringBuilder summaryBuilder = new StringBuilder(); StringBuilder summaryBuilder = new StringBuilder();
if (getTypedPercent() > 0.0) { if (manager.getTypedPercent() > 0.0) {
summaryBuilder.append( summaryBuilder.append(
SimpleFormat.format( SimpleFormat.format(
"%d error(s), %d warning(s), %.1f%% typed", "%d error(s), %d warning(s), %.1f%% typed",
getErrorCount(), getWarningCount(), getTypedPercent())); manager.getErrorCount(), manager.getWarningCount(), manager.getTypedPercent()));
} else { } else {
summaryBuilder.append( summaryBuilder.append(
SimpleFormat.format("%d error(s), %d warning(s)", getErrorCount(), getWarningCount())); SimpleFormat.format(
"%d error(s), %d warning(s)", manager.getErrorCount(), manager.getWarningCount()));
} }
jsonWriter.beginObject(); jsonWriter.beginObject();
jsonWriter.name("level").value("info"); jsonWriter.name("level").value("info");
Expand All @@ -139,27 +137,9 @@ public void generateReport() {


jsonWriter.endArray(); jsonWriter.endArray();
jsonWriter.flush(); jsonWriter.flush();
jsonWriter.close();
} catch (IOException e) { } catch (IOException e) {
throw new RuntimeException(e); throw new RuntimeException(e);
} }
stream.append(bufferedStream.toString()); stream.append(bufferedStream.toString());

// Restore the messages since some tests assert the values after generating the report.
messages.addAll(list);
}

// This class overrides generateReport(), so nothing will call println().
@Override
public void println(CheckLevel level, JSError error) {
throw new UnsupportedOperationException(
"should not be called for PrintStreamJSONErrorManager");
}

// This class overrides generateReport(), so nothing will call printSummary().
@Override
public void printSummary() {
throw new UnsupportedOperationException(
"should not be called for PrintStreamJSONErrorManager");
} }
} }

0 comments on commit 35a2aea

Please sign in to comment.