Fix CompilationStateBuilder for units that fail JDT compile.

Units that fail JDT compiler might be saved as cached units; these
might be resolved later and need their dependency information
to invalidate other units.

All the other information collected and checks performed in
UnitProcessorImpl that require traversing the JDT AST have
been turned off for units that fail JDT compile; when
units fail JDT compile, their JDT ASTs are left in an
inconsistent state that might cause exceptions when traversing.

This patch fixes a breakage when superdevmode recompiles
due to having units with JDT errors incorrectly represented.

TODO: have a saner approach to units that fail to compile
under JDT.

Change-Id: I8c9d7a59d4f4be424b81ba1a46993924e9ffc126
diff --git a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
index 1de4c16..aacba1a 100644
--- a/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java
@@ -34,6 +34,7 @@
 import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
 import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.EventType;
 import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
+import com.google.gwt.thirdparty.guava.common.collect.ImmutableMap;
 import com.google.gwt.thirdparty.guava.common.collect.Interner;
 import com.google.gwt.thirdparty.guava.common.collect.Lists;
 import com.google.gwt.thirdparty.guava.common.collect.Maps;
@@ -80,18 +81,25 @@
           List<CompiledClass> compiledClasses) {
         Event event = SpeedTracerLogger.start(DevModeEventType.CSB_PROCESS);
         try {
+          // Collect parameter method names event when the compilation unit has errors.
+
+          List<JDeclaredType> types = ImmutableList.of();
+          final Set<String> jsniDeps = Sets.newHashSet();
+          final Map<String, Binding> jsniRefs = Maps.newHashMap();
+          Map<MethodDeclaration, JsniMethod> jsniMethods = ImmutableMap.of();
+          List<String> apiRefs = ImmutableList.of();
+          MethodArgNamesLookup methodArgs = new MethodArgNamesLookup();
+
           if (!cud.compilationResult().hasErrors()) {
             // Only collect jsniMethod, artificial rescues, etc if the compilation unit
             // does not have errors.
-            Map<MethodDeclaration, JsniMethod> jsniMethods =
+            jsniMethods =
                 JsniMethodCollector.collectJsniMethods(cud, builder.getSourceMapPath(),
                     builder.getSource(), JsRootScope.INSTANCE, DummyCorrelationFactory.INSTANCE);
 
             JSORestrictionsChecker.check(jsoState, cud);
 
             // JSNI check + collect dependencies.
-            final Set<String> jsniDeps = Sets.newHashSet();
-            final Map<String, Binding> jsniRefs = Maps.newHashMap();
             JsniReferenceResolver
                 .resolve(cud, cudOriginaImports, jsoState, jsniMethods, jsniRefs,
                     new JsniReferenceResolver.TypeResolver() {
@@ -130,61 +138,58 @@
               BinaryTypeReferenceRestrictionsChecker.check(cud);
             }
 
-            MethodArgNamesLookup methodArgs = MethodParamCollector.collect(cud,
-                builder.getSourceMapPath());
-
-            final Interner<String> interner = StringInterner.get();
-            String packageName = interner.intern(Shared.getPackageName(builder.getTypeName()));
-
-            List<String> unresolvedSimple = Lists.newArrayList();
-            for (char[] simpleRef : cud.compilationResult().simpleNameReferences) {
-              unresolvedSimple.add(interner.intern(String.valueOf(simpleRef)));
-            }
-
-            List<String> unresolvedQualified = Lists.newArrayList();
-            for (char[][] qualifiedRef : cud.compilationResult().qualifiedReferences) {
-              unresolvedQualified.add(interner.intern(CharOperation.toString(qualifiedRef)));
-            }
-            for (String jsniDep : jsniDeps) {
-              unresolvedQualified.add(interner.intern(jsniDep));
-            }
-            List<String> apiRefs = compiler.collectApiRefs(cud);
-            for (int i = 0; i < apiRefs.size(); ++i) {
-              apiRefs.set(i, interner.intern(apiRefs.get(i)));
-            }
-
-            Dependencies dependencies =
-                new Dependencies(packageName, unresolvedQualified, unresolvedSimple, apiRefs);
-
-            List<JDeclaredType> types = ImmutableList.of();
             if (!cud.compilationResult().hasErrors()) {
-              // The above checks might have recorded errors.
+              // The above checks might have recorded errors; so we need to check here again.
               // So only construct the GWT AST if no JDT errors and no errors from our checks.
               types = astBuilder.process(cud, builder.getSourceMapPath(), artificialRescues,
                   jsniMethods, jsniRefs);
             }
 
-            for (CompiledClass cc : compiledClasses) {
-              allValidClasses.put(cc.getSourceName(), cc);
-            }
-            builder
-                .setTypes(types)
-                .setDependencies(dependencies)
-                .setJsniMethods(jsniMethods.values())
-                .setMethodArgs(methodArgs)
-                .setClasses(compiledClasses)
-                .setProblems(cud.compilationResult().getProblems());
-          } else {
-            // Compilation unit already has errors from JDT, make an empty builder so that the
-            // error state is retained and can be reported later.
-            builder
-                .setTypes(Collections.<JDeclaredType>emptyList())
-                .setDependencies(new Dependencies())
-                .setJsniMethods(Collections.<JsniMethod>emptyList())
-                .setMethodArgs(new MethodArgNamesLookup())
-                .setClasses(compiledClasses)
-                .setProblems(cud.compilationResult().getProblems());
+            // Only run this pass if JDT was able to compile the unit with no errors, otherwise
+            // the JDT AST traversal might throw exceptions.
+            apiRefs = compiler.collectApiRefs(cud);
+            methodArgs = MethodParamCollector.collect(cud, builder.getSourceMapPath());
           }
+
+          final Interner<String> interner = StringInterner.get();
+          String packageName = interner.intern(Shared.getPackageName(builder.getTypeName()));
+
+          List<String> unresolvedSimple = Lists.newArrayList();
+          for (char[] simpleRef : cud.compilationResult().simpleNameReferences) {
+            unresolvedSimple.add(interner.intern(String.valueOf(simpleRef)));
+          }
+
+          List<String> unresolvedQualified = Lists.newArrayList();
+          for (char[][] qualifiedRef : cud.compilationResult().qualifiedReferences) {
+            unresolvedQualified.add(interner.intern(CharOperation.toString(qualifiedRef)));
+          }
+          for (String jsniDep : jsniDeps) {
+            unresolvedQualified.add(interner.intern(jsniDep));
+          }
+          for (int i = 0; i < apiRefs.size(); ++i) {
+            apiRefs.set(i, interner.intern(apiRefs.get(i)));
+          }
+
+          // Dependencies need to be included even when the {@code cud} has errors as the unit might
+          // be saved as a cached unit and its dependencies might be used to further invalidate
+          // other units. See {@link
+          // CompilationStateBuilder#removeInvalidCachedUnitsAndRescheduleCorrespondingBuilders}.
+          Dependencies dependencies =
+              new Dependencies(packageName, unresolvedQualified, unresolvedSimple, apiRefs);
+
+          for (CompiledClass cc : compiledClasses) {
+            allValidClasses.put(cc.getSourceName(), cc);
+          }
+
+          // Even when compilation units have errors, return a consistent builder.
+          builder
+              .setTypes(types)
+              .setDependencies(dependencies)
+              .setJsniMethods(jsniMethods.values())
+              .setMethodArgs(methodArgs)
+              .setClasses(compiledClasses)
+              .setProblems(cud.compilationResult().getProblems());
+
           buildQueue.add(builder);
         } finally {
           event.end();
@@ -362,49 +367,7 @@
           unit.getDependencies().resolve(allValidClasses);
         }
 
-        /*
-         * Invalidate any cached units with invalid refs.
-         */
-        Collection<CompilationUnit> invalidatedUnits = Lists.newArrayList();
-        for (Iterator<Entry<CompilationUnitBuilder, CompilationUnit>> it =
-            cachedUnits.entrySet().iterator(); it.hasNext();) {
-          Entry<CompilationUnitBuilder, CompilationUnit> entry = it.next();
-          CompilationUnit unit = entry.getValue();
-          boolean isValid = unit.getDependencies().validate(logger, allValidClasses);
-          if (isValid && unit.isError()) {
-            // See if the unit has classes that can't provide a
-            // NameEnvironmentAnswer
-            for (CompiledClass cc : unit.getCompiledClasses()) {
-              try {
-                cc.getNameEnvironmentAnswer();
-              } catch (ClassFormatException ex) {
-                isValid = false;
-                break;
-              }
-            }
-          }
-          if (!isValid) {
-            if (logger.isLoggable(TreeLogger.TRACE)) {
-              logger.log(TreeLogger.TRACE, "Invalid Unit: " + unit.getTypeName());
-            }
-            invalidatedUnits.add(unit);
-            builders.add(entry.getKey());
-            it.remove();
-          }
-        }
-
-        if (invalidatedUnits.size() > 0) {
-          if (logger.isLoggable(TreeLogger.TRACE)) {
-            logger.log(TreeLogger.TRACE, "Invalid units found: " + invalidatedUnits.size());
-          }
-        }
-
-        // Any units we invalidated must now be removed from the valid classes.
-        for (CompilationUnit unit : invalidatedUnits) {
-          for (CompiledClass cc : unit.getCompiledClasses()) {
-            allValidClasses.remove(cc.getSourceName());
-          }
-        }
+        removeInvalidCachedUnitsAndRescheduleCorrespondingBuilders(logger, builders, cachedUnits);
       } while (builders.size() > 0);
 
       for (CompilationUnit unit : resultUnits) {
@@ -435,6 +398,59 @@
       }
       return resultUnits;
     }
+
+    /**
+     * Removes cached units that fail validation with the current set of valid classes; also
+     * add the builder of the invalidated unit back for retry later.
+     */
+    private void removeInvalidCachedUnitsAndRescheduleCorrespondingBuilders(TreeLogger logger,
+        Collection<CompilationUnitBuilder> builders,
+        Map<CompilationUnitBuilder, CompilationUnit> cachedUnits) {
+
+      /*
+       * Invalidate any cached units with invalid refs.
+       */
+      Collection<CompilationUnit> invalidatedUnits = Lists.newArrayList();
+      for (Iterator<Entry<CompilationUnitBuilder, CompilationUnit>> it =
+          cachedUnits.entrySet().iterator(); it.hasNext();) {
+        Entry<CompilationUnitBuilder, CompilationUnit> entry = it.next();
+        CompilationUnit unit = entry.getValue();
+        boolean isValid = unit.getDependencies().validate(logger, allValidClasses);
+        if (isValid && unit.isError()) {
+          // See if the unit has classes that can't provide a
+          // NameEnvironmentAnswer
+          for (CompiledClass cc : unit.getCompiledClasses()) {
+            try {
+              cc.getNameEnvironmentAnswer();
+            } catch (ClassFormatException ex) {
+              isValid = false;
+              break;
+            }
+          }
+        }
+        if (!isValid) {
+          if (logger.isLoggable(TreeLogger.TRACE)) {
+            logger.log(TreeLogger.TRACE, "Invalid Unit: " + unit.getTypeName());
+          }
+          invalidatedUnits.add(unit);
+          builders.add(entry.getKey());
+          it.remove();
+        }
+      }
+
+      if (invalidatedUnits.size() > 0) {
+        if (logger.isLoggable(TreeLogger.TRACE)) {
+          logger.log(TreeLogger.TRACE, "Invalid units found: " + invalidatedUnits.size());
+        }
+      }
+
+      // Any units we invalidated must now be removed from the valid classes.
+      for (CompilationUnit unit : invalidatedUnits) {
+        for (CompiledClass cc : unit.getCompiledClasses()) {
+          allValidClasses.remove(cc.getSourceName());
+        }
+      }
+    }
   }
 
   private static final CompilationStateBuilder instance = new CompilationStateBuilder();