From 0f1ac539a439ff8f6346deba68177a61b7f3210a Mon Sep 17 00:00:00 2001
From: Pierre-Yves Ricau <py@squareup.com>
Date: Tue, 31 Dec 2013 18:19:50 +0100
Subject: [PATCH] Adding error for fake complete=false module

It does not make sense to use "complete = false" on a module if there is no missing binding. This can happen after refactorings, or copy pasting. While you can't beat manual review, it also helps to have the compiler let you know that you have it wrong.

I've used this branch to chase our "fake incomplete" modules. Turns out we had more then a few, so I think it might be worth making that a feature.

This commit is not done yet: I can't build, I have mysterious "java.io package not allowed" errors and also apparently I need to upgrade Maven for Android to build. I'll try to fix that done or steal Jake's laptop.

I should also add some tests.

In the meantime, let me know if that sounds like the right way to do it. I wonder if it should live in GraphAnalysisProcessor or not.
---
 .../codegen/GraphAnalysisProcessor.java       | 25 +++++++++++++++----
 .../NonCompleteModuleErrorHandler.java        | 19 ++++++++++++++
 2 files changed, 39 insertions(+), 5 deletions(-)
 create mode 100644 compiler/src/main/java/dagger/internal/codegen/NonCompleteModuleErrorHandler.java

diff --git a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisProcessor.java b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisProcessor.java
index 8be0c964580..652d430c4ca 100644
--- a/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisProcessor.java
+++ b/compiler/src/main/java/dagger/internal/codegen/GraphAnalysisProcessor.java
@@ -56,6 +56,7 @@
 
 import static dagger.Provides.Type.SET;
 import static dagger.Provides.Type.SET_VALUES;
+import static dagger.internal.Linker.ErrorHandler.NULL;
 import static dagger.internal.codegen.Util.getAnnotation;
 import static dagger.internal.codegen.Util.getPackage;
 import static dagger.internal.codegen.Util.isInterface;
@@ -115,7 +116,9 @@ public final class GraphAnalysisProcessor extends AbstractProcessor {
       if (annotation.get("complete").equals(Boolean.TRUE)) {
         Map<String, Binding<?>> bindings;
         try {
-          bindings = processCompleteModule(moduleType, false);
+          Linker.ErrorHandler errorHandler = new GraphAnalysisErrorHandler(processingEnv,
+              moduleType.getQualifiedName().toString());
+          bindings = processCompleteModule(moduleType, errorHandler);
           new ProblemDetector().detectCircularDependencies(bindings.values());
         } catch (ModuleValidationException e) {
           error("Graph validation failed: " + e.getMessage(), e.source);
@@ -140,10 +143,24 @@ public final class GraphAnalysisProcessor extends AbstractProcessor {
               .printMessage(Diagnostic.Kind.WARNING,
                   "Graph visualization failed. Please report this as a bug.\n\n" + sw, moduleType);
         }
+      } else {
+        NonCompleteModuleErrorHandler errorHandler = new NonCompleteModuleErrorHandler();
+        try {
+          processCompleteModule(moduleType, errorHandler);
+        } catch (RuntimeException e) {
+          // Any error here is fine, we're not supposed to process real incomplete modules.
+          continue;
+        }
+        // A module with complete = false is expected to have graph validation errors.
+        // If there is none, this module should be declared complete.
+        if (errorHandler.shouldBeComplete()) {
+          error("A module with no missing binding not have complete = false", element);
+        }
       }
 
       if (annotation.get("library").equals(Boolean.FALSE)) {
-        Map<String, Binding<?>> bindings = processCompleteModule(moduleType, true);
+        Linker.ErrorHandler errorHandler = NULL;
+        Map<String, Binding<?>> bindings = processCompleteModule(moduleType, errorHandler);
         try {
           new ProblemDetector().detectUnusedBinding(bindings.values());
         } catch (IllegalStateException e) {
@@ -159,14 +176,12 @@ private void error(String message, Element element) {
   }
 
   private Map<String, Binding<?>> processCompleteModule(TypeElement rootModule,
-      boolean ignoreCompletenessErrors) {
+      Linker.ErrorHandler errorHandler) {
     Map<String, TypeElement> allModules = new LinkedHashMap<String, TypeElement>();
     collectIncludesRecursively(rootModule, allModules, new LinkedList<String>());
     ArrayList<GraphAnalysisStaticInjection> staticInjections =
         new ArrayList<GraphAnalysisStaticInjection>();
 
-    Linker.ErrorHandler errorHandler = ignoreCompletenessErrors ? Linker.ErrorHandler.NULL
-        : new GraphAnalysisErrorHandler(processingEnv, rootModule.getQualifiedName().toString());
     Linker linker = new Linker(null, new GraphAnalysisLoader(processingEnv), errorHandler);
     // Linker requires synchronization for calls to requestBinding and linkAll.
     // We know statically that we're single threaded, but we synchronize anyway
diff --git a/compiler/src/main/java/dagger/internal/codegen/NonCompleteModuleErrorHandler.java b/compiler/src/main/java/dagger/internal/codegen/NonCompleteModuleErrorHandler.java
new file mode 100644
index 00000000000..6700ac5bb7e
--- /dev/null
+++ b/compiler/src/main/java/dagger/internal/codegen/NonCompleteModuleErrorHandler.java
@@ -0,0 +1,19 @@
+package dagger.internal.codegen;
+
+import dagger.internal.Linker;
+import java.util.List;
+
+public class NonCompleteModuleErrorHandler implements Linker.ErrorHandler {
+
+  private boolean shouldBeComplete = true;
+
+  @Override public void handleErrors(List<String> errors) {
+    if (errors.size() > 0) {
+      shouldBeComplete = false;
+    }
+  }
+
+  public boolean shouldBeComplete() {
+    return shouldBeComplete;
+  }
+}