SONARJAVA-6843: Implement S9365: Copy constructors should initialize all fields

[nathsou]

Summary

• Implement S9365 to report copy constructors that leave eligible instance fields implicitly initialized.
• Exclude static, transient, and declaration-initialized fields; support resolved constructor delegation and instance helper methods.
• Add semantic and non-semantic tests, including conservative handling of unresolved initialization paths.

Jira: https://sonarsource.atlassian.net/browse/SONARJAVA-6843

RSPEC: https://github.com/SonarSource/rspec/pull/7958

AI disclosure

LLM model used for implementation: gpt-5.6-sol (high reasoning)

[hashicorp-vault-sonar-prod]

SONARJAVA-6843

[gitar-bot]

Code Review ✅ Approved 5 resolved / 5 findings

Implements rule S9365 to report copy constructors that leave eligible instance fields implicitly initialized, addressing issues with initialization blocks, symbol comparisons, and method receiver handling. No open findings remain.

✅ 5 resolved

✅ Bug: Fields initialized in an instance initializer block are flagged

📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:108-119 📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:85-99 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html:8-9 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html:49-50
eligibleFields() only excludes fields with a declaration initializer, and analyze() walks only the constructor body plus delegated constructors/instance helpers — nothing ever inspects Tree.Kind.INITIALIZER members. So for class C { private int v; { v = 1; } C(C other) {} } the rule reports v as uninitialized even though the instance initializer block runs for every constructor invocation, including this one; this is a false positive on a Sonar way rule (and the same applies to a blank final field assigned in an initializer block). Either treat fields assigned in an instance initializer block as already initialized (scan Tree.Kind.INITIALIZER members with the same AssignmentCollector) or exclude classes containing an instance initializer block, and add a sample case for it.

✅ Quality: Symbols compared by reference instead of equals()

📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:116 📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:163 📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:191
variable.symbol().owner() == owner, method.enclosingClass() == owner and method.enclosingClass() != owner rely on JTypeSymbol instances being canonical, but JSema caches symbols in a HashMap keyed by the raw ECJ binding while JSymbol.equals() compares normalized bindings — two symbols for the same class can therefore be equals() but not identical (e.g. a parameterized receiver binding vs. the type declaration binding). When that happens on the helper-invocation branch the call is silently skipped without setting complete = false, producing a false positive. Use equals() for symbol comparisons, as every other check in java-checks does.

✅ Edge Case: ++/-- on a field not counted while compound assignment is

📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:146-153 📄 java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java:70-84 📄 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html:10-11
AssignmentCollector only overrides visitAssignmentExpression, so value += other.value marks the field initialized (CompoundAssignment is compliant in the sample) while value++ does not (IncrementIsNotAssignment is asserted Noncompliant) — both forms read the default value and write the field, so the two results are inconsistent for equivalent code. Either treat unary increment/decrement on an eligible field as an explicit write, or exclude compound assignments too, and make the sample/RSPEC state the chosen semantics.

✅ Quality: New isCurrentInstance rejection branches are untested

📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:234-246 📄 java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java:296-310
The two samples added in this commit only exercise the positive path of isCurrentInstance (Owner.this.field / Owner.this.helper() where the qualifier is the owner). The rejection branches added at lines 240-245 — a member-select receiver whose identifier is not this (e.g. other.delegate.value = 1) and a qualified this whose enclosingClass() is an enclosing class rather than the owner (Outer.this.field = ... from a non-static inner class' copy constructor) — are unreachable from the sample file: every nested class in the sample is static and no receiver other than a bare identifier or X.this appears. Add samples covering both so the enclosing-instance discrimination is verified rather than assumed.

✅ Edge Case: Cast receiver on a self-call is neither followed nor treated as unknown

📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:226-240 📄 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java:155-169
isCurrentInstance only recognizes a bare/parenthesized this or X.this, so a receiver such as ((Foo) this) falls through to return false. In visitMethodInvocation that makes ((Foo) this).initialize(other); neither merge the helper's assignments nor set complete = false, so a copy constructor that fully initializes its fields through such a call is reported (false positive) instead of being skipped conservatively. Either unwrap type casts in isCurrentInstance, or make an unrecognized instance receiver whose resolved method is a non-static method of owner set complete = false.

Implementation Status ✅ 1 of 1 objectives covered

✅ SONARJAVA-6843 - 1 of 1 objectives covered

This PR implements rule S9365 requiring copy constructors to initialize all eligible instance fields.

✅ 1 covered here

• ✅ Implement rule S9365 requiring copy constructors to initialize all eligible instance fields

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqube-next]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
90.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube