Bug 346454

Comment 1 Ayushman Jain 2011-05-19 11:43:27 EDT

Note also that currenly we don't give any proposals after
Test<String> t = new Test<String>[CTRL-SPC]
even on HEAD. Maybe we can propose a constructor here.

Comment 2 Satyam Kandula 2011-05-25 07:40:09 EDT

Note that CompletionOnQualifiedAllocationExpression.resolveType() doesn't call super.resolveType() but does some other things and hence special processing for diamond cases may not be happening. Watch out for these and other parser related changes that should probably happen. 
Look at the patch for bug 343693 for probable changes required.

Comment 3 Ayushman Jain 2011-05-26 14:14:59 EDT

I don't see the NegativeSizeArrayEx anymore. It got fixed with bug 343637. Although, we may want to look at what we want to propose when content assist is invoked after new Test<String> or after new Test<> for both HEAD and BETA_JAVA7.

Comment 4 Ayushman Jain 2011-05-26 14:28:22 EDT

On more testing, however, it turns out the exception now appears on the following test case (note the dot after <>)

public class Test <T>{
    static class T2<Z>{
    }
    public void foo() {
        Test<String> x = new Test<>.
    }
}

Comment 5 Ayushman Jain 2011-05-26 14:30:51 EDT

This happens because currently Parser.genericsLengthStack not being zero can only imply Parameterized type. But now with diamond case, Parser.genericsLengthStack being -1 can also imply a parameterized type using <>.
All places need to be checked for this.

Comment 6 Ayushman Jain 2011-05-31 14:45:43 EDT

Created attachment 197027 [details]
proposed fix

The patch fixes a few issues with content assist used after <>

1) The first issue is the one reported in comment 4 and has been fixed in AssistParser
2) CompletionParser.consumeGenericTypeWithDiamond() has been added to make sure that the K_BINARY_OPERATOR type corresponding to < and > is popped off the stack when the generic type is consumed. In non <> cases, this gets taken care of by consumeReferenceType(), and other methods that consume the type parameters.
3) I observed that in cases where we request completion after something like Test<>| , we encounter the assist node before we have fully parsed the class instance creation expression and start the completion engine. So Parser.classInstanceCreation() is not called and even though Test<> is a diamond case, the ASTNode.IsDiamond flag remains unset. So, checkForDiamond has been added to CompletionParser.getTypeReferenceForGenericType() instead.

Without changes in 2) and 3) the cases in comment 0 and comment 4 no longer throw the NASException but give completion results that differ from when we have Test<String>

4) CompletionOnQualifiedAllocationExpression.resolve() has been modified to handle the diamond case. It has also been made more resilient so that CompletionEngine can still propose something even if type inference fails or the type cannot be resolved. A part of this resilience can also be seen in the changes to CompletionEngine, where if we couldn't infer the type and happen to reach findConstructors(..) with type as Test<>, the type.availableMethods() call with throw an exception and completion engine will silently bail out. This is because in the followig case
class Test<T> {
 public Test(T t) {
 }
}

the availableMethods will try and substitute the inferred type for T, but on fiding <> instead of type arguments, it will bail out.
So in findConstructors(..), I added some code to make sure we atleast return Test(T t) without substitution when the type inference failed. This will help the following case:
new Test<>(<CTRL-SPACE>)

Comment 7 Ayushman Jain 2011-05-31 14:46:32 EDT

Satyam, please review. Thanks!

Comment 8 Ayushman Jain 2011-06-02 01:05:41 EDT

Created attachment 197215 [details]
proposed fix v1.1 + regression tests

There was a small mistake in the prev. patch.

Comment 9 Satyam Kandula 2011-06-02 07:10:48 EDT

Changes look good to me. 
Minor comments:
1. Your patch missed the changes to CompletionTests. Your earlier patch has those tests. 
2. As the IsDiamond tag is being used for also allocation expressions, you would want to add the particular comment for this tag in ASTNode.java.

Comment 10 Ayushman Jain 2011-06-02 12:48:14 EDT

Created attachment 197247 [details]
released patch

Thanks Satyam

Comment 11 Ayushman Jain 2011-06-02 12:48:39 EDT

Released in BETA_JAVA7.

Comment 12 Srikanth Sankaran 2011-06-29 02:39:54 EDT

Verified using "Eclipse Java Development Tools Patch for Java 7 Support (BETA)	1.0.0.v20110623-0900	org.eclipse.jdt.patch.feature.group	Eclipse.org"