Adding better error msgs to configuration checks

toothpick-runtime/src/main/java/toothpick/Configuration.java

@@ -1,7 +1,9 @@
 package toothpick;
 
 import java.lang.annotation.Annotation;
-import java.util.Stack;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.Set;
 import toothpick.config.Binding;
 
 import static java.lang.String.format;
@@ -14,7 +16,7 @@ public abstract class Configuration {
 
   abstract void checkCyclesStart(Class clazz);
 
-  abstract void checkCyclesEnd();
+  abstract void checkCyclesEnd(Class clazz);
 
   static {
     //default mode is production
@@ -23,10 +25,12 @@ public abstract class Configuration {
 
   public static Configuration development() {
     return new Configuration() {
-      private ThreadLocal<Stack<Class>> cycleDetectionStack = new ThreadLocal<Stack<Class>>() {
+      // We need a LIFO structure here, but stack is thread safe and we use thread local,
+      // so this property is overkill and LinkedHashSet is faster on retrieval.
+      private ThreadLocal<LinkedHashSet<Class>> cycleDetectionStack = new ThreadLocal<LinkedHashSet<Class>>() {
         @Override
-        protected Stack<Class> initialValue() {
-          return new Stack<>();
+        protected LinkedHashSet<Class> initialValue() {
+          return new LinkedHashSet<>();
         }
       };
 
@@ -49,24 +53,23 @@ void checkIllegalBinding(Binding binding) {
 
         for (Annotation annotation : clazz.getAnnotations()) {
           if (annotation.annotationType().isAnnotationPresent(javax.inject.Scope.class)) {
-            throw new IllegalBindingException();
+            throw new IllegalBindingException(format("Class %s cannot be bound. It has an scope annotation", clazz.getName()));
           }
         }
       }
 
       @Override
       void checkCyclesStart(Class clazz) {
         if (cycleDetectionStack.get().contains(clazz)) {
-          //TODO make the message better
-          throw new CyclicDependencyException(format("Class %s creates a cycle", clazz.getName()));
+          throw new CyclicDependencyException(new ArrayList<>(cycleDetectionStack.get()), clazz);
         }
 
-        cycleDetectionStack.get().push(clazz);
+        cycleDetectionStack.get().add(clazz);
       }
 
       @Override
-      void checkCyclesEnd() {
-        cycleDetectionStack.get().pop();
+      void checkCyclesEnd(Class clazz) {
+        cycleDetectionStack.get().remove(clazz);
       }
     };
   }
@@ -84,7 +87,7 @@ void checkCyclesStart(Class clazz) {
       }
 
       @Override
-      void checkCyclesEnd() {
+      void checkCyclesEnd(Class clazz) {
         //do nothing
       }
     };

toothpick-runtime/src/main/java/toothpick/CyclicDependencyException.java

@@ -1,6 +1,11 @@
 package toothpick;
 
+import java.util.List;
+
 public class CyclicDependencyException extends RuntimeException {
+
+  private static final int MARGIN_SIZE = 3;
+
   public CyclicDependencyException() {
   }
 
@@ -19,4 +24,68 @@ public CyclicDependencyException(Throwable cause) {
   public CyclicDependencyException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
     super(message, cause, enableSuppression, writableStackTrace);
   }
+
+  public CyclicDependencyException(List<Class> path, Class startClass) {
+    this(String.format("Class %s creates a cycle:\n%s", startClass.getName(), format(path, startClass)));
+  }
+
+  private static String format(List<Class> path, Class startClass) {
+    if (path.size() == 0) {
+      throw new IllegalArgumentException();
+    }
+
+    int classPosition = Math.max(path.indexOf(startClass), 0);
+    path = path.subList(classPosition, path.size());
+
+    int maxWordLength = findLongestClassNameLength(path);
+    int middleWordPos = maxWordLength / 2 + MARGIN_SIZE;
+    int loopLinePosition = maxWordLength + 2 * MARGIN_SIZE;
+    StringBuilder builder = new StringBuilder();
+
+    addTopLines(builder, middleWordPos, loopLinePosition);
+    for (Class clazz : path) {
+      addLine(builder, clazz.getName(), middleWordPos, loopLinePosition);
+      addLine(builder, "||", middleWordPos, loopLinePosition);
+    }
+    addHorizontalLine(builder, middleWordPos, loopLinePosition);
+
+    return builder.toString();
+  }
+
+  private static void addTopLines(StringBuilder builder, int middleWordPos, int loopLinePosition) {
+    builder.append("\n");
+    addHorizontalLine(builder, middleWordPos, loopLinePosition);
+    addLine(builder, "||", middleWordPos, loopLinePosition);
+    addLine(builder, "\\/", middleWordPos, loopLinePosition);
+  }
+
+  private static void addHorizontalLine(StringBuilder builder, int middleWordPos, int loopLinePosition) {
+    builder.append(repeat(' ', middleWordPos));
+    builder.append(repeat('=', loopLinePosition - middleWordPos + 1));
+    builder.append("\n");
+  }
+
+  private static void addLine(StringBuilder builder, String content, int middleWordPos, int loopLinePosition) {
+    int leftMarginSize = middleWordPos - content.length() / 2;
+    int rightMarginSize = loopLinePosition - leftMarginSize - content.length();
+    builder.append(repeat(' ', leftMarginSize));
+    builder.append(content);
+    builder.append(repeat(' ', rightMarginSize));
+    builder.append("||\n");
+  }
+
+  private static int findLongestClassNameLength(List<Class> path) {
+    int length, max = 0;
+    for (Class clazz : path) {
+      length = clazz.getName().length();
+      if (length > max) {
+        max = length;
+      }
+    }
+    return max;
+  }
+
+  private static String repeat(char c, int n) {
+    return new String(new char[n]).replace('\0', c);
+  }
 }

toothpick-runtime/src/main/java/toothpick/ScopeImpl.java

@@ -41,15 +41,15 @@ public ScopeImpl(Object name) {
   public <T> T getInstance(Class<T> clazz) {
     Configuration.instance.checkCyclesStart(clazz);
     T t = lookupProvider(clazz, null).get(this);
-    Configuration.instance.checkCyclesEnd();
+    Configuration.instance.checkCyclesEnd(clazz);
     return t;
   }
 
   @Override
   public <T> T getInstance(Class<T> clazz, String name) {
     Configuration.instance.checkCyclesStart(clazz);
     T t = lookupProvider(clazz, name).get(this);
-    Configuration.instance.checkCyclesEnd();
+    Configuration.instance.checkCyclesEnd(clazz);
     return t;
   }
 

toothpick-runtime/src/test/java/toothpick/CyclicDependencyExceptionTest.java

@@ -0,0 +1,192 @@
+package toothpick;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Test;
+import toothpick.data.Bar;
+import toothpick.data.BarChild;
+import toothpick.data.CyclicFoo;
+import toothpick.data.Foo;
+import toothpick.data.FooProvider;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.fail;
+
+public class CyclicDependencyExceptionTest {
+
+  @Test
+  public void newCyclicDependencyException_showGenerateWholePath_whenCycleStartsPath() throws Exception {
+    String expectedErrorMessage =
+        "Class toothpick.data.CyclicFoo creates a cycle:\n"
+            + "\n"
+            + "               ================\n"
+            + "              ||              ||\n"
+            + "              \\/              ||\n"
+            + "   toothpick.data.CyclicFoo   ||\n"
+            + "              ||              ||\n"
+            + "      toothpick.data.Bar      ||\n"
+            + "              ||              ||\n"
+            + "      toothpick.data.Foo      ||\n"
+            + "              ||              ||\n"
+            + "    toothpick.data.BarChild   ||\n"
+            + "              ||              ||\n"
+            + "               ================\n";
+
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(CyclicFoo.class);
+    path.add(Bar.class);
+    path.add(Foo.class);
+    path.add(BarChild.class);
+
+    //WHEN
+    CyclicDependencyException exception = new CyclicDependencyException(path, CyclicFoo.class);
+
+    //THEN
+    assertThat(exception.getMessage(), is(expectedErrorMessage));
+  }
+
+  @Test
+  public void newCyclicDependencyException_showGeneratePartialPath_whenCycleStartsInTheMiddle() throws Exception {
+    String expectedErrorMessage =
+        "Class toothpick.data.CyclicFoo creates a cycle:\n"
+            + "\n"
+            + "               ================\n"
+            + "              ||              ||\n"
+            + "              \\/              ||\n"
+            + "   toothpick.data.CyclicFoo   ||\n"
+            + "              ||              ||\n"
+            + "      toothpick.data.Foo      ||\n"
+            + "              ||              ||\n"
+            + "    toothpick.data.BarChild   ||\n"
+            + "              ||              ||\n"
+            + "               ================\n";
+
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(Bar.class);
+    path.add(CyclicFoo.class);
+    path.add(Foo.class);
+    path.add(BarChild.class);
+
+    //WHEN
+    CyclicDependencyException exception = new CyclicDependencyException(path, CyclicFoo.class);
+
+    //THEN
+    assertThat(exception.getMessage(), is(expectedErrorMessage));
+  }
+
+  @Test
+  public void newCyclicDependencyException_showGenerateLastElementPath_whenCycleFinishesPath() throws Exception {
+    String expectedErrorMessage =
+        "Class toothpick.data.CyclicFoo creates a cycle:\n"
+            + "\n"
+            + "               ================\n"
+            + "              ||              ||\n"
+            + "              \\/              ||\n"
+            + "   toothpick.data.CyclicFoo   ||\n"
+            + "              ||              ||\n"
+            + "               ================\n";
+
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(Bar.class);
+    path.add(Foo.class);
+    path.add(BarChild.class);
+    path.add(CyclicFoo.class);
+
+    //WHEN
+    CyclicDependencyException exception = new CyclicDependencyException(path, CyclicFoo.class);
+
+    //THEN
+    assertThat(exception.getMessage(), is(expectedErrorMessage));
+  }
+
+  @Test
+  public void newCyclicDependencyException_showGenerateWholePath_whenStartCannotBeFound() throws Exception {
+    String expectedErrorMessage =
+        "Class toothpick.data.FooProvider creates a cycle:\n"
+            + "\n"
+            + "               ================\n"
+            + "              ||              ||\n"
+            + "              \\/              ||\n"
+            + "   toothpick.data.CyclicFoo   ||\n"
+            + "              ||              ||\n"
+            + "      toothpick.data.Bar      ||\n"
+            + "              ||              ||\n"
+            + "      toothpick.data.Foo      ||\n"
+            + "              ||              ||\n"
+            + "    toothpick.data.BarChild   ||\n"
+            + "              ||              ||\n"
+            + "               ================\n";
+
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(CyclicFoo.class);
+    path.add(Bar.class);
+    path.add(Foo.class);
+    path.add(BarChild.class);
+
+    //WHEN
+    CyclicDependencyException exception = new CyclicDependencyException(path, FooProvider.class);
+
+    //THEN
+    assertThat(exception.getMessage(), is(expectedErrorMessage));
+  }
+
+  @Test
+  public void newCyclicDependencyException_showGenerateOneElementPath_whenCycleContainsOneElement() throws Exception {
+    String expectedErrorMessage =
+        "Class toothpick.data.CyclicFoo creates a cycle:\n"
+            + "\n"
+            + "               ================\n"
+            + "              ||              ||\n"
+            + "              \\/              ||\n"
+            + "   toothpick.data.CyclicFoo   ||\n"
+            + "              ||              ||\n"
+            + "               ================\n";
+
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(CyclicFoo.class);
+
+    //WHEN
+    CyclicDependencyException exception = new CyclicDependencyException(path, CyclicFoo.class);
+
+    //THEN
+    assertThat(exception.getMessage(), is(expectedErrorMessage));
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void newCyclicDependencyException_showThrowNullPointerException_whenPathIsNull() throws Exception {
+    //GIVEN, WHEN
+    new CyclicDependencyException(null, CyclicFoo.class);
+
+    //THEN
+    fail("Should throw an exception as a wrong parameters are passed");
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void newCyclicDependencyException_showThrowIllegalArgumentException_whenPathIsEmpty() throws Exception {
+    //GIVEN, WHEN
+    new CyclicDependencyException(new ArrayList<Class>(), CyclicFoo.class);
+
+    //THEN
+    fail("Should throw an exception as a wrong parameters are passed");
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void newCyclicDependencyException_showThrowNullPointerException_whenStartClassIsNull() throws Exception {
+    //GIVEN
+    List<Class> path = new ArrayList<>();
+    path.add(CyclicFoo.class);
+
+    //WHEN
+    new CyclicDependencyException(path, null);
+
+    //THEN
+    fail("Should throw an exception as a wrong parameters are passed");
+  }
+}
+