Fix missing error reporting in SDM that leads to NPE.

In certain cases SDM would ignore errors in compilation units and
the compile would abort with NPE.

UnifyAST has code to avoid reporting the same error multimple times but
it was incorrect and might miss reporting an error.

Bug: #9425
Change-Id: I0b4898d9533e3da8ae98129353194b8b879b6d54
Bug-Link: https://github.com/gwtproject/gwt/issues/9425
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
index b09cc19..94a6c6c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -679,7 +679,7 @@
    * to an UnableToCompleteException at the UnifyAst public function boundaries
    */
   private boolean errorsFound = false;
-  private final Set<CompilationUnit> failedUnits = Sets.newIdentityHashSet();
+  private final Set<CompilationUnit> unitsWithErrorsAlreadyReported = Sets.newIdentityHashSet();
   private final Map<String, JField> fieldMap = Maps.newHashMap();
 
   /**
@@ -983,7 +983,7 @@
 
   private void assimilateSourceUnit(CompilationUnit unit, boolean reportErrors) {
     if (unit.isError()) {
-      if (failedUnits.add(unit) && reportErrors) {
+      if (reportErrors && unitsWithErrorsAlreadyReported.add(unit)) {
         CompilationProblemReporter.reportErrors(logger, unit, false);
         CompilationProblemReporter.logErrorTrace(logger, TreeLogger.ERROR,
             compilerContext, unit.getTypeName(), true);
diff --git a/dev/core/test/com/google/gwt/dev/CompilerTest.java b/dev/core/test/com/google/gwt/dev/CompilerTest.java
index 90c649f..51b0275 100644
--- a/dev/core/test/com/google/gwt/dev/CompilerTest.java
+++ b/dev/core/test/com/google/gwt/dev/CompilerTest.java
@@ -1582,6 +1582,76 @@
     checkIncrementalRecompile_unstableGeneratorReferencesModifiedType(JsOutputOption.DETAILED);
   }
 
+  public void testIncrementalRecompile_withErrors()
+      throws UnableToCompleteException, IOException, InterruptedException {
+    MockResource moduleResource =
+        JavaResourceBase.createMockResource(
+            "com/foo/Errors.gwt.xml",
+            "<module>",
+            "  <source path=''/>",
+            "  <entry-point class='com.foo.ErrorsEntryPoint'/>",
+            "  <add-linker name='xsiframe'/>",
+            "</module>");
+
+    MockJavaResource entryPointResource =
+        JavaResourceBase.createMockJavaResource(
+            "com.foo.ErrorsEntryPoint",
+            "package com.foo;",
+            "import com.google.gwt.core.client.EntryPoint;",
+            "public class ErrorsEntryPoint implements EntryPoint {",
+            "  public void onModuleLoad() {",
+            "    Foo.foo();",
+            "  }",
+            "}");
+
+    MockJavaResource fooResource =
+        JavaResourceBase.createMockJavaResource(
+            "com.foo.Foo",
+            "package com.foo;",
+            "public class Foo {",
+            "  public static void foo() {",
+            "  }",
+            "}");
+
+    MockJavaResource fooResourceWithErrors =
+        JavaResourceBase.createMockJavaResource(
+            "com.foo.Foo",
+            "package com.foo;",
+            "public class Foo {",
+            "  public static void foo() {",
+            "    // x() method is not defined anywhere, should result in a compile error.",
+            "    x();",
+            "  }",
+            "}");
+
+    MinimalRebuildCache minimalRebuildCache = new MinimalRebuildCache();
+    File applicationDir = Files.createTempDir();
+    CompilerOptions compilerOptions = new CompilerOptionsImpl();
+    compilerOptions.setUseDetailedTypeIds(true);
+    compilerOptions.setSourceLevel(SourceLevel.JAVA8);
+
+    // Compile the application with no errors.
+    compileToJs(TreeLogger.NULL, compilerOptions, applicationDir, "com.foo.Errors",
+        Lists.newArrayList(moduleResource, entryPointResource, fooResource), minimalRebuildCache,
+        emptySet, JsOutputOption.OBFUSCATED);
+
+    // Recompile and expect error reporting.
+    UnitTestTreeLogger.Builder builder = new UnitTestTreeLogger.Builder();
+    builder.setLowestLogLevel(TreeLogger.ERROR);
+    builder.expectError("Line 5: The method x() is undefined for the type Foo", null);
+    UnitTestTreeLogger errorLogger = builder.createLogger();
+    try {
+      // Recompile but now the changed file has an error
+      compileToJs(errorLogger, compilerOptions, applicationDir, "com.foo.Errors",
+          Lists.newArrayList(moduleResource, entryPointResource, fooResourceWithErrors),
+          minimalRebuildCache,
+          emptySet, JsOutputOption.OBFUSCATED);
+      fail("Compile should have failed");
+    } catch (UnableToCompleteException expected) {
+      errorLogger.assertLogEntriesContainExpected();
+    }
+  }
+
   public void testIncrementalRecompile_functionSignatureChange() throws UnableToCompleteException,
       IOException, InterruptedException {
     // Not testing recompile equality with Pretty/Obfuscated output since the JsIncrementalNamer's