Improves Grapher's display of names:
 - Instances are now rendered with their #toString() if they're not the
   default Object implementation.
 - String instances are displayed as string literals.
 - ProviderMethods are rendered as a method signature.
 - Instance sources are now just rendered as the file name and line number.

git-svn-id: https://google-guice.googlecode.com/svn/trunk@829 d779f126-a31b-0410-b53b-1d3aecad763e
diff --git a/extensions/grapher/src/com/google/inject/grapher/NameFactory.java b/extensions/grapher/src/com/google/inject/grapher/NameFactory.java
index b03ee30..4cd7fa6 100644
--- a/extensions/grapher/src/com/google/inject/grapher/NameFactory.java
+++ b/extensions/grapher/src/com/google/inject/grapher/NameFactory.java
@@ -29,7 +29,7 @@
 public interface NameFactory {
   String getMemberName(Member member);
   String getClassName(Key<?> key);
-  String getClassName(Object instance);
+  String getInstanceName(Object instance);
   String getAnnotationName(Key<?> key);
   String getSourceName(Object source);
 }
diff --git a/extensions/grapher/src/com/google/inject/grapher/ShortNameFactory.java b/extensions/grapher/src/com/google/inject/grapher/ShortNameFactory.java
index e50a950..b145740 100644
--- a/extensions/grapher/src/com/google/inject/grapher/ShortNameFactory.java
+++ b/extensions/grapher/src/com/google/inject/grapher/ShortNameFactory.java
@@ -20,9 +20,14 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
+import java.util.List;
 
+import com.google.common.base.Join;
+import com.google.common.collect.Lists;
 import com.google.inject.Key;
 import com.google.inject.TypeLiteral;
+import com.google.inject.internal.ProviderMethod;
+import com.google.inject.internal.StackTraceElements;
 
 /**
  * Reasonable implementation for {@link NameFactory}. Mostly takes various
@@ -65,14 +70,59 @@
     return stripPackages(typeLiteral.toString());
   }
 
-  public String getClassName(Object instance) {
-    return stripPackages(instance.getClass().getName());
+  public String getInstanceName(Object instance) {
+    if (instance instanceof ProviderMethod) {
+      return getMethodString(((ProviderMethod<?>) instance).getMethod());
+    }
+
+    if (instance instanceof CharSequence) {
+      return "\"" + instance + "\"";
+    }
+
+    try {
+      if (instance.getClass().getMethod("toString").getDeclaringClass().equals(Object.class)) {
+        return stripPackages(instance.getClass().getName());
+      }
+    } catch (SecurityException e) {
+      throw new AssertionError(e);
+    } catch (NoSuchMethodException e) {
+      throw new AssertionError(e);
+    }
+
+    return instance.toString();
   }
-  
+
+  /**
+   * Returns a name for a Guice "source" object. This will typically be either
+   * a {@link StackTraceElement} for when the binding is made to the instance,
+   * or a {@link Method} when a provider method is used.
+   */
   public String getSourceName(Object source) {
+    if (source instanceof Method) {
+      source = StackTraceElements.forMember((Method) source);
+    }
+
+    if (source instanceof StackTraceElement) {
+      return getFileString((StackTraceElement) source);
+    }
+
     return stripPackages(source.toString());
   }
 
+  protected String getFileString(StackTraceElement stackTraceElement) {
+    return stackTraceElement.getFileName() + ":" + stackTraceElement.getLineNumber();
+  }
+
+  protected String getMethodString(Method method) {
+    List<String> paramStrings = Lists.newArrayList();
+    for (Class<?> paramType : method.getParameterTypes()) {
+      paramStrings.add(paramType.getSimpleName());
+    }
+
+    String paramString = Join.join(", ", paramStrings);
+    return "#" + method.getName() + "(" + paramString + ")";
+  }
+
   /**
    * Eliminates runs of lowercase characters and numbers separated by periods.
    * Seems to remove packages from fully-qualified type names pretty well.
diff --git a/extensions/grapher/src/com/google/inject/grapher/graphviz/ImplementationNodeFactory.java b/extensions/grapher/src/com/google/inject/grapher/graphviz/ImplementationNodeFactory.java
index 1a92138..5813f8f 100644
--- a/extensions/grapher/src/com/google/inject/grapher/graphviz/ImplementationNodeFactory.java
+++ b/extensions/grapher/src/com/google/inject/grapher/graphviz/ImplementationNodeFactory.java
@@ -77,7 +77,7 @@
     public void setInstance(Object instance) {
       node.setHeaderBackgroundColor("#aaaaaa");
       node.setHeaderTextColor("#ffffff");
-      node.setTitle(nameFactory.getClassName(instance));
+      node.setTitle(nameFactory.getInstanceName(instance));
     }
 
     public void setSource(Object source) {
diff --git a/extensions/grapher/test/com/google/inject/grapher/ShortNameFactoryTest.java b/extensions/grapher/test/com/google/inject/grapher/ShortNameFactoryTest.java
index 556a3c9..43fc641 100644
--- a/extensions/grapher/test/com/google/inject/grapher/ShortNameFactoryTest.java
+++ b/extensions/grapher/test/com/google/inject/grapher/ShortNameFactoryTest.java
@@ -18,11 +18,19 @@
 
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
+import com.google.inject.AbstractModule;
 import com.google.inject.BindingAnnotation;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
 import com.google.inject.Key;
 import com.google.inject.Provider;
+import com.google.inject.Provides;
 import com.google.inject.TypeLiteral;
+import com.google.inject.internal.ProviderMethod;
+import com.google.inject.internal.StackTraceElements;
 import com.google.inject.name.Names;
+import com.google.inject.spi.DefaultBindingTargetVisitor;
+import com.google.inject.spi.ProviderInstanceBinding;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -37,6 +45,26 @@
  * @author phopkins@gmail.com (Pete Hopkins)
  */
 public class ShortNameFactoryTest extends TestCase {
+  // Helper objects are up here because their line numbers are tested below.
+  private static class Obj {
+    @Annotated
+    public String field;
+    Obj() {}
+    void method(String parameter) {}
+  }
+
+  private static class ToStringObj {
+    @Override
+    public String toString() {
+      return "I'm a ToStringObj";
+    }
+  }
+  
+  @Retention(RUNTIME)
+  @Target({ ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD })
+  @BindingAnnotation
+  private @interface Annotated {}
+
   private NameFactory nameFactory;
 
   @Override
@@ -89,21 +117,62 @@
         "Provider<String>", nameFactory.getClassName(key));
   }
 
+  /**
+   * Tests the case where a provider method is the source of the 
+   * @throws Exception
+   */
   public void testGetSourceName_method() throws Exception {
     Member method = Obj.class.getDeclaredMethod("method", String.class);
-    assertEquals("Method name and parameters should not have packages",
-        "void ShortNameFactoryTest$Obj.method(String)", nameFactory.getSourceName(method));
-  }
-  
-  private static class Obj {
-    @Annotated
-    String field;
-    Obj() {}
-    void method(String parameter) {}
+    assertEquals("Method should be identified by its file name and line number",
+        "ShortNameFactoryTest.java:53", nameFactory.getSourceName(method));
   }
 
-  @Retention(RUNTIME)
-  @Target({ ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD })
-  @BindingAnnotation
-  private @interface Annotated {}
+  public void testGetSourceName_stackTraceElement() throws Exception {
+    StackTraceElement element = 
+        (StackTraceElement) StackTraceElements.forMember(Obj.class.getField("field"));
+    assertEquals("Stack trace element should be identified by its file name and line number",
+        "ShortNameFactoryTest.java:52", nameFactory.getSourceName(element));
+  }
+
+  public void testGetInstanceName_defaultToString() throws Exception {
+    assertEquals("Should use class name instead of Object#toString()",
+        "ShortNameFactoryTest$Obj", nameFactory.getInstanceName(new Obj()));
+  }
+  
+  public void testGetInstanceName_customToString() throws Exception {
+    assertEquals("Should use class's toString() method since it's defined",
+        "I'm a ToStringObj", nameFactory.getInstanceName(new ToStringObj()));
+  }
+  
+  public void testGetInstanceName_string() throws Exception {
+    assertEquals("String should have quotes to evoke a string literal",
+        "\"My String Instance\"", nameFactory.getInstanceName("My String Instance"));
+  }
+  
+  public void testGetInstanceName_providerMethod() throws Exception {
+    final ProviderMethod<?>[] methodHolder = new ProviderMethod[1];
+    
+    Injector injector = Guice.createInjector(new ProvidingModule());
+    injector.getBinding(Integer.class).acceptTargetVisitor(
+        new DefaultBindingTargetVisitor<Object, Void>() {
+          @SuppressWarnings("unchecked") @Override
+          public Void visitProviderInstance(ProviderInstanceBinding<?> binding) {
+            methodHolder[0] = (ProviderMethod) binding.getProviderInstance();
+            return null;
+          }
+    });
+    
+    assertEquals("Method provider should pretty print as the method signature",
+        "#provideInteger(String)", nameFactory.getInstanceName(methodHolder[0]));
+  }
+  
+  private static class ProvidingModule extends AbstractModule {
+    @Override
+    protected void configure() {}
+    
+    @Provides
+    public Integer provideInteger(String string) {
+      return Integer.valueOf(string);
+    }
+  }
 }