Community
Participate
Working Groups
In bug 334281 we were getting 'Missing @since tag' on several newly-generified methods in org.eclipse.core.databinding.property, even though the generified classes are binary compatible with their previous non-generic variants. Digging into the situation, it appears that the IMethodDescriptor instances are created from the method generic signatures, rather than their normal non-parameterized signatures. For example, SimpleListProperty has a method doSetList: public abstract class SimpleListProperty<S, E> extends ListProperty<S, E> { public final void setList(S source, List<E> list, ListDiff<E> diff) {…} } The MemberDescriptorImpl for this class has the method listed with signature '(QS;QList;QListDiff;)V'. But the method being resolved from the corresponding .class file has signature '(Ljava/lang/Object;Ljava/util/List;Lorg/eclipse/core/databinding/observable/list/ListDiff;)V' and so they are not matching. These MethodDescriptors are generated within TagScanner$Visitor#scanMethodJavaDoc(MethodDeclaration) which uses the Signatures#getMethodSignatureFromNode(MethodDeclaration) to craft a method signature. The heavy lifting is performed by Signatures#getParametersTypeNames(List<SingleVariableDefinition>), which isn't producing a binary signature as would be found in a .class file.
Method lookups in .class files must be done using the type-erased method signature. JLS (JavaSE 8) §4.6 Type Erasure says: "The erasure of a constructor or method signature _s_ is a signature consisting of the same name as _s_ and the erasures of all the formal parameter types given in _s_." It lists the erasures of a type T, denoted |T|, as: - the erasure of parameterized type G<T1,…> is |G| - the erasure of nested type T.C is |T|.C - the erasure of array type T[] is |T|[] - the erasure of a type variable is the erasure of its leftmost bound - the erasure of every other type is the type itself
The patch modifies Signatures.getMethodSignatureFromNode() to optionally return a type-erased signature for an AST MethodDeclaration. The callers from the source-scanning TagScanner requests type erasure. ApiDescriptionProcessor does not, but I have not been able to figure out how/when this class is actually called.
New Gerrit change created: https://git.eclipse.org/r/68853
With this patch, and reverting commit a49c16f6 from platform.ui, I see no more API errors. Making modifications to the public methods on SimpleListProperty properly show API errors. All API Tools Plugin tests pass locally (well, except for the testOldStylePlugin test).
1) It works well for platform.ui plugin 2) All tests passes Minor issue--> import org.eclipse.pde.api.tools.util.tests.ApiBaselineManagerTests.SourceChangeVisitor; in ApiBaselineManagerTests is unused.
The changes looks good. However, 1) the test seems to check Tests default methods annotate API descriptions with @noreference * and @nooverride How about a test for signature. There is a test here https://git.eclipse.org/r/#/c/44715/ 2) getTypeSignature(Type type, boolean erased) At every place, true is used for boolean erased, can we do away with this extra parameter?
Can I have 1 single gerrit patch?
Bug 334281 and this bug are related but different - Bug 334281 is a problem resolving a JDT IMethod given a .class method. - This bug is about resolving a .class method given a JDT method. Each binary method now has two signatures: the classic erased binary signature, and a generic signature. You can see the two signatures with "javap -p -v java.util.Set": public interface java.util.Set<E extends java.lang.Object> extends java.util.Collection<E> Constant pool: [. . .] #24 = Utf8 (Ljava/util/Collection;)Z #25 = Utf8 (Ljava/util/Collection<*>;)Z [. . .] public abstract boolean removeAll(java.util.Collection<?>); descriptor: (Ljava/util/Collection;)Z flags: ACC_PUBLIC, ACC_ABSTRACT Signature: #25 // (Ljava/util/Collection<*>;)Z > Tests default methods annotate API descriptions with @noreference > * and @nooverride > > How about a test for signature. There is a test here > > https://git.eclipse.org/r/#/c/44715/ The testGenericMethodWithBounds is badly described. This particular issue is really to verify that the method signature for the generic method is properly resolved using the type-erased method signature, as is recorded by the API tooling and in the .class file. public class TestGenericMethod1<E, C extends Collection>{ public int m1(E object); public int m2(C collection) {} } The .class file signature for these methods use Object and Collection as the erased type parameters, producing "(QObject;)I" and "(QCollection;)I" respectively. But API Tooling was embedding the generic types E and C, "(QE;)I" and "(QC;)I" respectively — which is a valid generic signature. The actual resolution calls into the Signatures class (viz ApiBaselineManagerTests.java and ApiDescriptionProcessorTests.java). The @noreference and @nooveride is superfluous for this test; I'll remove them and fix the description. > 2) getTypeSignature(Type type, boolean erased) > > At every place, true is used for boolean erased, can we do away > with this extra parameter? I had originally omitted the parameter, but added it back in just in case. As this method was public and its javadocs didn't indicate that it was intended to be limited to creating erased signatures, and it could be useful to have the generic signature, I thought it best to introduce the parameter. I thought one of the other callers might have wanted the generic signature.
https://git.eclipse.org/r/68853 looks good. I had removed unused imports and changed license year.
Once this is fixed http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a49c16f6fe864442b9ca58cd951515af414ffe17. can be reverted
Gerrit change https://git.eclipse.org/r/68853 was merged to [master]. Commit: http://git.eclipse.org/c/pde/eclipse.pde.ui.git/commit/?id=08e8b9a3759b7aeb3fd512a8f22ae5b1715b1bdd
Thanks Brian ! Now all the api filters similar to these http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a49c16f6fe864442b9ca58cd951515af414ffe17. can be removed.
I see few of java.lang.ClassCastException: org.eclipse.jdt.core.dom.ParameterizedType cannot be cast to org.eclipse.jdt.core.dom.SimpleType at org.eclipse.pde.api.tools.internal.util.Signatures.getTypeSignature(Signatures.java:628) at org.eclipse.pde.api.tools.internal.util.Signatures.getParametersTypeNames(Signatures.java:527) in my log. May be related to this.
opened and fixed bug 491068.
Oops! Thanks for the quick fix.
verified on Version: Neon (4.6) Build id: N20160419-2000 This file no longer required org.eclipse.core.databinding.property.settings/.api_filters
