Community
Participate
Working Groups
I20060919-0010 Matching method references are now tagged "polymorphic" if the invocation is on a receiver whose declared class is a super- or a subclass of the class declaring the method. See bug 73401 for discussions and the scenarios where that behavior makes sense. As discussed in bug 73401 comment 17, I think that subclass matches are of "higher quality" than superclass matches, and that superclass matches should therefore be flagged as potential. Consider the following CU. A search on B.m() finds the three matches, one direct (on b) and two polymorphic (on a and c) matches in User.method(). While the matches on b and c are guaranteed to be the searched method or an overloaded version thereof, the match on a is not: it may well be an A instance that thas nothing to do with class B. With other words: the receivers of the calls to b.m() and c.m() can be safely cast to a B, while the receiver of a.m() cannot. I would therefore expect the latter to be a potential match. ------------ A.java ------------- package polysearch; class A { public void m() {} } class B extends A { @Override public void m() {} } class C extends B { } class User { void method() { C c= new C(); B b= c; A a= c; a.m(); b.m(); c.m(); } } ---------------------------------
I would distinguish a.m() as a polymorphic match (or whatever you call it), whereas b.m() and c.m() are equivalent since they target the exact same method (statically).
(In reply to comment #1) b.m() and c.m() may be equivalent from a compile-time point of view, but semantically, they are different calls that would bind differently if C would override m(). Let's postulate a C2 which overrides m(): class C2 extends B { @Override public void m() {} } ... and add a call: C2 c2= new C2(); c2.m(); Would you tag c2.m() as equivalent to b.m() too? Or as polymorphic like a.m()? If c2.m() and a.m() are both polymorphic, then they look exactly the same, despite the difference mentioned in comment 0, paragraph 3 (read "overriding" where the comment 0 says "overloaded").
You could demonstrate the same with introducing a B2, which could in turn override #m(). c2.m() would be considered polymorphic to me, where c.m() not, since unless overriden, it is invoking the exact same method (which is the same as any other reference to a B variable, which could in turn invoke a B2#m()). Along these line, I suspect interface methods need to be considered differently, since you would miss all interesting subtype scenarii.
(In reply to comment #0) > > As discussed in bug 73401 comment 17, I think that subclass matches are of > "higher quality" than superclass matches, I think we all agree on this point. > and that superclass matches should therefore be flagged as potential. > But not on this one. Potential matches are reserved when Search Engine is definitely unable to say that a match with correct name and qualification is valid or not. This flag is typically set when there's no binding; unknown reference or external reference in a class file. IMO, a possible approach would be to consider different flags in all cases we want to distinguish (and need brain storming for names)... Currently, I see 3+1 different cases for referenced method: 1) call to super type method: a.m() 2) call to type method but from a sub-type: c.m() 3) call to a sub-type method which overrides the searched method: c2.m() 4) 1)+3) (if I add "class D2 extends C2 {}" and "d2.m();" to the test case)??? BTW, I still do not know why this is interesting to distinguish the sub-types methods from exact matches? From bug 73401 and bug 156491 only case 1) is annoying, we didn't any complain about 2), 3) and of course 4) ones...
(In reply to comment #3) > > Along these line, I suspect interface methods need to be considered > differently, since you would miss all interesting subtype scenarii. > Indeed, if we consider following test case: class A { public void m() {} } class B extends A { public void m() {} } class C extends B implements I { } interface I { void m(); } public class Test { void method() { I i = new C(); i.m(); } } Method reference i.m() will be flagged as potential although it should be accurate! So, even for interface, potential flag is not a good solution and should be also reverted. We may consider this case as the fifth one - or fourth if 1)+3) does not really make sense...
(In reply to comment #4) > Potential matches are reserved when Search Engine is > definitely unable to say that a match with correct name and qualification is > valid or not. Ok, I misinterpreted the 'potential' flag, then. I do not require that specific flag, all I wanted is a way to distinguish the two cases. > BTW, I still do not know why this is interesting to distinguish the sub-types > methods from exact matches? I am all for only flagging super-type polymorphic matches. Rationale: IMO the use cases for this flag (certainly bug 73401) focus on the possible runtime type X of the receiver r of a method call, and whether X is assignment compatible to the type Y declaring the method we are searching for. In the sub-type case, this is certain, while in the super-type case, this may or may not be true. Hence, the super-type case can produce false-positives, possibly many.
I agree with Tom, the flag seems to be necessary only for super types. But, in addition, I would say that we need to distinguish specific test case for interface (comment 5). I also think we need to find another name for this flag which sounds now not really appropriate as polymorphic methods may be both on super-types and sub-types... I have no precise idea about it but something like SUPER_TYPE_METHOD and POTENTIAL_INTERFACE_METHOD (if we distinguish interface test cases...). Markus, I definitely need your point of view on this before finalizing the implementation. As this functionality is already in 3.3 M2, I think we need to finalize it to have correct behavior in an integration build as soon as possible => increase priority to P2
I agree that case 1) of comment 4 is the only interesting case if we don't consider interfaces. Proposed flag for case 1): /** * Returns whether the reference is on a method that is overridden by the * search target or not. If <code>true</code>, the method called at run-time * may or may not be the search target, depending on the run-time type * of the receiver object. * * @return <code>true</code> if the reference is on a method that is * overridden by the search target, <code>false </code> otherwise */ public boolean isOverridden() Since we don't have any requests for the potential interface matches (references to methods with same signature in interfaces that are not supertypes or subtypes of the target method), I think it would be best to remove them again and just have the isOverridden() property for now. People that are interested in potential references to the interface method can always search for references to that method instead.
Created attachment 51564 [details] Proposed patch Replace isPolymorphic with isOverridden, thanks Markus for the well documented API proposal :-)
Created attachment 51565 [details] Proposed patch for JDT/UI To test this I've also create a patch for JDT/UI... Hope this help :-)
Released for 3.3 M3 in HEAD stream. Note that I've put back MethodReferenceMatch.isPolymorphic() method otherwise nightly build will have compilation error. I've set as deprecated and will remove it only when JDT/UI will remove all references to it... No specific test case added, existing ones already shows the change of behavior...
Released improvement for API name + additional test in JavaSearchBugsTest Common agreement was done on following API for MethodReferenceMatch: /** * Returns whether the reference is on a message sent from a type * which is a super type of the searched method declaring type. * If <code>true</code>, the method called at run-time may or may not be * the search target, depending on the run-time type of the receiver object. * * @return <code>true</code> if the reference is on a message sent from * a super-type of the searched method declaring class, <code>false </code> otherwise */ public boolean isSuperInvocation()
Verified for 3.3 M3 using warm-up build I20061030-0010
