Bug 407056

Comment 1 Markus Keller 2013-05-02 06:00:50 EDT

Note that ASTRewrite has not yet been implemented for this node (bug 406469). But we can still start with identifying problem areas and working on solutions.

Comment 2 Martin Mathew 2013-05-23 08:41:25 EDT

The issue to be tackled with this bug is to fix all the places where we currently assume the SimpleType(QualifiedName) structure. I.e. ASTVisitors and any other code that:
- assumes it knows it gets a SimpleType(QualifiedName), but now gets a PackageQualifiedType
- assumes that the parent of a QualifiedName or a SimpleName must be one of the old a subtypes of Type, but now it can also be a PackageQualifiedType

Comment 3 Martin Mathew 2013-06-05 01:38:00 EDT

Updated the potential locations where PackageQualifiedType can occur in JDT UI except in #thrownExceptionTypes. Need to perform one more round of checking once the bug 403924 which deals with updating MethodDeclaration#thrownExceptions() with MethodDeclaration#thrownExceptionTypes() is resolved. #thrownExceptionTypes() can return a List with SimpleType and PackageQualifiedType. Hence i'm holding the patch for the time being.

Comment 4 Martin Mathew 2013-06-27 00:55:59 EDT

Created attachment 232819 [details]
WIP Patch

This is a WIP patch. Uploading this in case Noopur you have time to look into it once bug 403923 is completed.

Comment 5 Martin Mathew 2013-06-27 00:57:01 EDT

I meant bug 403924 in my previous comment.

Comment 6 Martin Mathew 2013-10-29 02:29:40 EDT

Created attachment 236994 [details]
Patch created against BETA_JAVA8 ws

I have created the patch after updating most of the potential locations where PackageQualifiedType can occur in JDT UI. Below are the files where SimpleType(QualifiedName/SimpleName) usage appears but I have ignored them as I felt PackageQualifiedType could not occur in those scenario. Listing the files ignored for verification.
=> ASTNodeFactory.java (2 matches): 
	-> 286: if (type instanceof SimpleType) { 
	-> 354: if (type instanceof SimpleType) { 
In the above locations, type is created via ImportRewrite.addImport(..) and that never returns a PQT. Hence ignored the occurrence of PQT in the above cases.

=> MoveInnerToTopRefactoring.java (3 matches) -> Here the QualifiedName of a SimpleType is used to handle the import statement, hence ignoring the occurrence of PQT at this place.

=> Java50Fix.java#isRawTypeReference(ASTNode) -> This method handles only a SimpleType. What is the usecase, is it possibe that the type can be a PackageQualifiedType at this point?

Comment 7 Martin Mathew 2014-01-06 22:28:50 EST

Created attachment 238708 [details]
Updated Patch

Patch updated. Created against BETA_JAVA8 branch as per bug 424920.

Comment 8 Martin Mathew 2014-01-15 19:47:37 EST

(In reply to Manju Mathew from comment #6)
> => ASTNodeFactory.java (2 matches): 
> 	-> 286: if (type instanceof SimpleType) { 
> 	-> 354: if (type instanceof SimpleType) { 
> In the above locations, type is created via ImportRewrite.addImport(..) and
> that never returns a PQT. Hence ignored the occurrence of PQT in the above
> cases.

The above assumption would be wrong after bug 417937 is fixed. ImportRewrite.addImport(..) can return a NQT.

Comment 9 Markus Keller 2014-02-07 19:25:02 EST

Released Manju's patch with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=815164b3b52c2daf5d745ad9399c77ceb9c881fa and my additions with http://git.eclipse.org/c/jdt/eclipse.jdt.ui.git/commit/?id=f9694bfb4ba224ca3ff558a3eef89f9433b055e5

(In reply to Manju Mathew from comment #8)
> The above assumption would be wrong after bug 417937 is fixed.
> ImportRewrite.addImport(..) can return a NQT.

That's correct in general, but we know this can't happen for an annotation type, because the grammar doesn't allow type annotations on such a type name reference.
The newAnnotation(..) methods have been moved into ImportRewrite, where it's safe to keep this assumption.