Skip to content

Commit

Permalink
Don't copy parameter annotations when creating a ParameterSpec. (#770)
Browse files Browse the repository at this point in the history
* Don't copy parameter annotations when creating a ParameterSpec.

This further preserves the behaviour discussed in #482.
Unifying it for both MethodSpec.overriding and ParameterSpec.get.

* Add compilation test for when overriding a method with private annotations.

* Address PR comments:

* Remove unused import
* Rename newly added test
  • Loading branch information
danysantiago committed Apr 20, 2020
1 parent 4462270 commit 622e07e
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 20 deletions.
12 changes: 1 addition & 11 deletions src/main/java/com/squareup/javapoet/MethodSpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.Element;
import javax.lang.model.element.ExecutableElement;
Expand Down Expand Up @@ -233,16 +232,7 @@ public static Builder overriding(ExecutableElement method) {
}

methodBuilder.returns(TypeName.get(method.getReturnType()));
// Copying parameter annotations from the overridden method can be incorrect so we're
// deliberately dropping them. See https://github.com/square/javapoet/issues/482.
methodBuilder.addParameters(ParameterSpec.parametersOf(method)
.stream()
.map(parameterSpec -> {
ParameterSpec.Builder builder = parameterSpec.toBuilder();
builder.annotations.clear();
return builder.build();
})
.collect(Collectors.toList()));
methodBuilder.addParameters(ParameterSpec.parametersOf(method));
methodBuilder.varargs(method.isVarArgs());

for (TypeMirror thrownType : method.getThrownTypes()) {
Expand Down
10 changes: 2 additions & 8 deletions src/main/java/com/squareup/javapoet/ParameterSpec.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.ExecutableElement;
Expand Down Expand Up @@ -87,17 +86,12 @@ void emit(CodeWriter codeWriter, boolean varargs) throws IOException {
public static ParameterSpec get(VariableElement element) {
checkArgument(element.getKind().equals(ElementKind.PARAMETER), "element is not a parameter");

// Copy over any annotations from element.
List<AnnotationSpec> annotations = element.getAnnotationMirrors()
.stream()
.map((mirror) -> AnnotationSpec.get(mirror))
.collect(Collectors.toList());

TypeName type = TypeName.get(element.asType());
String name = element.getSimpleName().toString();
// Copying parameter annotations can be incorrect so we're deliberately not including them.
// See https://github.com/square/javapoet/issues/482.
return ParameterSpec.builder(type, name)
.addModifiers(element.getModifiers())
.addAnnotations(annotations)
.build();
}

Expand Down
26 changes: 26 additions & 0 deletions src/test/java/com/squareup/javapoet/MethodSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.squareup.javapoet;

import com.google.testing.compile.Compilation;
import com.google.testing.compile.CompilationRule;
import java.io.Closeable;
import java.io.IOException;
Expand All @@ -30,14 +31,19 @@
import javax.lang.model.element.Modifier;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.util.ElementFilter;
import javax.lang.model.util.Elements;
import javax.lang.model.util.Types;
import javax.tools.JavaFileObject;

import com.google.testing.compile.CompilationSubject;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;

import static com.google.common.collect.Iterables.getOnlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.testing.compile.Compiler.javac;
import static com.squareup.javapoet.MethodSpec.CONSTRUCTOR;
import static com.squareup.javapoet.TestUtil.findFirst;
import static javax.lang.model.util.ElementFilter.methodsIn;
Expand Down Expand Up @@ -247,6 +253,26 @@ static void staticMethod() {
}
}

abstract static class AbstractClassWithPrivateAnnotation {

private @interface PrivateAnnotation{ }

abstract void foo(@PrivateAnnotation final String bar);
}

@Test public void overrideDoesNotCopyParameterAnnotations() {
TypeElement abstractTypeElement = getElement(AbstractClassWithPrivateAnnotation.class);
ExecutableElement fooElement = ElementFilter.methodsIn(abstractTypeElement.getEnclosedElements()).get(0);
ClassName implClassName = ClassName.get("com.squareup.javapoet", "Impl");
TypeSpec type = TypeSpec.classBuilder(implClassName)
.superclass(abstractTypeElement.asType())
.addMethod(MethodSpec.overriding(fooElement).build())
.build();
JavaFileObject jfo = JavaFile.builder(implClassName.packageName, type).build().toJavaFileObject();
Compilation compilation = javac().compile(jfo);
CompilationSubject.assertThat(compilation).succeeded();
}

@Test public void equalsAndHashCode() {
MethodSpec a = MethodSpec.constructorBuilder().build();
MethodSpec b = MethodSpec.constructorBuilder().build();
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/squareup/javapoet/ParameterSpecTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ public void foo(@Nullable final String bar) {
VariableElement parameterElement = element.getParameters().get(0);

assertThat(ParameterSpec.get(parameterElement).toString())
.isEqualTo("@javax.annotation.Nullable java.lang.String arg0");
.isEqualTo("java.lang.String arg0");
}

@Test public void addNonFinalModifier() {
Expand Down

0 comments on commit 622e07e

Please sign in to comment.