Bug 308754 - CompilationUnit.rewrite messes up .class-literal in annotation instead of changing class to interface
Summary: CompilationUnit.rewrite messes up .class-literal in annotation instead of cha...
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.6 M7   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-11 11:32 EDT by Fabian Stolz CLA
Modified: 2010-04-27 02:41 EDT (History)
2 users (show)

See Also:


Attachments
Proposed fix + regression tests (8.80 KB, patch)
2010-04-12 11:38 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Fabian Stolz CLA 2010-04-11 11:32:23 EDT
Build Identifier: 20090619-0625

I'm modifying an AST created by using ASTParser to parse an ICompilationUnit. The class parsed (let's call it C) has an annotation (A) which accepts a .class-literal (X.class) as parameter.

The whole thing looks like this before the modification:

@A(X.class) public class C {/*[...]*/}

The modification comprises changing C from class to interface by calling setInterface(true) on the corresponding TypeDeclaration. I then use CompilationUnit's rewrite method to create a TextFileChange which I want to use to apply the modification.

The result should look like this:

@A(X.class) public interface C {/*[...]*/}

But what happens instead is:

@A(X.interface) public class C {/*[...]*/}


See steps to reproduce for a detailed example. 

Reproducible: Always

Steps to Reproduce:
1. Use org.junit.tests.experimental.ExperimentalTests.java from JUnit 4.7 sources as ICompilationUnit "input" to reproduce the problem. The relevant part of its source is: 

package org.junit.tests.experimental;

import org.junit.runner.RunWith;
import org.junit.runners.Suite.SuiteClasses;
/*[...]*/
@RunWith(Suite.class)
@SuiteClasses( { /*[...]*/})
public class ExperimentalTests {

}

2. Parse the code:
ASTParser parser = ASTParser.newParser(AST.JLS3);
parser.setKind(ASTParser.K_COMPILATION_UNIT);
parser.setResolveBindings(true);
parser.setSource(input);
parser.setProject(input.getJavaProject());
CompilationUnit parsed = (CompilationUnit)parser.createAST(new NullProgressMonitor());

3. Activate recording of modifications:
parsed.recordModifications();

4. Modify the source using an ASTVisitor:
parsed.accept(new ASTVisitor() {
    @Override
    public boolean visit(TypeDeclaration node) {
        node.setInterface(true);
    }
});

5. Create TextFileChange (I'm using TextFileChange because the modifications are to be part of a refactoring):
TextFileChange change = new TextFileChange(input.getElementName(), (IFile)input.getResource());
change.setTextType("java");
Document document = new Document(input.getSource());
TextEdit edit = parsed.rewrite(document, input.getJavaProject().getOptions(true));
change.setEdit(edit);

6. Apply the TextFileChange:
change.perform(new NullProgressMonitor());
Comment 1 Olivier Thomann CLA 2010-04-12 10:43:56 EDT
This is bad. I am investigating.
Comment 2 Olivier Thomann CLA 2010-04-12 11:38:30 EDT
Created attachment 164574 [details]
Proposed fix + regression tests
Comment 3 Olivier Thomann CLA 2010-04-12 11:42:17 EDT
Released for 3.6M7.
Added tests in:
org.eclipse.jdt.core.tests.rewrite.describing.ASTRewritingTypeDeclTest#testTypeDeclarationChange
org.eclipse.jdt.core.tests.rewrite.describing.ASTRewritingTypeDeclTest#testTypeDeclarationChange2
org.eclipse.jdt.core.tests.rewrite.modifying.ASTRewritingModifyingOtherTest#test0005
org.eclipse.jdt.core.tests.rewrite.modifying.ASTRewritingModifyingOtherTest#test0006
Comment 4 Olivier Thomann CLA 2010-04-12 11:45:03 EDT
(In reply to comment #0)
> 2. Parse the code:
> ASTParser parser = ASTParser.newParser(AST.JLS3);
> parser.setKind(ASTParser.K_COMPILATION_UNIT);
> parser.setResolveBindings(true);
> parser.setSource(input);
> parser.setProject(input.getJavaProject());
> CompilationUnit parsed = (CompilationUnit)parser.createAST(new
> NullProgressMonitor());
Note that org.eclipse.jdt.core.dom.ASTParser.setSource(ICompilationUnit) also sets the project. So setProject(..) call is not necessary.

Let me know if you find any issue with the next I-build.
Comment 5 Fabian Stolz CLA 2010-04-12 13:28:30 EDT
(In reply to comment #4)
> Let me know if you find any issue with the next I-build.

Wow, that was fast. Thank you very much. I'll try out the latest build asap.
Comment 6 Olivier Thomann CLA 2010-04-12 13:31:32 EDT
(In reply to comment #5)
> Wow, that was fast. Thank you very much. I'll try out the latest build asap.
When the bug is well explained and the steps to reproduce are clear, it helps to get it fixed.
Comment 7 Jay Arthanareeswaran CLA 2010-04-27 02:41:34 EDT
Verified for 3.6M7 by code inspection and unit tests.