Bug 191090 - [compiler] Preserve annotations for the problem methods
Summary: [compiler] Preserve annotations for the problem methods
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 enhancement with 1 vote (vote)
Target Milestone: 3.4 M5   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-05 14:31 EDT by Olivier Thomann CLA
Modified: 2008-02-04 11:40 EST (History)
2 users (show)

See Also:


Attachments
Proposed fix (12.91 KB, patch)
2008-01-22 08:34 EST, Olivier Thomann CLA
no flags Details | Diff
new patch and regression tests (27.21 KB, patch)
2008-01-22 20:59 EST, 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 Olivier Thomann CLA 2007-06-05 14:31:33 EDT
When a method cannot be compiled because of errors in its body we should try to preserve the annotations defined for the method.
This would improve the usage of problem methods for JUnit4 tests.

Using this test case,

import org.junit.Test;

public class MyJUnit4Test
{
    @Test
    public void test1()
    {
        syntax error
    }

    @Test
    public void test2()
    {

    }

} 

test1 is not failing in a JUnit 4 test harness. Since the @Test annotation is not preserved, the JUnit4 test harness doesn't "recognized" test1 as a JUnit test.
Comment 1 Philipe Mulet CLA 2008-01-16 06:20:19 EST
another example
@interface Annot {}
public class X {
	@Annot
	void foo() {  #  }
	@Annot
	void bar() {	zork();	}
	@Annot
	void baz() {}
}


I'd like to see this one addressed for 3.4.
Olivier - would you have time for it ? It seems that in
ClassFile#generateMethodInfoAttribute(MethodBinding methodBinding, boolean createProblemMethod) 
we already check for problem methods, and bypass annotation dumping if so.

Also, we should consider dumping problem type annotations as well.
Comment 2 Olivier Thomann CLA 2008-01-16 08:37:06 EST
I can quickly look at this one.
I remembered that I got some NPE when enabling it, but maybe your support for problems in annotation might fix it.
Comment 3 Philipe Mulet CLA 2008-01-16 09:35:26 EST
You may want to wait until I am done with bug 196200. Some of the changes near annotation resilience have not been released yet (unclear whether you need them)
Comment 4 Olivier Thomann CLA 2008-01-16 13:16:51 EST
Philippe,

What behavior do you want in the case the annotation contains problems ?
Look at:
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test198
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test219
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test220

If I remove the filtering for problem method, I end up with ClassCastException when trying to generate the annotations for the three cases.
In these cases, we might simply want to remove the annotations from the .class files. This would still be an improvement to the existing code where they are never present for problem methods.

Let me know what you think and I'll adjust the implementation.
Thanks.
Comment 5 Olivier Thomann CLA 2008-01-22 08:34:57 EST
Created attachment 87512 [details]
Proposed fix

This patch removes annotations that cannot be generated, but preserves the other ones.
Comment 6 Olivier Thomann CLA 2008-01-22 08:35:32 EST
Philippe, please review and let me know what you think.
I'll add regression tests for this support.
Comment 7 Philipe Mulet CLA 2008-01-22 10:21:44 EST
Looks good to me. 
Comment 8 Olivier Thomann CLA 2008-01-22 20:59:36 EST
Created attachment 87590 [details]
new patch and regression tests

Better patch.
If the parameter annotations are not properly generated, they are now completely removed (works for visible or invisible annotations).
I added a catch block for ShouldNotImplement exception.
Comment 9 Olivier Thomann CLA 2008-01-23 09:00:44 EST
Released for 3.4M5.

Added regression tests in:
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test249
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test250
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test251
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test252
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test253
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test254
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test255
org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test256
Comment 10 Jerome Lanneluc CLA 2008-02-04 11:40:57 EST
Verified for 3.4M5 using I20080204-0010