Comment 1 Jerome Lanneluc 2006-10-04 12:05:59 EDT

Sorry but there is nothing I can do without a test case.

Comment 2 Olivier Thomann 2006-10-04 12:56:29 EDT

It is a case where the bound cannot be resolved.

Comment 3 Olivier Thomann 2006-10-05 10:45:44 EDT

Closing as REMIND till I get it again.

Comment 4 Jerome Lanneluc 2007-03-09 07:19:44 EST

Closing since no test case was provided

Comment 5 Denis Roy 2009-08-30 02:05:16 EDT

As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.

Comment 6 Tom Hofmann 2010-05-06 03:29:16 EDT

Reopening as I just got this with 3.6M7.

Adding my stack trace to get updated line numbers:

java.lang.NullPointerException
	at org.eclipse.jdt.internal.compiler.lookup.WildcardBinding.computeUniqueKey(WildcardBinding.java:360)
	at org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding.computeUniqueKey(ParameterizedTypeBinding.java:284)
	at org.eclipse.jdt.internal.compiler.lookup.Binding.computeUniqueKey(Binding.java:60)
	at org.eclipse.jdt.internal.core.SourceType.resolved(SourceType.java:818)
	at org.eclipse.jdt.core.dom.TypeBinding.getJavaElement(TypeBinding.java:503)
	at org.eclipse.jdt.internal.ui.text.java.LazyGenericTypeProposal.computeTypeArgumentProposals(LazyGenericTypeProposal.java:313)
	at org.eclipse.jdt.internal.ui.text.java.LazyGenericTypeProposal.apply(LazyGenericTypeProposal.java:205)
	at org.eclipse.jdt.internal.ui.text.java.AbstractJavaCompletionProposal.apply(AbstractJavaCompletionProposal.java:317)
	at org.eclipse.jdt.internal.ui.text.java.AbstractJavaCompletionProposal.apply(AbstractJavaCompletionProposal.java:332)
	at org.eclipse.jdt.internal.ui.text.java.JavaTypeCompletionProposal.apply(JavaTypeCompletionProposal.java:111)
	at org.eclipse.jdt.internal.ui.text.java.AnonymousTypeCompletionProposal.apply(AnonymousTypeCompletionProposal.java:353)
	at org.eclipse.jdt.internal.ui.text.java.AbstractJavaCompletionProposal.apply(AbstractJavaCompletionProposal.java:482)
	at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertProposal(CompletionProposalPopup.java:928)

Comment 7 Tom Hofmann 2010-05-06 03:52:02 EDT

... and I have got steps!

- create the two files below (it is necessary that there are two separate CUs!)
- invoke content assist in Bug157847.java after "new PropListener"
-> boom

---------------- Prop.java ---------------
package bugs.bug157847;

abstract class Prop<T> {
	abstract void add(PropListener<? super T> listener);
}

interface PropListener<T> {}

------------------------------------------

--------------- Bug157847.java -----------
package bugs.bug157847;

import java.util.Set;

class Bug157847 {
	void method(Set<Prop<?>> set) {
		for (Prop<?> prop : set) {
			prop.add(new PropListener)
		}
	}
}

------------------------------------------

Comment 8 Srikanth Sankaran 2010-05-06 04:09:02 EDT

(In reply to comment #7)
> ... and I have got steps!
> 
> - create the two files below (it is necessary that there are two separate CUs!)
> - invoke content assist in Bug157847.java after "new PropListener"
> -> boom

I tried this and after a while did see the exception trace in the
log file. But I don't know how to reproduce this consistently.
Since that time I cleared the log contents, I haven't been able
to reproduce this any more. Do you get this consistently ?

Comment 9 Tom Hofmann 2010-05-06 04:10:39 EDT

Strange - yes I do get this every time.

You probably need to enable the Java > Editor > Content Assist > Guess argument names preference.

Comment 10 Ayushman Jain 2010-05-06 04:43:21 EDT

I can reproduce the problem in the step where we apply the proposal that is selected. ie. i can correctly see the proposal for anonymous type for PropListener, but when I click on it to apply it, it fails with an NPE.

At first glance, the problem seems to be cascading up from a UI code in LazyGenericTypeProposal.computeTypeArgumentProposals() where expectedTypeBinding is computed. Somehow, the WildCardBinding$bounds is still null at the line where we get the NPE.

Comment 11 Srikanth Sankaran 2010-05-06 06:22:28 EDT

(In reply to comment #10)
> I can reproduce the problem in the step where we apply the proposal that is
> selected. ie. i can correctly see the proposal for anonymous type for
> PropListener, but when I click on it to apply it, it fails with an NPE.
> 
> At first glance, the problem seems to be cascading up from a UI code in
> LazyGenericTypeProposal.computeTypeArgumentProposals() where
> expectedTypeBinding is computed. Somehow, the WildCardBinding$bounds is still
> null at the line where we get the NPE.

Problem arises since we end up with a bound kind indicating Wildcard.SUPER,
but with actual bounds set to null.
The key from which the binding is reconstructed looks like:
[Lbugs/bug157847/Prop~PropListener<Lbugs/bug157847/Prop~PropListener;{0}-!Lbugs/bug157847/Prop;{0}*159;>;]

Comment 12 Srikanth Sankaran 2010-05-06 22:29:54 EDT

> The key from which the binding is reconstructed looks like:
> [Lbugs/bug157847/Prop~PropListener<Lbugs/bug157847/Prop~PropListener;{0}-
                                                          ^^^^^^^^^^^^^^^^^

That part of the key looks odd. I'll investigate this further.

Comment 13 Srikanth Sankaran 2010-05-07 02:16:52 EDT

(In reply to comment #12)
> > The key from which the binding is reconstructed looks like:
> > [Lbugs/bug157847/Prop~PropListener<Lbugs/bug157847/Prop~PropListener;{0}-
>                                                           ^^^^^^^^^^^^^^^^^
> 
> That part of the key looks odd. I'll investigate this further.

Expected type PropListener<? super capture#1-of ?> encodes into the key
Lbugs/bug157847/Prop~PropListener<Lbugs/bug157847/Prop~PropListener;{0}-!Lbugs/bug157847/Prop;{0}*159;>;

So this part appears ok, will check for mismatch between encoder & decoder.

Comment 14 Srikanth Sankaran 2010-05-07 03:55:00 EDT

(In reply to comment #13)
> (In reply to comment #12)

> So this part appears ok, will check for mismatch between encoder & decoder.

The binding key for the wildcard capture encodes the source position
of the message send's receiver (Bug157847.java). In attempting to
reconstruct the bindings, we are operating on the AST for Prop.java
naturally failing to find suitable compiler bindings there. In any event
the wildcard capture makes sense only at the referring unit and
not at the declarating unit (unless they happen to be the same)

This also explains first bullet in comment# 7 i.e why things work
if they are in the same translation unit.

Comment 15 Srikanth Sankaran 2010-05-07 09:31:47 EDT

Created attachment 167470 [details]
Proposed patch

org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createWildcard(ReferenceBinding, int, TypeBinding, TypeBinding[], int) should not be creating
bogus wildcards of the form '? super <null>' or '? extends <null>'

The current patch ensures that we don't return such internally inconsistent
wild card types that come back to bite us in the form of an NPE later.
We will simply return null instead to signal failure.

Markus, where does that leave the UI ? Do you have a fall back plan
that is acceptable to you ?

Comment 16 Srikanth Sankaran 2010-05-07 09:34:27 EDT

Olivier, please review. 
Let me know if you want this for RC1.

Comment 17 Olivier Thomann 2010-05-07 10:25:10 EDT

I don't think we should return null in org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment.createWildcard(ReferenceBinding, int, TypeBinding, TypeBinding[], int).
If the binding cannot be decoded for the bound, I would prefer to get a binding that is unbound. Returning null for this method is unexpected for some of the callers.
Srikanth, let me know what you think.

Comment 18 Markus Keller 2010-05-07 11:37:02 EDT

(In reply to comment #15)
> Markus, where does that leave the UI ? Do you have a fall back plan
> that is acceptable to you ?

The patch returns null from a method that never returned null before. To assess the effects of this, one would have to look at the caller hierarchy of that method and check all callers to make sure they can handle it. I didn't do any investigations in callers.

It's hard for me to predict the effects in the UI, since I don't even know what APIs are affected by the change. I played a bit with the patch and didn't see any problems though.

But like Srikanth in comment 8, I also had trouble reproducing the issue sometimes. In those cases, the example from comment 7 doesn't insert type arguments.
=> The problem is in TypeProposalInfo#resolveMember(): The call to IJavaProject#findType(String) does not always find the type PropListener, since it is a secondary type. The type is only found if it's in its own CU or if the CU is open in an editor (and thus the type is found because types in working copies are always known). If the type is not found, you get no type arguments and the bug can not be reproduced.

Comment 19 Srikanth Sankaran 2010-05-10 04:12:10 EDT

Created attachment 167664 [details]
Revised patch

Earlier patch was trying to way more general
than needed to be. A more conservative fix
is employed in this patch.

Comment 20 Srikanth Sankaran 2010-05-10 04:22:55 EDT

(In reply to comment #17)

[...]

> Returning null for this method is unexpected for some of the
> callers.

[...]

(In reply to comment #18)

> The patch returns null from a method that never returned null before. To assess
> the effects of this, one would have to look at the caller hierarchy of that

Thanks Olivier & Markus for the comments.

The current patch addresses these concerns by limiting the fix
to the pertinent case at hand.

The current patch also results in similar behavior for the missing
capture binding case as the existing behavior if a binding key were
to refer to a non-existing (top level) type.

To illustrate this better I have added a new test org.eclipse.jdt.core.tests.dom.ASTModelBridgeTests.test157847a() which
will pass even without this patch. In this test we are looking for
the binding corresponding to "LBug157847A~ThisTypeDoesNotExist;" a
non existent type.

The new tests 
org.eclipse.jdt.core.tests.dom.ASTModelBridgeTests.test157847b() and
org.eclipse.jdt.core.tests.dom.ASTModelBridgeTests.test157847c()
demonstrate and test for the new behavior for the missing capture
binding cases.

Olivier, let me know what you think.

Comment 21 Olivier Thomann 2010-05-10 10:36:54 EDT

Created attachment 167718 [details]
Same patch with minor changes

Comment 22 Olivier Thomann 2010-05-10 10:38:05 EDT

I like this patch better than the previous one.
I made small changes.
I rewrote the conditional expression to use an if statement (I find this easier to read) and I updated the copyright on the test compilation unit.
Let me know what you think.

Comment 23 Olivier Thomann 2010-05-10 16:59:24 EDT

Targeting for 3.6RC1 if you accept my last change.

Comment 24 Srikanth Sankaran 2010-05-11 01:25:54 EDT

(In reply to comment #23)
> Targeting for 3.6RC1 if you accept my last change.

Looks good, Released in HEAD for 3.6RC1. 

(Don't know why the copyright update plugin skipped this file).

Comment 25 Jay Arthanareeswaran 2010-05-17 02:50:08 EDT

Verified for 3.6RC1 using build I20100513-1500.