Repository navigation
SONARJAVA-7167 Implement rule S9438 - Private fields that are never reassigned after construction should be declared "final" - #6347
Conversation
This comment has been minimized.
This comment has been minimized.
|
⚖️ Ruling update ready for review: #6348 |
Ruling Diff SummaryDetected changes in 7 rule files: 0 issues removed, 307 issues added. S9438 (
|
|
⚖️ Ruling update ready for review: #6348 |
1 similar comment
|
⚖️ Ruling update ready for review: #6348 |
0982ece to
e5f6e3c
Compare
|
⚖️ Ruling update ready for review: #6348 |
1 similar comment
|
⚖️ Ruling update ready for review: #6348 |
272e824 to
952391e
Compare
|
⚖️ Ruling update ready for review: #6351 |
|
⚖️ Ruling update ready for review: #6351 |
bc39f50 to
e887a58
Compare
e887a58 to
9b9da57
Compare
9b9da57 to
3ba0950
Compare
3ba0950 to
f886d0e
Compare
|
Code Review ✅ Approved 13 closed / 13 findings🟡 Medium risk · Adding S9438 analysis and a quick fix that marks fields final could misclassify initialization writes, producing compile-breaking edits. Adds rule S9438 to flag private instance fields that are assigned once during initialization and never reassigned, so they can be declared ✅ 13 closed✅ Bug: Writes in delegating constructors are ignored, so the rule raises false positives
✅ Bug: Initializer blocks of nested or anonymous classes count as the field's own
✅ Bug: Quick fix makes every variable in a multi-variable declaration final
✅ Edge Case: An early return in a constructor before the assignment is not detected
✅ Edge Case: Lombok @NoArgsConstructor classes are reported, but final breaks compilation
...and 8 more closed from earlier reviews Verification Knowledge ✅ 4 · 🔎 1 PRs 1 inspectedFunctional validation No results Rules No rules evaluated Knowledge ✅ 4 expectations met 🔎 1 informed a finding
Broader impact PRs 1 inspectedCross-PR 1 PR inspected 0 open impactsAuto-approval Not enabledFor more verification details, comment OptionsAuto-apply is off → Gitar will not commit updates to this branch. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar |
|
/pvf |
|
📊 Performance Validation complete |
|
📊 Performance Validation complete |
|
|
||
| private String assignedInConstructor; // Noncompliant {{Make this private field "final", it is never reassigned after construction.}} [[secondary=39]] | ||
| //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| private List<String> assignedAtDeclaration = new ArrayList<>(); // Noncompliant |
There was a problem hiding this comment.
can you put the issue message here as well?
There was a problem hiding this comment.
The issue message is not dynamically constructed, it is a constant one. I dont see the point of testing it multiple times.
There was a problem hiding this comment.
!CHANGE ISSUE MESSAGE (NOT USE CONSTRUCTION WORD)
| <p>The rule raises an issue on a <code>private</code>, non-<code>final</code> instance field whose value is assigned only during object initialization | ||
| and never reassigned afterwards.</p> |
There was a problem hiding this comment.
I think this should be the first sentence in the rule description
| <li>Fields that are not assigned on every constructor path, since a <code>final</code> field must be definitely assigned by the end of each | ||
| constructor and the code would no longer compile.</li> |
There was a problem hiding this comment.
Do we really check that in the implementation? That would require using the CFG
| private static Tree enclosingWriteExpression(IdentifierTree usage) { | ||
| Tree target = usage; | ||
| Tree parent = usage.parent(); | ||
| if (parent != null && parent.is(Tree.Kind.MEMBER_SELECT) && ((MemberSelectExpressionTree) parent).identifier() == usage) { |
There was a problem hiding this comment.
We need to unwrap parentheses around the expression, looking just at the parent is not enough.
|
📊 Performance Validation complete |




Implements S9438 for Java.
The rule raises on a private, non-static, non-final instance field that is assigned exactly once during object initialization and never reassigned afterwards, so it can be declared
final.RSPEC: https://github.com/SonarSource/rspec/pull/8340
Jira: https://sonarsource.atlassian.net/browse/SONARJAVA-7167
Scope
A field is reported only when
finalwould actually compile:Not reported
try, lambda or inner class, since they do not always run exactly onceother.field = ...)LombokFilterfinalwould not compileA quick fix adds the
finalmodifier.