fix: cleaning of method signature computation

src/main/java/spoon/reflect/declaration/CtExecutable.java

@@ -115,7 +115,17 @@ public interface CtExecutable<R> extends CtNamedElement, CtTypedElement<R>, CtBo
  boolean removeThrownType(CtTypeReference<? extends Throwable> throwType);
 
  /**
- * Gets the signature of this method or constructor as specified by chapter "8.4.2 Method Signature" of the Java specification
+ * Gets the signature of this method or constructor.
+ * The signature is composed of the method name and the parameter types, all fully-qualified, eg "int foo(java.lang.String)".
+ * The core contract is that in a type, there cannot be two methods with the same signature.
+ *
+ * Note that the concept of method signature in Java is not well defined (see chapter "8.4.2 Method Signature" of the Java specification, which defines what relations between signatures but not what a signature is exactly).
+ *
+ * Note also that the signature of a method reference is the same as the signature of the corresponding method if and only if the method parameters does not involve generics in their types. Otherwise, one has eg m(String) (reference) and m(T) (declaration)
+ *
+ * Reference: "In the Java programming language, a method signature is the method name and the number and type of its parameters. Return types and thrown exceptions are not considered to be a part of the method signature."
+ * see https://stackoverflow.com/questions/16149285/does-a-methods-signature-in-java-include-its-return-type
+ * see https://en.wikipedia.org/wiki/Type_signature
  */
  String getSignature();
 

src/main/java/spoon/reflect/reference/CtExecutableReference.java

@@ -147,7 +147,7 @@ public interface CtExecutableReference<T> extends CtReference, CtActualTypeConta
  boolean isFinal();
 
  /**
- * Gets the signature of this method or constructor as specified by chapter "8.4.2 Method Signature" of the Java specification
+ * Gets the signature of this method or constructor, as explained in {@link spoon.reflect.declaration.CtMethod#getSignature()}.
  */
  String getSignature();
 

src/main/java/spoon/reflect/visitor/ImportScannerImpl.java

@@ -448,13 +448,20 @@ protected boolean addMethodImport(CtExecutableReference ref) {
  protected boolean isImportedInMethodImports(CtExecutableReference<?> ref) {
  if (!(ref.isImplicit()) && methodImports.containsKey(ref.getSimpleName())) {
  CtExecutableReference<?> exist = methodImports.get(ref.getSimpleName());
- if (exist.getSignature().equals(ref.getSignature())) {
+ if (getSignature(exist).equals(
+ getSignature(ref))
+ ) {
  return true;
  }
  }
  return false;
  }
 
+ private String getSignature(CtExecutableReference<?> exist) {
+ return (exist.getDeclaringType() != null ? exist.getDeclaringType().getQualifiedName() : "")
+ + "." + exist.getSignature();
+ }
+
  protected boolean addFieldImport(CtFieldReference ref) {
  // static import is not supported below java 1.5
  if (ref.getFactory().getEnvironment().getComplianceLevel() < 5) {

src/main/java/spoon/support/template/SubstitutionVisitor.java

@@ -53,6 +53,7 @@
 import spoon.reflect.reference.CtTypeReference;
 import spoon.reflect.visitor.CtInheritanceScanner;
 import spoon.reflect.visitor.CtScanner;
+import spoon.support.visitor.SignaturePrinter;
 import spoon.template.AbstractTemplate;
 import spoon.template.Parameter;
 import spoon.template.Template;
@@ -617,11 +618,11 @@ private String getParameterValueAsString(Object parameterValue) {
  } else if (parameterValue instanceof Class) {
  return ((Class) parameterValue).getSimpleName();
  } else if (parameterValue instanceof CtInvocation) {
- return getShortSignature(((CtInvocation<?>) parameterValue).getExecutable().getSignature());
+ return getShortSignatureForJavadoc(((CtInvocation<?>) parameterValue).getExecutable());
  } else if (parameterValue instanceof CtExecutableReference) {
- return getShortSignature(((CtExecutableReference<?>) parameterValue).getSignature());
+ return getShortSignatureForJavadoc((CtExecutableReference<?>) parameterValue);
  } else if (parameterValue instanceof CtExecutable) {
- return getShortSignature(((CtExecutable<?>) parameterValue).getSignature());
+ return getShortSignatureForJavadoc(((CtExecutable<?>) parameterValue).getReference());
  } else if (parameterValue instanceof CtLiteral) {
  Object val = ((CtLiteral<Object>) parameterValue).getValue();
  return val == null ? null : val.toString();
@@ -630,10 +631,12 @@ private String getParameterValueAsString(Object parameterValue) {
  }
 
  /*
- * cut the package name. We always convert types to simple names here
+ * return the typical Javadoc style link Foo#method(). The class name is not fully qualified.
  */
- private static String getShortSignature(String fullSignature) {
- return fullSignature.substring(fullSignature.lastIndexOf('.') + 1);
+ private static String getShortSignatureForJavadoc(CtExecutableReference<?> ref) {
+ SignaturePrinter sp = new SignaturePrinter();
+ sp.writeNameAndParameters(ref);
+ return ref.getDeclaringType().getSimpleName() + CtExecutable.EXECUTABLE_SEPARATOR + sp.getSignature();
  }
 
  /**

src/main/java/spoon/support/util/SignatureBasedSortedSet.java

@@ -16,12 +16,12 @@
  */
 package spoon.support.util;
 
-import java.util.Collection;
-import java.util.TreeSet;
-
 import spoon.reflect.declaration.CtExecutable;
 import spoon.support.comparator.SignatureComparator;
 
+import java.util.Collection;
+import java.util.TreeSet;
+
 /** maintains unicity with method signatures */
 public class SignatureBasedSortedSet<E extends CtExecutable<?>> extends TreeSet<E> {
 

src/main/java/spoon/support/visitor/SignaturePrinter.java

@@ -18,10 +18,8 @@
 
 import spoon.reflect.declaration.CtAnnotationMethod;
 import spoon.reflect.declaration.CtConstructor;
-import spoon.reflect.declaration.CtExecutable;
 import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.declaration.CtParameter;
-import spoon.reflect.declaration.CtTypeParameter;
 import spoon.reflect.reference.CtArrayTypeReference;
 import spoon.reflect.reference.CtExecutableReference;
 import spoon.reflect.reference.CtIntersectionTypeReference;
@@ -36,7 +34,9 @@
  */
 public class SignaturePrinter extends CtScanner {
 
- private final StringBuffer signature = new StringBuffer();
+ private final StringBuilder signature = new StringBuilder();
+
+ public SignaturePrinter() { }
 
  public String getSignature() {
  return signature.toString();
@@ -50,11 +50,12 @@ public <T> void visitCtArrayTypeReference(CtArrayTypeReference<T> reference) {
 
  @Override
  public <T> void visitCtExecutableReference(CtExecutableReference<T> reference) {
- scan(reference.getDeclaringType());
+ writeNameAndParameters(reference);
+ }
 
-  write(CtExecutable.EXECUTABLE_SEPARATOR);
+ public <T> void writeNameAndParameters(CtExecutableReference<T> reference) {
  if (reference.isConstructor()) {
- write(reference.getDeclaringType().getSimpleName());
+ write(reference.getDeclaringType().getQualifiedName());
  } else {
  write(reference.getSimpleName());
  }
@@ -66,11 +67,10 @@ public <T> void visitCtExecutableReference(CtExecutableReference<T> reference) {
  } else {
  write(CtExecutableReference.UNKNOWN_TYPE);
  }
- write(", ");
+ write(",");
  }
  if (reference.getParameters().size() > 0) {
  clearLast(); // ","
- clearLast(); // space
  }
  }
  write(")");
@@ -98,10 +98,9 @@ public void visitCtTypeParameterReference(CtTypeParameterReference ref) {
  public <T> void visitCtIntersectionTypeReference(CtIntersectionTypeReference<T> reference) {
  for (CtTypeReference<?> bound : reference.getBounds()) {
  scan(bound);
- write(", ");
+ write(",");
  }
  clearLast();
- clearLast();
  }
 
  @Override
@@ -133,22 +132,6 @@ public <T> void visitCtAnnotationMethod(CtAnnotationMethod<T> annotationMethod)
  */
  @Override
  public <T> void visitCtMethod(CtMethod<T> m) {
- if (!m.getFormalCtTypeParameters().isEmpty()) {
- write("<");
- for (CtTypeParameter typeParameter : m.getFormalCtTypeParameters()) {
- scan(typeParameter.getReference());
- write(",");
- }
- if (m.getFormalCtTypeParameters().size() > 0) {
- clearLast();
- }
- write("> ");
- }
- // the return type is required, see example in SimilarSignatureMethodes in test code (name and arguments are identical)
- if (m.getType() != null) {
- write(m.getType().getQualifiedName());
- }
- write(" ");
  write(m.getSimpleName());
  write("(");
  for (CtParameter<?> p : m.getParameters()) {

src/test/java/spoon/test/api/NoClasspathTest.java

@@ -120,7 +120,7 @@ public void testBug20141021() {
  pr.scan(ref);
  String s = pr.getSignature();
 
- assertEquals("#foo()", s);
+ assertEquals("foo()", s);
  }
 
  @Test

src/test/java/spoon/test/comment/CommentTest.java

@@ -686,7 +686,9 @@ public void testSnippedWithComments(){
  assertTrue(clazz1==builder.getSnippetCompilationUnit().getDeclaredTypes().get(0));
 
  CtMethod<?> methodString = (CtMethod<?>) clazz1.getMethods().toArray()[0];
- assertEquals("java.io.File foo(java.lang.String)", methodString.getSignature());
+ // we don't call getSignature in order to encapsulate a little bit the changes
+ // for the next time we will change the signature :-)
+ assertEquals("foo", methodString.getSimpleName());
  assertEquals(1, methodString.getComments().size());
  assertEquals("method javadoc comment", methodString.getComments().get(0).getContent());
 

src/test/java/spoon/test/imports/ImportTest.java

@@ -238,7 +238,7 @@ public void testAnotherMissingImport() throws Exception {
  CtInvocation invocation = (CtInvocation) invocationStatement;
  CtExecutableReference executableReference = invocation.getExecutable();
 
- assertEquals("fr.inria.AnotherMissingImport#doSomething(externallib.SomeType)", executableReference.getSignature());
+ assertEquals("doSomething(externallib.SomeType)", executableReference.getSignature());
  assertSame(methods.get(0), executableReference.getDeclaration());
  }
 

src/test/java/spoon/test/main/MainTest.java

@@ -12,9 +12,11 @@
 import spoon.reflect.code.CtFieldWrite;
 import spoon.reflect.code.CtLambda;
 import spoon.reflect.code.CtVariableWrite;
+import spoon.reflect.declaration.CtConstructor;
 import spoon.reflect.declaration.CtElement;
 import spoon.reflect.declaration.CtExecutable;
 import spoon.reflect.declaration.CtField;
+import spoon.reflect.declaration.CtMethod;
 import spoon.reflect.declaration.CtPackage;
 import spoon.reflect.declaration.CtShadowable;
 import spoon.reflect.declaration.CtType;
@@ -161,9 +163,9 @@ public <T> void visitCtTypeReference(CtTypeReference<T> reference) {
 
  @Override
  public <T> void visitCtExecutableReference(CtExecutableReference<T> reference) {
+ super.visitCtExecutableReference(reference);
  assertNotNull(reference);
  if (isLanguageExecutable(reference)) {
- super.visitCtExecutableReference(reference);
  return;
  }
  final CtExecutable<T> executableDeclaration = reference.getExecutableDeclaration();
@@ -173,23 +175,38 @@ public <T> void visitCtExecutableReference(CtExecutableReference<T> reference) {
  // when a generic type is used in a parameter and return type, the shadow type doesn't have these information.
  for (int i = 0; i < reference.getParameters().size(); i++) {
  if (reference.getParameters().get(i) instanceof CtTypeParameterReference) {
- continue;
+ return;
  }
  if (reference.getParameters().get(i) instanceof CtArrayTypeReference && ((CtArrayTypeReference) reference.getParameters().get(i)).getComponentType() instanceof CtTypeParameterReference) {
- continue;
+ return;
  }
  //TODO assertions which are checking lambdas. Till then ignore lambdas.
  if (executableDeclaration instanceof CtLambda) {
- continue;
+ return;
  }
  assertEquals(reference.getParameters().get(i).getQualifiedName(), executableDeclaration.getParameters().get(i).getType().getQualifiedName());
  }
 
+ // contract: the reference and method signature are the same
+ if (reference.getActualTypeArguments().size() == 0
+ && executableDeclaration instanceof CtMethod
+ && ((CtMethod)executableDeclaration).getFormalCtTypeParameters().size() != 0
+ ) {
+ assertEquals(reference.getSignature(), executableDeclaration.getSignature());
+ }
+
+ // contract: the reference and constructor signature are the same
+ if (reference.getActualTypeArguments().size() == 0
+ && executableDeclaration instanceof CtConstructor
+ && ((CtConstructor)executableDeclaration).getFormalCtTypeParameters().size() != 0
+ ) {
+ assertEquals(reference.getSignature(), executableDeclaration.getSignature());
+ }
+
  if (reference.getDeclaration() == null && CtShadowable.class.isAssignableFrom(executableDeclaration.getClass())) {
  assertTrue(((CtShadowable) executableDeclaration).isShadow());
  }
 
- super.visitCtExecutableReference(reference);
  }
 
  private <T> boolean isLanguageExecutable(CtExecutableReference<T> reference) {

src/test/java/spoon/test/reference/ExecutableReferenceGenericTest.java

@@ -119,7 +119,7 @@ public void testMultiReferenceBetweenMethodsWithGenericInSameClass() throws Exce
 
  //T has more information in the invocation than its declaration because of the argument type
  //assertEquals(expectedMethod1, refsMethod2.get(0).getDeclaration());
- assertEquals(execRefsMethods2.getSignature(), "<T extends java.lang.String> void method1(T extends java.lang.String)");
+ assertEquals(execRefsMethods2.getSignature(), "method1(T extends java.lang.String)");
  assertEquals(expectedMethod1, refsMethod2.get(1).getDeclaration());
  assertEquals(expectedMethod5, refsMethod2.get(2).getDeclaration());
  }
@@ -172,7 +172,7 @@ public void testOneReferenceWithGenericMethodOutOfTheClass() throws Exception {
  CtExecutable execRefsMethods2 = refsMethodA.get(0).getDeclaration();
  //T has more information in the invocation than its declaration because of the argument type
  // assertEquals(expectedMethod1, refsMethodA.get(0).getDeclaration());
- assertEquals(execRefsMethods2.getSignature(), "<T extends java.lang.String> void method1(T extends java.lang.String)");
+ assertEquals(execRefsMethods2.getSignature(), "method1(T extends java.lang.String)");
  }
 
  @Test
@@ -247,15 +247,15 @@ public boolean matches(CtExecutableReference<?> reference) {
  assertEquals(11, refsExecutableClass1.size());
  for (CtExecutableReference<?> ref : refsExecutableClass1) {
  assertNotNull(ref);
- if (!ref.toString().equals("java.lang.Object#Object()")) {
+ if (!ref.toString().equals("java.lang.Object()")) {
  assertNotNull(ref.getDeclaration());
  }
  }
 
  assertEquals(9, refsExecutableClass2.size());
  for (CtExecutableReference<?> ref : refsExecutableClass2) {
  assertNotNull(ref);
- if (!ref.toString().equals("java.lang.Object#Object()")) {
+ if (!ref.toString().equals("java.lang.Object()")) {
  assertNotNull(ref.getDeclaration());
  }
  }

src/test/java/spoon/test/reference/ExecutableReferenceTest.java

@@ -54,7 +54,7 @@ public boolean matches(CtInvocation<?> element) {
  assertNotNull(executableZeroParameter.getDeclaringType());
  assertNull(executableZeroParameter.getType());
  assertEquals(0, executableZeroParameter.getParameters().size());
- assertEquals("Bar#m()", executableZeroParameter.toString());
+ assertEquals("m()", executableZeroParameter.toString());
  assertEquals("new Bar().m()", invocations.get(0).toString());
 
  // Executable reference with 1 parameter and return type.
@@ -63,7 +63,7 @@ public boolean matches(CtInvocation<?> element) {
  assertNotNull(executableOneParameter.getType());
  assertEquals(1, executableOneParameter.getParameters().size());
  assertNotEquals(executableZeroParameter, executableOneParameter);
- assertEquals("Bar#m(int)", executableOneParameter.toString());
+ assertEquals("m(int)", executableOneParameter.toString());
  assertEquals("bar.m(1)", invocations.get(1).toString());
 
  // Executable reference with 2 parameters.
@@ -73,7 +73,7 @@ public boolean matches(CtInvocation<?> element) {
  assertEquals(2, executableTwoParameters.getParameters().size());
  assertNotEquals(executableTwoParameters, executableZeroParameter);
  assertNotEquals(executableTwoParameters, executableOneParameter);
- assertEquals("Bar#m(int, java.lang.String)", executableTwoParameters.toString());
+ assertEquals("m(int,java.lang.String)", executableTwoParameters.toString());
  assertEquals("new Bar().m(1, \"5\")", invocations.get(2).toString());
 
  // Static Executable reference.
@@ -83,7 +83,7 @@ public boolean matches(CtInvocation<?> element) {
  assertEquals(1, staticExecutable.getParameters().size());
  assertNotEquals(staticExecutable, executableZeroParameter);
  assertNotEquals(staticExecutable, executableOneParameter);
- assertEquals("Bar#m(java.lang.String)", staticExecutable.toString());
+ assertEquals("m(java.lang.String)", staticExecutable.toString());
  assertEquals("Bar.m(\"42\")", invocations.get(3).toString());
  }
 

src/test/java/spoon/test/replace/ReplaceTest.java

@@ -393,13 +393,13 @@ public void testReplaceExecutableReferenceByAnotherOne() throws Exception {
  final CtExecutableReference<Object> newExecutable = factory.Executable().createReference("void java.io.PrintStream#print(java.lang.String)");
 
  assertSame(oldExecutable, inv.getExecutable());
- assertEquals("java.io.PrintStream#println(java.lang.String)", inv.getExecutable().toString());
 
  oldExecutable.replace(newExecutable);
 
  assertSame(newExecutable, inv.getExecutable());
- assertEquals("java.io.PrintStream#print(java.lang.String)", inv.getExecutable().toString());
-
+ assertEquals("print(java.lang.String)", inv.getExecutable().toString());
+ assertEquals("java.io.PrintStream", inv.getExecutable().getDeclaringType().toString());
+
  //contract: replace of single value by multiple values in single value field must fail
  try {
  newExecutable.replace(Arrays.asList(oldExecutable, null));

src/test/java/spoon/test/secondaryclasses/ClassesTest.java

@@ -93,7 +93,7 @@ public void testAnonymousClass() throws Exception {
 
  assertNotNull(y.getType().getDeclaration());
 
- assertEquals("spoon.test.secondaryclasses.AnonymousClass$2#2()", y.getExecutable().toString());
+ assertEquals("spoon.test.secondaryclasses.AnonymousClass$2()", y.getExecutable().toString());
 
  assertEquals(type.getFactory().Type().createReference(I.class), y.getAnonymousClass().getSuperInterfaces().toArray(new CtTypeReference[0])[0]);