Community
Participate
Working Groups
Consider the following code: ---------------------------- import org.springframework.beans.factory.annotation.Autowired; public class Auto { @Autowired private Auto auto1; @SuppressWarnings("unused") @Autowired private Auto auto2; } Actual behaviour: ----------------- There is a warning “Unnecessary @SuppressWarnings("unused")” on field auto2. Expected behaviour: ------------------- There should not be a warning on field auto2. There should be “The value of the field Auto.auto1 is not used” warning on field auto1. Rationale: The @Autowired annotation indeed means it is possible that this field is written at some time, however, the field is never read and therefore it is superfluous. No warnings are left. This is different for the new version 4.2 – the older 3.8 did not consider @Autowired fields as used. I consider this a regression.
Because of the fix for bug 365437, this is expected behaviour. Please see bug 365437 comment 33 for more detailed explanation. I intend to close as INVALID. (Please also see bug 376590 and let us know if you have a rationale for making an exception for '@Autowired', just like it is requested for '@Inject')
The annotation @Autowired declares a dependency thus introduce coupling between Spring beans. If these dependencies are unused, the tool should warn me about that because I have the chance to reduce coupling simply by removing these unused fields. The annotation org.springframework.beans.factory.annotation.Autowired when used on a field will trigger writing (by reflection) at creation time by the Spring container. It does not have any other effect.
That way almost all the library annotations should be excluded. I'm not sure how scalable that is. Anyway, marking as dup of bug 376590 *** This bug has been marked as a duplicate of bug 376590 ***
As stated in bug 376590 comment 15, please reopen this bug. Goal: also ignore the @Autowired annotation when performing unused field analysis.
Not promising a fix. Reopening so this is in the radar for 4.4. We have no time to look at this now. Post March 2014, when the Java 8 work would be complete, we can take a look - perhaps only to close as WONTFIX.
Just housekeeping - assigning the bug to Srikanth.
(In reply to Srikanth Sankaran from comment #5) > Not promising a fix. Reopening so this is in the radar for 4.4. We have no > time to > look at this now. Post March 2014, when the Java 8 work would be complete, > we can > take a look - perhaps only to close as WONTFIX. Deferring to 4.5 - very busy with maintenance work for Java 8.
This bug is also present when using the javax.annotation.Resource annotation to inject fields. Could that be fixed as part of this also?
Another annotation that should be ignored is @Mock annotation from frameworks such as Mockito, as an alternative, could there simply be an option like "Ignore unused private fields with annotations".
I very much like Patrick's idea. Given the multitude of frameworks one can use nowadays that include annotations, trying to cover each and every case would be an ongoing, frustrating task. More importantly, people surely will have different opinions on this topic. Myself for instance, never agreed with the fix for the original "issue" (https://bugs.eclipse.org/bugs/show_bug.cgi?id=365437). Programming tools should, as much as possible, avoid being opinionated, therefore, it would be definitely ideal to give people the freedom to add or remove annotations to the list.
Created attachment 247916 [details] Spring Autowired Patch I have the same problem with the Spring Autowired annotation. I implemented a patch to fixed this. Ulrich
Too many noise diffs in this patch.
Created attachment 247924 [details] Fixed Autowired Patch I remove the unnecessary changes from the patch. Now the patch contains only the necessary changes. Sorry for that.
Jay, there is a proposed patch - I haven't had time to look at it. This issue has a colorful history and I don't remember all the details.
For simpler review commit the same patch to Gerrit: https://git.eclipse.org/r/36356
I know I am late in getting into this discussion. If we are fixing this, shouldn't we consider the reported issue in comment #8 also. However, shouldn't we take a moment to think about something that will be future proof? I went through this bug and related ones and found two proposals: 2. Provide a preference to exclude annotated private fields from this analysis. (or) 2. Provide a way for the user to configure what annotations he would like to be considered. I prefer the first. The second one will be complicated as it's not just the annotations' name but the members/values too that would dictate the terms. BTW, I went through the patch and looks good, save few white space issues.
I stumbled upon a bogus code comment, see https://git.eclipse.org/r/#/c/36356/1/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java Regarding "future proof": I don't have more answers than what I already wrote in the discussion of bug 376590, sorry.
(In reply to Ulrich Grave from comment #15) > For simpler review commit the same patch to Gerrit: > https://git.eclipse.org/r/36356 Ulrich, can you find some time to look at Stephan's comments on the patch? Thanks!
(In reply to Jay Arthanareeswaran from comment #18) > (In reply to Ulrich Grave from comment #15) > > For simpler review commit the same patch to Gerrit: > > https://git.eclipse.org/r/36356 > > Ulrich, can you find some time to look at Stephan's comments on the patch? > Thanks! Actually on a close look, the code is still intact. Am I missing something here?
(In reply to Jay Arthanareeswaran from comment #19) > (In reply to Jay Arthanareeswaran from comment #18) > > (In reply to Ulrich Grave from comment #15) > > > For simpler review commit the same patch to Gerrit: > > > https://git.eclipse.org/r/36356 > > > > Ulrich, can you find some time to look at Stephan's comments on the patch? > > Thanks! > > Actually on a close look, the code is still intact. Am I missing something > here? My sincere apology, I don't know what exactly was the problem but it certainly was sitting on my chair.
Gerrit change https://git.eclipse.org/r/36356 was merged to [master]. Commit: http://git.eclipse.org/c/jdt/eclipse.jdt.core.git/commit/?id=270b2d0dc66fd2f8c8012264ce82bca4953b85cb
Thanks Stephan, I have removed the comment and released via Gerrit.
Verified for 4.5 M7 with build I20150422-1000
(In reply to Jay Arthanareeswaran from comment #16) > [...] > 2. Provide a way for the user to configure what annotations he would like to > be considered. > > I prefer the first. The second one will be complicated as it's not just the > annotations' name but the members/values too that would dictate the terms. FYI: interested parties may want to join me discussing bug 547451 comment 24.
