Qualify types masked by type variables (square#657)

* Use fully qualified names if a type variable masks a type name, even if it is in the same package * Add a makeshift multiset to handle square#657
monperrus · Aug 22, 2018 · b879b58 · b879b58
1 parent dfb5bc6
commit b879b58
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 1 deletion.
diff --git a/src/main/java/com/squareup/javapoet/CodeWriter.java b/src/main/java/com/squareup/javapoet/CodeWriter.java
@@ -57,6 +57,7 @@ final class CodeWriter {
   private final Map<String, ClassName> importedTypes;
   private final Map<String, ClassName> importableTypes = new LinkedHashMap<>();
   private final Set<String> referencedNames = new LinkedHashSet<>();
+  private final Multiset<String> currentTypeVariables = new Multiset<>();
   private boolean trailingNewline;
 
   /**
@@ -187,6 +188,8 @@ public void emitModifiers(Set<Modifier> modifiers) throws IOException {
   public void emitTypeVariables(List<TypeVariableName> typeVariables) throws IOException {
     if (typeVariables.isEmpty()) return;
 
+    typeVariables.forEach(typeVariable -> currentTypeVariables.add(typeVariable.name));
+
     emit("<");
     boolean firstTypeVariable = true;
     for (TypeVariableName typeVariable : typeVariables) {
@@ -203,6 +206,10 @@ public void emitTypeVariables(List<TypeVariableName> typeVariables) throws IOExc
     emit(">");
   }
 
+  public void popTypeVariables(List<TypeVariableName> typeVariables) throws IOException {
+    typeVariables.forEach(typeVariable -> currentTypeVariables.remove(typeVariable.name));
+  }
+
   public CodeWriter emit(String s) throws IOException {
     return emitAndIndent(s);
   }
@@ -353,6 +360,12 @@ private void emitLiteral(Object o) throws IOException {
    * names visible due to inheritance.
    */
   String lookupName(ClassName className) {
+    // If the top level simple name is masked by a current type variable, use the canonical name.
+    String topLevelSimpleName = className.topLevelClassName().simpleName();
+    if (currentTypeVariables.contains(topLevelSimpleName)) {
+      return className.canonicalName;
+    }
+
     // Find the shortest suffix of className that resolves to className. This uses both local type
     // names (so `Entry` in `Map` refers to `Map.Entry`). Also uses imports.
     boolean nameResolved = false;
@@ -374,7 +387,7 @@ String lookupName(ClassName className) {
 
     // If the class is in the same package, we're done.
     if (Objects.equals(packageName, className.packageName())) {
-      referencedNames.add(className.topLevelClassName().simpleName());
+      referencedNames.add(topLevelSimpleName);
       return join(".", className.simpleNames());
     }
 
@@ -494,4 +507,26 @@ Map<String, ClassName> suggestedImports() {
     result.keySet().removeAll(referencedNames);
     return result;
   }
+
+  // A makeshift multi-set implementation
+  private static final class Multiset<T> {
+    private final Map<T, Integer> map = new LinkedHashMap<>();
+
+    void add(T t) {
+      int count = map.getOrDefault(t, 0);
+      map.put(t, count + 1);
+    }
+
+    void remove(T t) {
+      int count = map.getOrDefault(t, 0);
+      if (count == 0) {
+        throw new IllegalStateException(t + " is not in the multiset");
+      }
+      map.put(t, count - 1);
+    }
+
+    boolean contains(T t) {
+      return map.getOrDefault(t, 0) > 0;
+    }
+  }
 }
diff --git a/src/main/java/com/squareup/javapoet/MethodSpec.java b/src/main/java/com/squareup/javapoet/MethodSpec.java
@@ -137,6 +137,7 @@ void emit(CodeWriter codeWriter, String enclosingName, Set<Modifier> implicitMod
 
       codeWriter.emit("}\n");
     }
+    codeWriter.popTypeVariables(typeVariables);
   }
 
   public boolean hasModifier(Modifier modifier) {

diff --git a/src/main/java/com/squareup/javapoet/TypeSpec.java b/src/main/java/com/squareup/javapoet/TypeSpec.java
@@ -316,6 +316,7 @@ void emit(CodeWriter codeWriter, String enumName, Set<Modifier> implicitModifier
 
       codeWriter.unindent();
       codeWriter.popType();
+      codeWriter.popTypeVariables(typeVariables);
 
       codeWriter.emit("}");
       if (enumName == null && anonymousTypeArguments == null) {

diff --git a/src/test/java/com/squareup/javapoet/TypeSpecTest.java b/src/test/java/com/squareup/javapoet/TypeSpecTest.java
@@ -985,6 +985,74 @@ public void interfaceDefaultMethods() throws Exception {
         + "}\n");
   }
 
+  @Test public void simpleNameConflictsWithTypeVariable() {
+    ClassName inPackage = ClassName.get("com.squareup.tacos", "InPackage");
+    ClassName otherType = ClassName.get("com.other", "OtherType");
+    ClassName methodInPackage = ClassName.get("com.squareup.tacos", "MethodInPackage");
+    ClassName methodOtherType = ClassName.get("com.other", "MethodOtherType");
+    TypeSpec gen = TypeSpec.classBuilder("Gen")
+        .addTypeVariable(TypeVariableName.get("InPackage"))
+        .addTypeVariable(TypeVariableName.get("OtherType"))
+        .addField(FieldSpec.builder(inPackage, "inPackage").build())
+        .addField(FieldSpec.builder(otherType, "otherType").build())
+        .addMethod(MethodSpec.methodBuilder("withTypeVariables")
+            .addTypeVariable(TypeVariableName.get("MethodInPackage"))
+            .addTypeVariable(TypeVariableName.get("MethodOtherType"))
+            .addStatement("$T inPackage = null", methodInPackage)
+            .addStatement("$T otherType = null", methodOtherType)
+            .build())
+        .addMethod(MethodSpec.methodBuilder("withoutTypeVariables")
+            .addStatement("$T inPackage = null", methodInPackage)
+            .addStatement("$T otherType = null", methodOtherType)
+            .build())
+        .addMethod(MethodSpec.methodBuilder("againWithTypeVariables")
+            .addTypeVariable(TypeVariableName.get("MethodInPackage"))
+            .addTypeVariable(TypeVariableName.get("MethodOtherType"))
+            .addStatement("$T inPackage = null", methodInPackage)
+            .addStatement("$T otherType = null", methodOtherType)
+            .build())
+        // https://github.com/square/javapoet/pull/657#discussion_r205514292
+        .addMethod(MethodSpec.methodBuilder("masksEnclosingTypeVariable")
+            .addTypeVariable(TypeVariableName.get("InPackage"))
+            .build())
+        .addMethod(MethodSpec.methodBuilder("hasSimpleNameThatWasPreviouslyMasked")
+            .addStatement("$T inPackage = null", inPackage)
+            .build())
+        .build();
+    assertThat(toString(gen)).isEqualTo(""
+        + "package com.squareup.tacos;\n"
+        + "\n"
+        + "import com.other.MethodOtherType;\n"
+        + "\n"
+        + "class Gen<InPackage, OtherType> {\n"
+        + "  com.squareup.tacos.InPackage inPackage;\n"
+        + "\n"
+        + "  com.other.OtherType otherType;\n"
+        + "\n"
+        + "  <MethodInPackage, MethodOtherType> void withTypeVariables() {\n"
+        + "    com.squareup.tacos.MethodInPackage inPackage = null;\n"
+        + "    com.other.MethodOtherType otherType = null;\n"
+        + "  }\n"
+        + "\n"
+        + "  void withoutTypeVariables() {\n"
+        + "    MethodInPackage inPackage = null;\n"
+        + "    MethodOtherType otherType = null;\n"
+        + "  }\n"
+        + "\n"
+        + "  <MethodInPackage, MethodOtherType> void againWithTypeVariables() {\n"
+        + "    com.squareup.tacos.MethodInPackage inPackage = null;\n"
+        + "    com.other.MethodOtherType otherType = null;\n"
+        + "  }\n"
+        + "\n"
+        + "  <InPackage> void masksEnclosingTypeVariable() {\n"
+        + "  }\n"
+        + "\n"
+        + "  void hasSimpleNameThatWasPreviouslyMasked() {\n"
+        + "    com.squareup.tacos.InPackage inPackage = null;\n"
+        + "  }\n"
+        + "}\n");
+  }
+
   @Test public void originatingElementsIncludesThoseOfNestedTypes() {
     Element outerElement = Mockito.mock(Element.class);
     Element innerElement = Mockito.mock(Element.class);