Community
Participate
Working Groups
Build ID: 3.3RC2 Currently having a javadoc referencing a private field causes the "never read locally" warning to disappear. The analysis should only consider "real" read access.
(In reply to comment #0) Did not manage to reproduce the behavior you described. Though having Javadoc comments referencing an unused private field, the "never read locally" warning is still present. May you please provide us with some sample code that reproduces described behavior? Thanks.
Clean install of 3.3RC3: public class Class1 { private int unused; /** * Same value as {@link #unused} */ private int unused2; } The field "unused" is not marked as "never read locally".
This is an interesting discussion. Same applies to private/local methods (or should). When a user cares about javadoc consistency (which is the case here), is it ok to let him discard the field, and thus get a problem in his javadoc ?
(In reply to comment #2) the position of the Javadoc comment impacts the behavior: - if the Javadoc is placed before the declaration, then the warning is raised - if the Javadoc is placed after (which is the case here), then no warning anymore (reproduced using a 3.2.2)
Hmm... then indeed this motivates ignoring javadoc then.
Eric - pls investigate a fix.
Created attachment 70372 [details] [file]: proposed fix with 2 test cases Added 2 corresponding test cases org.eclipse.jdt.internal.compiler.ast#test445 and test446. All jdt.core tests completed.
Created attachment 70450 [details] [file]: better patch
OK for me. However, I have two cosmetic remarks: 1) I would have kept a method name with deprecated as the goal of this method is to verify whether the field use is deprecated or not: isFieldDeprecated, fieldUseDeprecated or isFieldUseDeprecate... 2) IMO, method javadoc comment could be clearer: /** * Same implementation than {@link ASTNode#isFieldUseDeprecated(...)} without * touching the {@link ExtraCompilerModifiers#AccLocallyUsed} to keep private * fields unused when they are only referenced in javadoc comments. * * @see "https://bugs.eclipse.org/bugs/show_bug.cgi?id=190970" */
(In reply to comment #9) Thanks for the review. 1 - I've integrated your Javadoc comment proposal 2 - Regarding the method name, I selected this one as, as I understand it, it's more than checking whether the field is deprecated or not (we also check access restriction).
Created attachment 70465 [details] [file]: patch with new Javadoc comment
Created attachment 77111 [details] [patch] improved version
I'd rather avoid having overriding method duplicate so much from the super method. Pls rather introduce a boolean param to original method to condition the usageCheck.
Created attachment 77238 [details] as per Philippe suggestion in comment #13 - all jdt.core tests OK
Since this is not a regression comparing to 3.2 (the same problem existed in 3.2), we will target 3.4.
(In reply to comment #14) Please review last proposed patch
(In reply to comment #16) > (In reply to comment #14) > Please review last proposed patch > KO for me as same issue exist for method references in javadoc and is not fixed by the patch: public class TestMethods { private void unused1() {} /** * Same value as {@link #unused1()} */ private void unused2() {} }
(In reply to comment #17) correct: Philippe's comment #3 was not taken into account.
I also suspect private/local types to also reveal the same issue.
Created attachment 77558 [details] patch + test case covers unused fields, methods and types
(In reply to comment #20) The patch looks good to me. So +1 for this patch.
(In reply to comment #20) > Created an attachment (id=77558) [details] > patch + test case > > covers unused fields, methods and types > The patch is not good as the added tests pass without the fix applied on org.eclipse.jdt.core project: so, -1 for me
Created attachment 79473 [details] [proposed patch + test case] on top v815 - all jdt.core tests OK as per Frederic buddy check: - corrected test cases - alternative patch that uses ASTNode.InsideJavadoc to determine whether
(In reply to comment #23) ... whether a field, method or type is used or not.
(In reply to comment #23) Indeed, much simpler. +1
Released for 3.4M3 in HEAD stream
Verified for 3.4M3 using I20071029-0010 build.
