Community
Participate
Working Groups
Build id : I20100504-0800 Test case: import java.util.*; class Bug { @SuppressWarnings("rawtypes") void foo(ArrayList arg) { for ( boolean a= arg.add(1), b= arg.add(1); Boolean.FALSE; ) { System.out.println(a && b); } } } 1) Invoke quick fix on the first fragment(a= arg.add(1)) adds the SuppressWarnings("unchecked") in the beginning of the expression but does not apply to the whole declaration expression . Can still see the warning on the second declaration fragment. (b= arg.add(1)) 2)If quick fix is invoked on the second fragment, it results in another "unchecked" token being added with a warning and quick fix proposal to remove it as unnecessary: for ( @SuppressWarnings({ "unchecked", "unchecked" }) boolean a= arg.add(1), b= arg.add(1); Boolean.FALSE; ) { Expected: Should apply to the whole expression similar to the field declaration : import java.util.*; class Bug { @SuppressWarnings("rawtypes") ArrayList arg; @SuppressWarnings("unchecked") boolean a= arg.add(1), b= arg.add(1); }
I'll investigate for 3.6RC1
Created attachment 167310 [details] Proposed fix + regression tests
Srikanth, please review.
Note that the quick fix is still in the works (bug 105668), so you need to put the @SuppressWarnings("unchecked") at the right place manually. The proposed fix works fine for me (though it's a little strange that declarationSourceStart/End means the whole field for FieldDeclarations but only a single variable for LocalDeclarations...).
(In reply to comment #4) > Note that the quick fix is still in the works (bug 105668), so you need to put > the @SuppressWarnings("unchecked") at the right place manually. Bug 105668 is tagged as RC1. So I thought this one should be fixed quickly as it blocks a RC1 bug. > The proposed fix works fine for me (though it's a little strange that > declarationSourceStart/End means the whole field for FieldDeclarations but only > a single variable for LocalDeclarations...). This is the case since the beginning for historical reasons. I won't change that now for sure. Sometimes we have to live with inconsistencies :-).
(In reply to comment #5) Thanks, +1 then. A comment in declarationSourceStart about the different usages would make this more easily understandable. +1 for adding that if you agree.
Released for 3.6RC1. Added regression tests in: org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test287 org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test288 Note org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test288 would pass without the fix. It is there to ensure that field declaration source ends cover multiple fields declarations in case we want to fix to change that in the future.
(In reply to comment #3) > Srikanth, please review. Patch looks good. Note that the junits references does not reference this bug number, but the related bug# 304031 instead.
(In reply to comment #7) Verified. Thanks for the quick fix Olivier!
The fix only works if the locals don't have other annotations. E.g. here, it still warns on the second arg.add(..) because of @Other (same with @Deprecated): import java.util.*; class Bug { @SuppressWarnings("rawtypes") void foo(ArrayList arg) { for ( // @Deprecated @Other @SuppressWarnings("unchecked") boolean a = arg.add(1), b = arg.add(2); Boolean.FALSE; ) { System.out.println(a && b); } } } @interface Other { }
Created attachment 167791 [details] Proposed fix + regression test Good catch. I got caught by the return statement. So only the first annotation would be checked. This patch fixes that issue.
Frédéric, Markus, please review. I would like this to be part of tomorrow's JDT contribution.
Patch looks good to me.
Released for 3.6RC1. New regression test added in: org.eclipse.jdt.core.tests.compiler.regression.AnnotationTest#test289
I noticed one inconsistency. In the following code import java.util.*; class Bug { @SuppressWarnings("rawtypes") void foo(ArrayList arg) { @SuppressWarnings("unchecked") // bad: unnecessary suppressWarnings warn boolean aa = arg.add(1), // good: local variable a never read warning bb = arg.add(1); if (bb) System.out.println("hi"); } } If either aa, or bb is not read the suppressWarnings annotation is warned by the compiler as unnecessary. This is a bogus warning since the annotation applies for atleast one variable in the declaration series. Olivier, what do you think?
(In reply to comment #15) Good catch, that's a regression compared to 3.6M7. Please open a new bug for this (we don't reopen bugs after the target milestone has already shipped).
Raised bug 313109 for FUP.
Verified for 3.6 RC1 using build I20100513-1500.
Verified.
