Rollforward of "Migrate default error manager in Compiler to the new …

…SortingErrorManager"

Automated g4 rollback of changelist 212361708.

*** Reason for rollback ***

Rollforward with fixes

*** Original change description ***

Automated g4 rollback of changelist 212339047.

*** Reason for rollback ***

Hits ConcurrentModificationExceptions:

*** Original change description ***

Migrate default error manager in Compiler to the new SortingErrorManager

This also involved porting the old PrintStreamErrorManager to a
ErrorReportGenertor, since there was no functionality rel...

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=212677704

src/com/google/javascript/jscomp/BasicErrorManager.java

@@ -17,8 +17,6 @@
 package com.google.javascript.jscomp;
 
 import com.google.common.collect.ImmutableSet;
-import java.util.ArrayList;
-import java.util.List;
 
 /**
  * An error manager that generates a sorted report when the {@link #generateReport()} method is
@@ -37,13 +35,9 @@ public BasicErrorManager() {
 
   @Override
   public void generateReport() {
-    List<ErrorWithLevel> list = new ArrayList<>();
-    for (ErrorWithLevel message = messages.poll(); message != null; message = messages.poll()) {
+    for (ErrorWithLevel message : super.getSortedDiagnostics()) {
       println(message.level, message.error);
-      list.add(message);
     }
-    // Restore the messages since some tests assert the values after generating the report.
-    messages.addAll(list);
     printSummary();
   }
 

src/com/google/javascript/jscomp/Compiler.java

@@ -27,6 +27,7 @@
 import com.google.common.base.Supplier;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Sets;
 import com.google.debugging.sourcemap.SourceMapConsumerV3;
@@ -336,10 +337,10 @@ public void initOptions(CompilerOptions options) {
         setErrorManager(
             new LoggerErrorManager(createMessageFormatter(), logger));
       } else {
-        PrintStreamErrorManager printer =
-            new PrintStreamErrorManager(createMessageFormatter(), this.outStream);
-        printer.setSummaryDetailLevel(options.summaryDetailLevel);
-        setErrorManager(printer);
+        PrintStreamErrorReportGenerator printer =
+            new PrintStreamErrorReportGenerator(
+                createMessageFormatter(), this.outStream, options.summaryDetailLevel);
+        setErrorManager(new SortingErrorManager(ImmutableSet.of(printer)));
       }
     }
 

src/com/google/javascript/jscomp/PrintStreamErrorManager.java

@@ -17,18 +17,19 @@
 package com.google.javascript.jscomp;
 
 import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat;
-
 import java.io.PrintStream;
 
 /**
- * <p>An error manager that prints errors and warnings to the print stream
- * provided in addition to the functionality of the
- * {@link BasicErrorManager}.</p>
+ * An error manager that prints errors and warnings to the print stream provided in addition to the
+ * functionality of the {@link BasicErrorManager}.
  *
- * <p>It collaborates with a {@link SourceExcerptProvider} via a
- * {@link MessageFormatter} to display error messages with source context.</p>
+ * <p>It collaborates with a {@link SourceExcerptProvider} via a {@link MessageFormatter} to display
+ * error messages with source context.
  *
+ * @deprecated, Please use the {#SortingErrorManger} with a {#PrintStreamErrorReportGenerator}
+ *     instead.
  */
+@Deprecated
 public class PrintStreamErrorManager extends BasicErrorManager {
   private final MessageFormatter formatter;
   private final PrintStream stream;

src/com/google/javascript/jscomp/PrintStreamErrorReportGenerator.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright 2007 The Closure Compiler Authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.javascript.jscomp;
+
+import com.google.javascript.jscomp.parsing.parser.util.format.SimpleFormat;
+import java.io.PrintStream;
+
+/**
+ * An error report generator that prints errors and warnings to the print stream provided.
+ *
+ * <p>It collaborates with a {@link SourceExcerptProvider} via a {@link MessageFormatter} to display
+ * error messages with source context.
+ */
+public class PrintStreamErrorReportGenerator implements SortingErrorManager.ErrorReportGenerator {
+  private final MessageFormatter formatter;
+  private final PrintStream stream;
+  private final int summaryDetailLevel;
+
+  /**
+   * Creates an error report generator.
+   *
+   * @param formatter the message formatter used to format the messages
+   * @param stream the stream on which the errors and warnings should be printed. This class does
+   *     not close the stream
+   */
+  public PrintStreamErrorReportGenerator(
+      MessageFormatter formatter, PrintStream stream, int summaryDetailLevel) {
+    this.formatter = formatter;
+    this.stream = stream;
+    this.summaryDetailLevel = summaryDetailLevel;
+  }
+
+  @Override
+  public void generateReport(SortingErrorManager manager) {
+    for (SortingErrorManager.ErrorWithLevel e : manager.getSortedDiagnostics()) {
+      println(e.level, e.error);
+    }
+    printSummary(manager);
+  }
+
+  private void println(CheckLevel level, JSError error) {
+    stream.println(error.format(level, formatter));
+  }
+
+  private void printSummary(ErrorManager manager) {
+    if (summaryDetailLevel >= 3
+        || (summaryDetailLevel >= 1 && manager.getErrorCount() + manager.getWarningCount() > 0)
+        || (summaryDetailLevel >= 2 && manager.getTypedPercent() > 0.0)) {
+      if (manager.getTypedPercent() > 0.0) {
+        stream.print(
+            SimpleFormat.format(
+                "%d error(s), %d warning(s), %.1f%% typed%n",
+                manager.getErrorCount(), manager.getWarningCount(), manager.getTypedPercent()));
+      } else {
+        stream.print(
+            SimpleFormat.format(
+                "%d error(s), %d warning(s)%n",
+                manager.getErrorCount(), manager.getWarningCount()));
+      }
+    }
+  }
+}

src/com/google/javascript/jscomp/SortingErrorManager.java

@@ -16,25 +16,22 @@
 
 package com.google.javascript.jscomp;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
-import java.util.PriorityQueue;
 import java.util.Set;
+import java.util.TreeSet;
 
 /**
  * A customizable error manager that sorts all errors and warnings reported to it, and has
  * customizable output through the {@link ErrorReportGenerator} interface.
  */
 public class SortingErrorManager implements ErrorManager {
 
-  final PriorityQueue<ErrorWithLevel> messages =
-      new PriorityQueue<>(1, new LeveledJSErrorComparator());
-  private final Set<ErrorWithLevel> alreadyAdded = new HashSet<>();
+  private final TreeSet<ErrorWithLevel> messages = new TreeSet<>(new LeveledJSErrorComparator());
   private int originalErrorCount = 0;
   private int promotedErrorCount = 0;
   private int warningCount = 0;
@@ -50,8 +47,7 @@ public SortingErrorManager(Set<ErrorReportGenerator> errorReportGenerators) {
   @Override
   public void report(CheckLevel level, JSError error) {
     ErrorWithLevel e = new ErrorWithLevel(error, level);
-    if (alreadyAdded.add(e)) {
-      messages.add(e);
+    if (messages.add(e)) {
       if (level == CheckLevel.ERROR) {
         if (error.getType().level == CheckLevel.ERROR) {
           originalErrorCount++;
@@ -90,7 +86,9 @@ public JSError[] getWarnings() {
   }
 
   Iterable<ErrorWithLevel> getSortedDiagnostics() {
-    return Collections.unmodifiableCollection(messages);
+    // TODO(b/114762232): It should be possible to remove the copying here and switch to an
+    // unmodifiable collection once we get rid of usages that add warnings during generateReport.
+    return ImmutableList.copyOf(messages);
   }
 
   @Override
@@ -113,6 +111,8 @@ private JSError[] toArray(CheckLevel level) {
     return errors.toArray(new JSError[0]);
   }
 
+  // TODO(b/114762232): It should be invalid to report errors during the execution of this method;
+  // doing so will become impossible once all subclases have migrated to an ErrorReportGenerator.
   @Override
   public void generateReport() {
     for (ErrorReportGenerator generator : this.errorReportGenerators) {

test/com/google/javascript/jscomp/SortingErrorManagerTest.java

@@ -123,11 +123,7 @@ protected void printSummary() { }
     assertThat(printedErrors).hasSize(1);
   }
 
-  // Ensure that more warnings can be added from generating the report.
-  // One case in which this happened is when the report tries to use the source maps to map back to
-  // the original source, yet a warning was produced because of a corrupted source map. This test
-  // ensures that there is no java.util.ConcurrentModificationException while adding a new warning
-  // when iterating over the existing warnings to print them out.
+  // This test is testing a "feature" that seems bogus and should likely be forbidden.
   public void testGenerateReportCausesMoreWarnings() {
     BasicErrorManager manager =
         new BasicErrorManager() {
@@ -136,14 +132,16 @@ public void testGenerateReportCausesMoreWarnings() {
           @Override
           public void println(CheckLevel level, JSError error) {
             if (error.getType().equals(FOO_TYPE)) {
+              // TODO(b/114762232) This behavior should not be supported, and will become malformed
+              // when migrating to a SortingErrorManager with an ErrorReportGenerator.
               this.report(CheckLevel.ERROR, JSError.make(NULL_SOURCE, -1, -1, JOO_TYPE));
             }
             printed++;
           }
 
           @Override
           protected void printSummary() {
-            assertEquals(2, printed);
+            assertEquals(1, printed);
           }
         };
     manager.report(CheckLevel.ERROR, JSError.make(NULL_SOURCE, -1, -1, FOO_TYPE));