Skip to content

Commit

Permalink
Fix conflicts with types with same name as type being declared (squar…
Browse files Browse the repository at this point in the history
…e#559)

* Add failing tests for types conflicting with type being declared

Also removes previous (flawed) test, which should have
been in JavaFileTest anyway, and add a separate test for
the "superclass references self" case (already handled
in TypeSpecTest#interfaceExtends btw, but this makes it
clearer what is tested and expected).
And add a separate test for classes in the same package
conflicting with a superclass, for completeness.

Reproduces square#470

* Fix conflicts with types with same name as type being declared

Those types (annotations, or referenced in annotation values,
in type variable bounds, superclass, or superinterfaces)
were erroneously imported rather than being emitted with their
qualified name.

Fixes square#470
  • Loading branch information
tbroyer authored and swankjesse committed May 13, 2017
1 parent 40dfa0c commit 4cd975b
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 20 deletions.
2 changes: 1 addition & 1 deletion checkstyle.xml
Expand Up @@ -68,7 +68,7 @@
<property name="max" value="100"/>
</module>
<module name="MethodLength">
<property name="max" value="155"/>
<property name="max" value="160"/>
</module>
<module name="ParameterNumber"/>

Expand Down
29 changes: 29 additions & 0 deletions src/main/java/com/squareup/javapoet/TypeSpec.java
Expand Up @@ -82,6 +82,30 @@ private TypeSpec(Builder builder) {
this.originatingElements = Util.immutableList(originatingElementsMutable);
}

/**
* Creates a dummy type spec for type-resolution only (in CodeWriter)
* while emitting the type declaration but before entering the type body.
*/
private TypeSpec(TypeSpec type) {
assert type.anonymousTypeArguments == null;
this.kind = type.kind;
this.name = type.name;
this.anonymousTypeArguments = null;
this.javadoc = type.javadoc;
this.annotations = Collections.emptyList();
this.modifiers = Collections.emptySet();
this.typeVariables = Collections.emptyList();
this.superclass = null;
this.superinterfaces = Collections.emptyList();
this.enumConstants = Collections.emptyMap();
this.fieldSpecs = Collections.emptyList();
this.staticBlock = type.staticBlock;
this.initializerBlock = type.initializerBlock;
this.methodSpecs = Collections.emptyList();
this.typeSpecs = Collections.emptyList();
this.originatingElements = Collections.emptyList();
}

public boolean hasModifier(Modifier modifier) {
return modifiers.contains(modifier);
}
Expand Down Expand Up @@ -172,6 +196,9 @@ void emit(CodeWriter codeWriter, String enumName, Set<Modifier> implicitModifier
codeWriter.emit(anonymousTypeArguments);
codeWriter.emit(") {\n");
} else {
// Push an empty type (specifically without nested types) for type-resolution.
codeWriter.pushType(new TypeSpec(this));

codeWriter.emitJavadoc(javadoc);
codeWriter.emitAnnotations(annotations, false);
codeWriter.emitModifiers(modifiers, Util.union(implicitModifiers, kind.asMemberModifiers));
Expand Down Expand Up @@ -214,6 +241,8 @@ void emit(CodeWriter codeWriter, String enumName, Set<Modifier> implicitModifier
}
}

codeWriter.popType();

codeWriter.emit(" {\n");
}

Expand Down
94 changes: 94 additions & 0 deletions src/test/java/com/squareup/javapoet/JavaFileTest.java
Expand Up @@ -465,6 +465,84 @@ private TypeSpec importStaticTypeSpec(String name) {
+ "}\n");
}

@Test public void classAndSuperclassShareName() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.superclass(ClassName.get("com.taco.bell", "Taco"))
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "class Taco extends com.taco.bell.Taco {\n"
+ "}\n");
}

@Test public void conflictingAnnotation() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.addAnnotation(ClassName.get("com.taco.bell", "Taco"))
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "@com.taco.bell.Taco\n"
+ "class Taco {\n"
+ "}\n");
}

@Test public void conflictingAnnotationReferencedClass() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.addAnnotation(AnnotationSpec.builder(ClassName.get("com.squareup.tacos", "MyAnno"))
.addMember("value", "$T.class", ClassName.get("com.taco.bell", "Taco"))
.build())
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "@MyAnno(com.taco.bell.Taco.class)\n"
+ "class Taco {\n"
+ "}\n");
}

@Test public void conflictingTypeVariableBound() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.addTypeVariable(
TypeVariableName.get("T", ClassName.get("com.taco.bell", "Taco")))
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "class Taco<T extends com.taco.bell.Taco> {\n"
+ "}\n");
}

@Test public void superclassReferencesSelf() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.superclass(ParameterizedTypeName.get(
ClassName.get(Comparable.class), ClassName.get("com.squareup.tacos", "Taco")))
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "import java.lang.Comparable;\n"
+ "\n"
+ "class Taco extends Comparable<Taco> {\n"
+ "}\n");
}

/** https://github.com/square/javapoet/issues/366 */
@Test public void annotationIsNestedClass() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
Expand Down Expand Up @@ -573,4 +651,20 @@ private TypeSpec importStaticTypeSpec(String name) {
+ " }\n"
+ "}\n");
}

@Test public void packageClassConflictsWithSuperlass() throws Exception {
String source = JavaFile.builder("com.squareup.tacos",
TypeSpec.classBuilder("Taco")
.superclass(ClassName.get("com.taco.bell", "A"))
.addField(ClassName.get("com.squareup.tacos", "A"), "a")
.build())
.build()
.toString();
assertThat(source).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "class Taco extends com.taco.bell.A {\n"
+ " A a;\n"
+ "}\n");
}
}
19 changes: 0 additions & 19 deletions src/test/java/com/squareup/javapoet/TypeSpecTest.java
Expand Up @@ -625,25 +625,6 @@ private boolean isJava8() {
+ "}\n");
}

@Test public void classImplementsExtendsSameName() throws Exception {
ClassName javapoetTaco = ClassName.get(tacosPackage, "Taco");
ClassName tacoBellTaco = ClassName.get("com.taco.bell", "Taco");
ClassName fishTaco = ClassName.get("org.fish.taco", "Taco");
TypeSpec typeSpec = TypeSpec.classBuilder("Taco")
.superclass(fishTaco)
.addSuperinterface(ParameterizedTypeName.get(ClassName.get(Comparable.class), javapoetTaco))
.addSuperinterface(tacoBellTaco)
.build();
assertThat(toString(typeSpec)).isEqualTo(""
+ "package com.squareup.tacos;\n"
+ "\n"
+ "import java.lang.Comparable;\n"
+ "\n"
+ "class Taco extends org.fish.taco.Taco "
+ "implements Comparable<Taco>, com.taco.bell.Taco {\n"
+ "}\n");
}

@Test public void classImplementsNestedClass() throws Exception {
ClassName outer = ClassName.get(tacosPackage, "Outer");
ClassName inner = outer.nestedClass("Inner");
Expand Down

0 comments on commit 4cd975b

Please sign in to comment.