Bug 212857 - [dom] AST has wrong source range after parameter with array-valued annotation
Summary: [dom] AST has wrong source range after parameter with array-valued annotation
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Frederic Fusier CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-13 05:42 EST by Markus Keller CLA
Modified: 2008-02-05 05:50 EST (History)
4 users (show)

See Also:


Attachments
Proposed fix + regression tests (10.96 KB, patch)
2007-12-14 11:29 EST, Olivier Thomann CLA
no flags Details | Diff
New proposed patch (26.21 KB, patch)
2007-12-18 06:19 EST, Frederic Fusier CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Keller CLA 2007-12-13 05:42:09 EST
I20071212-1800

AST has a wrong source range for a method parameter with an array-valued annotation:

package xy;

public class C {
	void m(@SuppressWarnings({"unused", "bla"}) int arg) {
		int local;
	}
}


The MethodDeclaration [34, 109] has the correct source range, but its body's Block [59, 17] has a wrong range (namely, the range of the ArrayInitializer in the SuppressWarnings annotation).

This causes e.g. an NPE when you select "bla", invoke Quick Fix, and select the "Extract to local variable" proposal:

Caused by: java.lang.NullPointerException
at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.insertAt(ExtractTempRefactoring.java:710)
at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.createAndInsertTempDeclaration(ExtractTempRefactoring.java:645)
at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.createTempDeclaration(ExtractTempRefactoring.java:737)
at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.doCreateChange(ExtractTempRefactoring.java:525)
at org.eclipse.jdt.internal.corext.refactoring.code.ExtractTempRefactoring.checkFinalConditions(ExtractTempRefactoring.java:468)
at org.eclipse.jdt.internal.ui.text.correction.QuickAssistProcessor$RefactoringCorrectionProposal.createTextChange(QuickAssistProcessor.java:1543)
at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.createChange(CUCorrectionProposal.java:478)
at org.eclipse.jdt.internal.ui.text.correction.proposals.ChangeCorrectionProposal.getChange(ChangeCorrectionProposal.java:229)
at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.getTextChange(CUCorrectionProposal.java:488)
at org.eclipse.jdt.internal.ui.text.correction.proposals.CUCorrectionProposal.getAdditionalProposalInfo(CUCorrectionProposal.java:172)
at org.eclipse.jface.text.contentassist.AdditionalInfoController$6.run(AdditionalInfoController.java:162)
Comment 1 Frederic Fusier CLA 2007-12-13 07:25:44 EST
Problem comes from the ASTConverter.retrieveStartBlockPosition(int,int) method which stops at the first opening brace although in this case it should not.
Comment 2 Frederic Fusier CLA 2007-12-13 07:28:52 EST
Already happens in 3.3 => not a stop ship for 3.4M4
Comment 3 Olivier Thomann CLA 2007-12-13 12:09:02 EST
I'll have a look.
Comment 4 Olivier Thomann CLA 2007-12-13 17:07:58 EST
The fix is trivial. It could have made it into 3.4M4 if this blocks an important feature from the UI, otherwise it can wait for 3.4M5.
Comment 5 Olivier Thomann CLA 2007-12-13 20:18:08 EST
Once fixed, this would be a good candidate for 3.3.2 as it can break any users of the DOM/AST tree.
Comment 6 Olivier Thomann CLA 2007-12-14 11:29:00 EST
Created attachment 85291 [details]
Proposed fix + regression tests

Frédéric, let me know what you think.
Comment 7 Frederic Fusier CLA 2007-12-18 06:19:32 EST
Created attachment 85450 [details]
New proposed patch

I'm OK with the patch you posted previously. I just modified to avoid code duplication and use recovery position when there was no opening brace.

I also added tests in the new class I created since I got dom component responsibility (ASTConverterBugsTest) to identify my contributions more easily.

All JDT/Core and JDT/UI tests pass with this patch
Comment 8 Frederic Fusier CLA 2007-12-18 06:20:04 EST
Released for 3.4M5 in HEAD stream.
Comment 9 David Audel CLA 2008-02-05 05:50:45 EST
Verified for 3.4M5 using build I20080204-0010.

A NPE still occurs i entered another bug report (bug 217806)because the source range bug is fixed .