From 4f9bfcfe045d86e2c450dde8a0e3bd50b6dd13da Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 25 Aug 2026 10:10:51 +0200 Subject: [PATCH 1/5] Implement S9365 --- ...CopyConstructorMissesFieldCheckSample.java | 32 ++ ...CopyConstructorMissesFieldCheckSample.java | 295 ++++++++++++++++++ .../CopyConstructorMissesFieldCheck.java | 240 ++++++++++++++ .../CopyConstructorMissesFieldCheckTest.java | 53 ++++ .../org/sonar/l10n/java/rules/java/S9365.html | 61 ++++ .../org/sonar/l10n/java/rules/java/S9365.json | 23 ++ .../main/resources/profiles/Sonar_way/S9365 | 0 7 files changed, 704 insertions(+) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/CopyConstructorMissesFieldCheckSample.java create mode 100644 java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/CopyConstructorMissesFieldCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.json create mode 100644 sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9365 diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/CopyConstructorMissesFieldCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/CopyConstructorMissesFieldCheckSample.java new file mode 100644 index 00000000000..ba414e9c9c8 --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/CopyConstructorMissesFieldCheckSample.java @@ -0,0 +1,32 @@ +package checks; + +class CopyConstructorWithUnresolvedHelper { + private int value; + + CopyConstructorWithUnresolvedHelper(CopyConstructorWithUnresolvedHelper other) { + this.initializeFromMissingDependency(other); + } +} + +class CopyConstructorWithImplicitUnresolvedHelper { + private int value; + + CopyConstructorWithImplicitUnresolvedHelper(CopyConstructorWithImplicitUnresolvedHelper other) { + initializeFromMissingDependency(other); + } +} + +class ConstructorWithUnresolvedParameter { + private int value; + + ConstructorWithUnresolvedParameter(MissingType other) { + } +} + +class CopyConstructorWithUnresolvedDelegation { + private int value; + + CopyConstructorWithUnresolvedDelegation(CopyConstructorWithUnresolvedDelegation other) { + this(other, MissingDependency.VALUE); + } +} diff --git a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java new file mode 100644 index 00000000000..6a74e3c506f --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java @@ -0,0 +1,295 @@ +package checks; + +class CopyConstructorMissesFieldCheckSample { + + static class Basic { + private static int instances; + private transient Object cache; + private Object initialized = new Object(); + private final String name; + private String note; + private int retries; + private boolean enabled; + + Basic(Basic other) { // Noncompliant {{This copy constructor leaves eligible fields uninitialized; initialize them explicitly to distinguish omissions from intentional resets.}} [[secondary=10,11,12]] + name = other.name; + } + } + + static class ExplicitDefaults { + private String text; + private int number; + private boolean flag; + + ExplicitDefaults(ExplicitDefaults other) { + this.text = null; + number = 0; + this.flag = false; + } + } + + static class ConstructorsThatAreNotCopies { + private String value; + + ConstructorsThatAreNotCopies() { + } + + ConstructorsThatAreNotCopies(String value) { + } + + ConstructorsThatAreNotCopies(ConstructorsThatAreNotCopies other, int ignored) { + } + } + + static class Generic { + private T first; + private T second; + + Generic(Generic other) { // Noncompliant [[secondary=46]] + first = other.first; + } + } + + static class ForeignAssignments { + private int value; + + ForeignAssignments(ForeignAssignments other) { // Noncompliant [[secondary=54]] + other.value = 1; + } + } + + static class ShadowedField { + private int value; + + ShadowedField(ShadowedField other) { // Noncompliant [[secondary=62]] + int value = other.value; + value = 1; + } + } + + static class CompoundAssignment { + private int value; + + CompoundAssignment(CompoundAssignment other) { + value += other.value; + } + } + + static class IncrementIsNotAssignment { + private int value; + + IncrementIsNotAssignment(IncrementIsNotAssignment other) { // Noncompliant [[secondary=79]] + value++; + } + } + + static class Delegation { + private String name; + private int count; + + Delegation(Delegation other) { + this(other.name, other.count); + } + + Delegation(String name, int count) { + this.name = name; + this.count = count; + } + } + + static class IncompleteDelegation { + private String name; + private int count; + + IncompleteDelegation(IncompleteDelegation other) { // Noncompliant [[secondary=102]] + this(other.name); + } + + IncompleteDelegation(String name) { + this.name = name; + } + } + + static class Helpers { + private String name; + private int count; + + Helpers(Helpers other) { + copyName(other); + this.copyCount(other); + } + + private void copyName(Helpers other) { + name = other.name; + } + + private void copyCount(Helpers other) { + finishCount(other); + } + + private void finishCount(Helpers other) { + count = other.count; + } + } + + static class IncompleteHelper { + private String name; + private int count; + + IncompleteHelper(IncompleteHelper other) { // Noncompliant [[secondary=137]] + setName(other); + } + + private void setName(IncompleteHelper other) { + name = other.name; + } + } + + static class CallsOnOtherDoNotCount { + private int value; + + CallsOnOtherDoNotCount(CallsOnOtherDoNotCount other) { // Noncompliant [[secondary=149]] + other.initialize(); + } + + private void initialize() { + value = 1; + } + } + + static class StaticHelperDoesNotCount { + private int value; + + StaticHelperDoesNotCount(StaticHelperDoesNotCount other) { // Noncompliant [[secondary=161]] + initialize(other); + } + + private static void initialize(StaticHelperDoesNotCount target) { + target.value = 1; + } + } + + static class ConditionalAssignment { + private int value; + + ConditionalAssignment(ConditionalAssignment other) { + if (other.value > 0) { + value = other.value; + } + } + } + + static class DeferredAssignments { + private int lambdaValue; + private int localClassValue; + private int anonymousClassValue; + + DeferredAssignments(DeferredAssignments other) { // Noncompliant [[secondary=183,184,185]] + Runnable lambda = () -> lambdaValue = other.lambdaValue; + class Local { + void set() { + localClassValue = other.localClassValue; + } + } + Runnable anonymous = new Runnable() { + @Override + public void run() { + anonymousClassValue = other.anonymousClassValue; + } + }; + } + } + + static class SelfTypedField { + private SelfTypedField parent; + private String label; + + SelfTypedField(SelfTypedField other) { // Noncompliant [[secondary=205]] + parent = other.parent; + } + } + + record Point(int x, int y) { + Point(Point other) { + this(other.x, other.y); + } + } + + static class MultipleCopyConstructors { + private int first; + private int second; + + MultipleCopyConstructors(MultipleCopyConstructors other) { // Noncompliant [[secondary=220]] + first = other.first; + } + + MultipleCopyConstructors(MultipleCopyConstructors other, boolean marker) { + second = other.second; + } + } + + static class ParenthesizedThis { + private int value; + + ParenthesizedThis(ParenthesizedThis other) { + (this).value = other.value; + } + } + + static class EligibilityExclusions { + private static int staticValue; + private transient int transientValue; + private int initializedValue = 1; + private final int initializedFinal = 2; + + EligibilityExclusions(EligibilityExclusions other) { + } + } + + static class DifferentTypeParameter { + private int value; + + DifferentTypeParameter(Basic other) { + } + } + + static class ParentType { + } + + static class ChildType extends ParentType { + private int value; + + ChildType(ParentType other) { + } + } + + static class RecursiveHelper { + private int value; + + RecursiveHelper(RecursiveHelper other) { + initialize(other); + } + + private void initialize(RecursiveHelper other) { + initialize(other); + } + } + + static class DelegationChain { + private String name; + private int value; + + DelegationChain(DelegationChain other) { + this(other.name); + } + + DelegationChain(String name) { + this(name, 0); + } + + DelegationChain(String name, int value) { + this.name = name; + this.value = value; + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java new file mode 100644 index 00000000000..cd14069b194 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java @@ -0,0 +1,240 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import java.util.ArrayList; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.java.model.ModifiersUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaFileScannerContext; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.semantic.Type; +import org.sonar.plugins.java.api.tree.AssignmentExpressionTree; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.ClassTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.LambdaExpressionTree; +import org.sonar.plugins.java.api.tree.MemberSelectExpressionTree; +import org.sonar.plugins.java.api.tree.MethodInvocationTree; +import org.sonar.plugins.java.api.tree.MethodTree; +import org.sonar.plugins.java.api.tree.Modifier; +import org.sonar.plugins.java.api.tree.NewClassTree; +import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.VariableTree; + +@Rule(key = "S9365") +public class CopyConstructorMissesFieldCheck extends IssuableSubscriptionVisitor { + + private static final String ISSUE_MESSAGE = + "This copy constructor leaves eligible fields uninitialized; initialize them explicitly to distinguish omissions from intentional resets."; + + @Override + public List nodesToVisit() { + return List.of(Tree.Kind.CONSTRUCTOR); + } + + @Override + public void visitNode(Tree tree) { + if (context.getSemanticModel() == null) { + return; + } + MethodTree constructor = (MethodTree) tree; + if (constructor.parameters().size() != 1 || constructor.block() == null) { + return; + } + + Symbol.MethodSymbol constructorSymbol = constructor.symbol(); + Symbol.TypeSymbol owner = constructorSymbol.enclosingClass(); + Type parameterType = constructor.parameters().get(0).symbol().type(); + if (constructorSymbol.isUnknown() + || owner == null + || owner.isUnknown() + || owner.type().isUnknown() + || parameterType.isUnknown() + || !owner.type().erasure().equals(parameterType.erasure())) { + return; + } + + ClassTree classTree = owner.declaration(); + // Record component fields are initialized implicitly by the canonical constructor. + if (classTree == null || classTree.is(Tree.Kind.RECORD)) { + return; + } + + Map eligibleFields = eligibleFields(classTree, owner); + if (eligibleFields.isEmpty()) { + return; + } + + AnalysisResult result = analyze(constructor, owner, eligibleFields.keySet(), new HashSet<>()); + if (!result.complete) { + return; + } + + List secondaries = new ArrayList<>(); + eligibleFields.forEach((field, declaration) -> { + if (!result.assignedFields.contains(field)) { + secondaries.add(new JavaFileScannerContext.Location( + "Field \"" + field.name() + "\" is not explicitly initialized by this copy constructor.", + declaration.simpleName())); + } + }); + if (!secondaries.isEmpty()) { + reportIssue(constructor.simpleName(), ISSUE_MESSAGE, secondaries, null); + } + } + + private static Map eligibleFields(ClassTree classTree, Symbol.TypeSymbol owner) { + Map fields = new LinkedHashMap<>(); + classTree.members().stream() + .filter(member -> member.is(Tree.Kind.VARIABLE)) + .map(VariableTree.class::cast) + .filter(variable -> variable.initializer() == null) + .filter(variable -> !ModifiersUtils.hasModifier(variable.modifiers(), Modifier.STATIC)) + .filter(variable -> !ModifiersUtils.hasModifier(variable.modifiers(), Modifier.TRANSIENT)) + .filter(variable -> variable.symbol().owner() == owner) + .forEach(variable -> fields.put(variable.symbol(), variable)); + return fields; + } + + private static AnalysisResult analyze(MethodTree method, Symbol.TypeSymbol owner, Set eligibleFields, + Set activeMethods) { + Symbol.MethodSymbol methodSymbol = method.symbol(); + if (methodSymbol.isUnknown() || method.block() == null || !activeMethods.add(methodSymbol)) { + return AnalysisResult.incomplete(); + } + AssignmentCollector collector = new AssignmentCollector(owner, eligibleFields, activeMethods); + method.block().accept(collector); + activeMethods.remove(methodSymbol); + return collector.result(); + } + + private static final class AssignmentCollector extends BaseTreeVisitor { + private final Symbol.TypeSymbol owner; + private final Set eligibleFields; + private final Set activeMethods; + private final Set assignedFields = new HashSet<>(); + private boolean complete = true; + + private AssignmentCollector(Symbol.TypeSymbol owner, Set eligibleFields, Set activeMethods) { + this.owner = owner; + this.eligibleFields = eligibleFields; + this.activeMethods = activeMethods; + } + + @Override + public void visitAssignmentExpression(AssignmentExpressionTree tree) { + Symbol assignedField = currentInstanceField(tree.variable()); + if (assignedField != null) { + assignedFields.add(assignedField); + } + super.visitAssignmentExpression(tree); + } + + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + if (isThisConstructorInvocation(tree)) { + mergeResolvedTarget(tree.methodSymbol()); + } else if (isInvocationOnThis(tree)) { + Symbol.MethodSymbol method = tree.methodSymbol(); + if (method.isUnknown()) { + complete = false; + } else if (!method.isStatic() && method.enclosingClass() == owner) { + mergeResolvedTarget(method); + } + } + // Arguments are executed in the current context and may contain assignments or helper calls. + super.visitMethodInvocation(tree); + } + + @Override + public void visitClass(ClassTree tree) { + // Local and anonymous class bodies are not executed as part of the current initialization path. + } + + @Override + public void visitLambdaExpression(LambdaExpressionTree tree) { + // Lambda bodies are not executed when the lambda is created. + } + + @Override + public void visitNewClass(NewClassTree tree) { + if (tree.enclosingExpression() != null) { + tree.enclosingExpression().accept(this); + } + tree.arguments().forEach(argument -> argument.accept(this)); + // Deliberately do not visit an anonymous class body. + } + + private void mergeResolvedTarget(Symbol.MethodSymbol method) { + if (method.isUnknown() || method.enclosingClass() != owner) { + complete = false; + return; + } + MethodTree declaration = method.declaration(); + if (declaration == null || declaration.block() == null) { + complete = false; + return; + } + AnalysisResult nested = analyze(declaration, owner, eligibleFields, activeMethods); + assignedFields.addAll(nested.assignedFields); + complete &= nested.complete; + } + + private Symbol currentInstanceField(ExpressionTree expression) { + ExpressionTree variable = ExpressionUtils.skipParentheses(expression); + if (variable instanceof IdentifierTree identifier) { + return eligibleFields.contains(identifier.symbol()) ? identifier.symbol() : null; + } + if (variable instanceof MemberSelectExpressionTree memberSelect + && ExpressionUtils.isThis(ExpressionUtils.skipParentheses(memberSelect.expression())) + && eligibleFields.contains(memberSelect.identifier().symbol())) { + return memberSelect.identifier().symbol(); + } + return null; + } + + private AnalysisResult result() { + return new AnalysisResult(Set.copyOf(assignedFields), complete); + } + + private static boolean isThisConstructorInvocation(MethodInvocationTree invocation) { + return invocation.methodSelect() instanceof IdentifierTree identifier && "this".equals(identifier.name()); + } + + private static boolean isInvocationOnThis(MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof IdentifierTree identifier) { + return !"super".equals(identifier.name()); + } + return invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect + && ExpressionUtils.isThis(ExpressionUtils.skipParentheses(memberSelect.expression())); + } + } + + private record AnalysisResult(Set assignedFields, boolean complete) { + private static AnalysisResult incomplete() { + return new AnalysisResult(Set.of(), false); + } + } +} diff --git a/java-checks/src/test/java/org/sonar/java/checks/CopyConstructorMissesFieldCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/CopyConstructorMissesFieldCheckTest.java new file mode 100644 index 00000000000..5cfa2236049 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/CopyConstructorMissesFieldCheckTest.java @@ -0,0 +1,53 @@ +/* + * SonarQube Java + * Copyright (C) SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * You can redistribute and/or modify this program under the terms of + * the Sonar Source-Available License Version 1, as published by SonarSource Sàrl. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. + * See the Sonar Source-Available License for more details. + * + * You should have received a copy of the Sonar Source-Available License + * along with this program; if not, see https://sonarsource.com/license/ssal/ + */ +package org.sonar.java.checks; + +import org.junit.jupiter.api.Test; +import org.sonar.java.checks.verifier.CheckVerifier; + +import static org.sonar.java.checks.verifier.TestUtils.mainCodeSourcesPath; +import static org.sonar.java.checks.verifier.TestUtils.nonCompilingTestSourcesPath; + +class CopyConstructorMissesFieldCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/CopyConstructorMissesFieldCheckSample.java")) + .withCheck(new CopyConstructorMissesFieldCheck()) + .withJavaVersion(17) + .verifyIssues(); + } + + @Test + void test_without_semantic() { + CheckVerifier.newVerifier() + .onFile(mainCodeSourcesPath("checks/CopyConstructorMissesFieldCheckSample.java")) + .withCheck(new CopyConstructorMissesFieldCheck()) + .withJavaVersion(17) + .withoutSemantic() + .verifyNoIssues(); + } + + @Test + void test_unresolved_initialization_paths() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/CopyConstructorMissesFieldCheckSample.java")) + .withCheck(new CopyConstructorMissesFieldCheck()) + .verifyNoIssues(); + } +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html new file mode 100644 index 00000000000..01592d408f5 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html @@ -0,0 +1,61 @@ +

A copy constructor creates an object from another object of the same class. It should explicitly initialize every eligible field that represents +the new object’s state.

+

Why is this an issue?

+

A class can evolve after its copy constructor is written. When a new field is added but the copy constructor is not updated, the copied object +silently keeps Java’s default value for that field, such as null, zero, or false. The result is an incomplete copy that can +behave differently from its source and cause failures far from the constructor.

+

This rule raises one issue on a constructor whose sole parameter has the same type as the enclosing class when one or more eligible fields are not +explicitly initialized. The declarations of the omitted fields are identified as secondary locations. Eligible fields are instance fields declared in +the enclosing class that are not transient and do not have declaration initializers.

+

Initialize each eligible field explicitly. An assignment of any value counts as initialization, including null, zero, or +false. Use such an explicit assignment when a field should intentionally be reset instead of copied.

+

Code examples

+

Noncompliant code example

+
+class Account {
+  private static int accountCount;
+  private String owner;
+  private String note;
+  private int retryCount;
+  private boolean enabled;
+  private transient Object cachedSession;
+  private Object auditData = new Object();
+
+  Account(Account other) { // Noncompliant: "note", "retryCount", and "enabled" are not explicitly initialized
+    this.owner = other.owner;
+  }
+}
+
+

Compliant solution

+
+class Account {
+  private static int accountCount;
+  private String owner;
+  private String note;
+  private int retryCount;
+  private boolean enabled;
+  private transient Object cachedSession;
+  private Object auditData = new Object();
+
+  Account(Account other) {
+    this.owner = other.owner;
+    this.note = null; // intentional reset
+    this.retryCount = 0; // intentional reset
+    this.enabled = false; // intentional reset
+  }
+}
+
+

Exceptions

+

Static fields do not belong to an individual object and are not considered. Fields with declaration initializers are already initialized +explicitly, and transient fields often represent state that should not be copied, so the rule does not require assignments for them.

+

The rule does not attempt to determine whether copying a field should be shallow or deep, or whether the value assigned to a field is correct. It +only checks whether each eligible field is explicitly initialized.

+

Assignments made directly, through constructor delegation, or through instance helper methods called on this count when the rule can +resolve the invoked code. It does not raise an issue when required semantic information or an invoked initialization path cannot be resolved.

+

Resources

+

Documentation

+ diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.json new file mode 100644 index 00000000000..687a6cd0c04 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.json @@ -0,0 +1,23 @@ +{ + "title": "Copy constructors should initialize all fields", + "type": "BUG", + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "suspicious" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-9365", + "sqKey": "S9365", + "scope": "Main", + "quickfix": "infeasible", + "code": { + "impacts": { + "RELIABILITY": "MEDIUM" + }, + "attribute": "COMPLETE" + } +} diff --git a/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9365 b/sonar-java-plugin/src/main/resources/profiles/Sonar_way/S9365 new file mode 100644 index 00000000000..e69de29bb2d From 088ac82fb0fafe2aef767dad5fe0852e5e0600ba Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 25 Aug 2026 10:20:19 +0200 Subject: [PATCH 2/5] Fix qualified this handling for S9365 --- ...CopyConstructorMissesFieldCheckSample.java | 20 +++++++++++++++++++ .../CopyConstructorMissesFieldCheck.java | 20 ++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java index 6a74e3c506f..7b44a99122d 100644 --- a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java @@ -292,4 +292,24 @@ static class DelegationChain { this.value = value; } } + + static class QualifiedDirectAssignment { + private int value; + + QualifiedDirectAssignment(QualifiedDirectAssignment other) { + QualifiedDirectAssignment.this.value = other.value; + } + } + + static class QualifiedHelperCall { + private int value; + + QualifiedHelperCall(QualifiedHelperCall other) { + QualifiedHelperCall.this.initialize(other); + } + + private void initialize(QualifiedHelperCall other) { + value = other.value; + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java index cd14069b194..a51b8b819f3 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java @@ -208,7 +208,7 @@ private Symbol currentInstanceField(ExpressionTree expression) { return eligibleFields.contains(identifier.symbol()) ? identifier.symbol() : null; } if (variable instanceof MemberSelectExpressionTree memberSelect - && ExpressionUtils.isThis(ExpressionUtils.skipParentheses(memberSelect.expression())) + && isCurrentInstance(memberSelect.expression()) && eligibleFields.contains(memberSelect.identifier().symbol())) { return memberSelect.identifier().symbol(); } @@ -223,12 +223,26 @@ private static boolean isThisConstructorInvocation(MethodInvocationTree invocati return invocation.methodSelect() instanceof IdentifierTree identifier && "this".equals(identifier.name()); } - private static boolean isInvocationOnThis(MethodInvocationTree invocation) { + private boolean isInvocationOnThis(MethodInvocationTree invocation) { if (invocation.methodSelect() instanceof IdentifierTree identifier) { return !"super".equals(identifier.name()); } return invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect - && ExpressionUtils.isThis(ExpressionUtils.skipParentheses(memberSelect.expression())); + && isCurrentInstance(memberSelect.expression()); + } + + private boolean isCurrentInstance(ExpressionTree expression) { + ExpressionTree receiver = ExpressionUtils.skipParentheses(expression); + if (receiver instanceof IdentifierTree identifier) { + // An unqualified `this` has no type binding in the syntax tree, but is unambiguous. + return "this".equals(identifier.name()); + } + if (!(receiver instanceof MemberSelectExpressionTree qualifiedThis) + || !ExpressionUtils.isThis(qualifiedThis.identifier())) { + return false; + } + Symbol thisSymbol = qualifiedThis.identifier().symbol(); + return !thisSymbol.isUnknown() && thisSymbol.enclosingClass() == owner; } } From 51097083c0c54deaf0ddfd92f2f4614a0a2514fd Mon Sep 17 00:00:00 2001 From: nathsou Date: Tue, 25 Aug 2026 10:31:07 +0200 Subject: [PATCH 3/5] Address review feedback for S9365 --- ...CopyConstructorMissesFieldCheckSample.java | 37 +++++++++++++-- .../CopyConstructorMissesFieldCheck.java | 46 +++++++++++++++++-- .../org/sonar/l10n/java/rules/java/S9365.html | 10 ++-- 3 files changed, 80 insertions(+), 13 deletions(-) diff --git a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java index 7b44a99122d..28dec80a266 100644 --- a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java +++ b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java @@ -75,11 +75,11 @@ static class CompoundAssignment { } } - static class IncrementIsNotAssignment { - private int value; + static class IncrementAndDecrement { + private int postfixIncrement, prefixIncrement, postfixDecrement, prefixDecrement; - IncrementIsNotAssignment(IncrementIsNotAssignment other) { // Noncompliant [[secondary=79]] - value++; + IncrementAndDecrement(IncrementAndDecrement other) { + postfixIncrement++; ++prefixIncrement; postfixDecrement--; --prefixDecrement; } } @@ -312,4 +312,33 @@ private void initialize(QualifiedHelperCall other) { value = other.value; } } + + static class InstanceInitializerAssignments { + private final int direct; + private int throughHelper; + + { + direct = 1; + initialize(); + } + + InstanceInitializerAssignments(InstanceInitializerAssignments other) { + } + + private void initialize() { + throughHelper = 1; + } + } + + static class GenericHelper { + private T value; + + GenericHelper(GenericHelper other) { + initialize(other); + } + + private void initialize(GenericHelper other) { + value = other.value; + } + } } diff --git a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java index a51b8b819f3..b5a78c6b848 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java @@ -41,6 +41,7 @@ import org.sonar.plugins.java.api.tree.Modifier; import org.sonar.plugins.java.api.tree.NewClassTree; import org.sonar.plugins.java.api.tree.Tree; +import org.sonar.plugins.java.api.tree.UnaryExpressionTree; import org.sonar.plugins.java.api.tree.VariableTree; @Rule(key = "S9365") @@ -87,7 +88,8 @@ public void visitNode(Tree tree) { return; } - AnalysisResult result = analyze(constructor, owner, eligibleFields.keySet(), new HashSet<>()); + AnalysisResult result = analyzeInitializers(classTree, owner, eligibleFields.keySet()) + .merge(analyze(constructor, owner, eligibleFields.keySet(), new HashSet<>())); if (!result.complete) { return; } @@ -113,11 +115,24 @@ private static Map eligibleFields(ClassTree classTree, Sym .filter(variable -> variable.initializer() == null) .filter(variable -> !ModifiersUtils.hasModifier(variable.modifiers(), Modifier.STATIC)) .filter(variable -> !ModifiersUtils.hasModifier(variable.modifiers(), Modifier.TRANSIENT)) - .filter(variable -> variable.symbol().owner() == owner) + .filter(variable -> owner.equals(variable.symbol().owner())) .forEach(variable -> fields.put(variable.symbol(), variable)); return fields; } + private static AnalysisResult analyzeInitializers(ClassTree classTree, Symbol.TypeSymbol owner, Set eligibleFields) { + AnalysisResult result = AnalysisResult.emptyComplete(); + Set activeMethods = new HashSet<>(); + for (Tree member : classTree.members()) { + if (member.is(Tree.Kind.INITIALIZER)) { + AssignmentCollector collector = new AssignmentCollector(owner, eligibleFields, activeMethods); + member.accept(collector); + result = result.merge(collector.result()); + } + } + return result; + } + private static AnalysisResult analyze(MethodTree method, Symbol.TypeSymbol owner, Set eligibleFields, Set activeMethods) { Symbol.MethodSymbol methodSymbol = method.symbol(); @@ -152,6 +167,17 @@ public void visitAssignmentExpression(AssignmentExpressionTree tree) { super.visitAssignmentExpression(tree); } + @Override + public void visitUnaryExpression(UnaryExpressionTree tree) { + if (tree.is(Tree.Kind.POSTFIX_INCREMENT, Tree.Kind.POSTFIX_DECREMENT, Tree.Kind.PREFIX_INCREMENT, Tree.Kind.PREFIX_DECREMENT)) { + Symbol assignedField = currentInstanceField(tree.expression()); + if (assignedField != null) { + assignedFields.add(assignedField); + } + } + super.visitUnaryExpression(tree); + } + @Override public void visitMethodInvocation(MethodInvocationTree tree) { if (isThisConstructorInvocation(tree)) { @@ -160,7 +186,7 @@ public void visitMethodInvocation(MethodInvocationTree tree) { Symbol.MethodSymbol method = tree.methodSymbol(); if (method.isUnknown()) { complete = false; - } else if (!method.isStatic() && method.enclosingClass() == owner) { + } else if (!method.isStatic() && owner.equals(method.enclosingClass())) { mergeResolvedTarget(method); } } @@ -188,7 +214,7 @@ public void visitNewClass(NewClassTree tree) { } private void mergeResolvedTarget(Symbol.MethodSymbol method) { - if (method.isUnknown() || method.enclosingClass() != owner) { + if (method.isUnknown() || !owner.equals(method.enclosingClass())) { complete = false; return; } @@ -242,13 +268,23 @@ private boolean isCurrentInstance(ExpressionTree expression) { return false; } Symbol thisSymbol = qualifiedThis.identifier().symbol(); - return !thisSymbol.isUnknown() && thisSymbol.enclosingClass() == owner; + return !thisSymbol.isUnknown() && owner.equals(thisSymbol.enclosingClass()); } } private record AnalysisResult(Set assignedFields, boolean complete) { + private static AnalysisResult emptyComplete() { + return new AnalysisResult(Set.of(), true); + } + private static AnalysisResult incomplete() { return new AnalysisResult(Set.of(), false); } + + private AnalysisResult merge(AnalysisResult other) { + Set mergedAssignments = new HashSet<>(assignedFields); + mergedAssignments.addAll(other.assignedFields); + return new AnalysisResult(Set.copyOf(mergedAssignments), complete && other.complete); + } } } diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html index 01592d408f5..520a935a190 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html @@ -7,8 +7,9 @@

Why is this an issue?

This rule raises one issue on a constructor whose sole parameter has the same type as the enclosing class when one or more eligible fields are not explicitly initialized. The declarations of the omitted fields are identified as secondary locations. Eligible fields are instance fields declared in the enclosing class that are not transient and do not have declaration initializers.

-

Initialize each eligible field explicitly. An assignment of any value counts as initialization, including null, zero, or -false. Use such an explicit assignment when a field should intentionally be reset instead of copied.

+

Initialize each eligible field explicitly. Any explicit write to an eligible field counts as initialization. This includes assignments of any +value, such as null, zero, or false, as well as increment and decrement operations. Use an explicit assignment when a field +should intentionally be reset instead of copied.

Code examples

Noncompliant code example

@@ -50,8 +51,9 @@ 
Exceptions

 explicitly, and transient fields often represent state that should not be copied, so the rule does not require assignments for them.

 
The rule does not attempt to determine whether copying a field should be shallow or deep, or whether the value assigned to a field is correct. It
 only checks whether each eligible field is explicitly initialized.

-
Assignments made directly, through constructor delegation, or through instance helper methods called on this count when the rule can
-resolve the invoked code. It does not raise an issue when required semantic information or an invoked initialization path cannot be resolved.

+
Explicit writes made in instance initializer blocks, directly in the constructor, through constructor delegation, or through instance helper
+methods called on this count when the rule can resolve the invoked code. It does not raise an issue when required semantic information or
+an invoked initialization path cannot be resolved.

 
Resources

 
Documentation

 
    

From 372a565ab4631655fba09eafdee8b4fa16b04cd8 Mon Sep 17 00:00:00 2001
From: nathsou 
Date: Tue, 25 Aug 2026 11:20:32 +0200
Subject: [PATCH 4/5] Address remaining S9365 review feedback

---
 ...CopyConstructorMissesFieldCheckSample.java | 42 +++++++++++++++++++
 .../CopyConstructorMissesFieldCheck.java      |  4 ++
 2 files changed, 46 insertions(+)

diff --git a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java
index 28dec80a266..cd951db520b 100644
--- a/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java
+++ b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java
@@ -341,4 +341,46 @@ private void initialize(GenericHelper other) {
       value = other.value;
     }
   }
+
+  static class QualifiedOuterThis {
+    private int outerValue;
+
+    class Inner {
+      private int innerValue;
+
+      Inner(Inner other) { // Noncompliant [[secondary=349]]
+        QualifiedOuterThis.this.outerValue = other.innerValue;
+      }
+    }
+  }
+
+  static class NestedReceiverIsNotThis {
+    private int value;
+    private NestedReceiverIsNotThis delegate = this;
+
+    NestedReceiverIsNotThis(NestedReceiverIsNotThis other) { // Noncompliant [[secondary=358]]
+      other.delegate.value = 1;
+    }
+  }
+
+  static class CastReceiver {
+    private int value;
+
+    CastReceiver(CastReceiver other) {
+      ((CastReceiver) this).initialize(other);
+    }
+
+    private void initialize(CastReceiver other) {
+      value = other.value;
+    }
+  }
+
+  static class RejectedUnaryWrites {
+    private int value;
+
+    RejectedUnaryWrites(RejectedUnaryWrites other) { // Noncompliant [[secondary=379]]
+      int ignored = -other.value;
+      other.value++;
+    }
+  }
 }
diff --git a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
index b5a78c6b848..06fd4262479 100644
--- a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
+++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
@@ -41,6 +41,7 @@
 import org.sonar.plugins.java.api.tree.Modifier;
 import org.sonar.plugins.java.api.tree.NewClassTree;
 import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.TypeCastTree;
 import org.sonar.plugins.java.api.tree.UnaryExpressionTree;
 import org.sonar.plugins.java.api.tree.VariableTree;
 
@@ -259,6 +260,9 @@ private boolean isInvocationOnThis(MethodInvocationTree invocation) {
 
     private boolean isCurrentInstance(ExpressionTree expression) {
       ExpressionTree receiver = ExpressionUtils.skipParentheses(expression);
+      while (receiver instanceof TypeCastTree cast) {
+        receiver = ExpressionUtils.skipParentheses(cast.expression());
+      }
       if (receiver instanceof IdentifierTree identifier) {
         // An unqualified `this` has no type binding in the syntax tree, but is unambiguous.
         return "this".equals(identifier.name());

From be56c968897c2d1041172e4fc8de45ad77fccd1e Mon Sep 17 00:00:00 2001
From: nathsou 
Date: Tue, 25 Aug 2026 14:23:47 +0200
Subject: [PATCH 5/5] Document S9365 assignment analysis

---
 .../checks/CopyConstructorMissesFieldCheck.java  | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
index 06fd4262479..526117cdbac 100644
--- a/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
+++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java
@@ -90,7 +90,7 @@ public void visitNode(Tree tree) {
     }
 
     AnalysisResult result = analyzeInitializers(classTree, owner, eligibleFields.keySet())
-      .merge(analyze(constructor, owner, eligibleFields.keySet(), new HashSet<>()));
+      .merge(analyzeMethod(constructor, owner, eligibleFields.keySet(), new HashSet<>()));
     if (!result.complete) {
       return;
     }
@@ -134,7 +134,11 @@ private static AnalysisResult analyzeInitializers(ClassTree classTree, Symbol.Ty
     return result;
   }
 
-  private static AnalysisResult analyze(MethodTree method, Symbol.TypeSymbol owner, Set eligibleFields,
+  /**
+   * Analyzes explicit field writes performed by a constructor or helper method, including writes reached through
+   * resolvable calls on the current instance. Active methods form the current call chain and prevent infinite recursion.
+   */
+  private static AnalysisResult analyzeMethod(MethodTree method, Symbol.TypeSymbol owner, Set eligibleFields,
     Set activeMethods) {
     Symbol.MethodSymbol methodSymbol = method.symbol();
     if (methodSymbol.isUnknown() || method.block() == null || !activeMethods.add(methodSymbol)) {
@@ -146,6 +150,12 @@ private static AnalysisResult analyze(MethodTree method, Symbol.TypeSymbol owner
     return collector.result();
   }
 
+  /**
+   * Collects explicit writes to eligible fields while following resolvable constructor and helper calls on the current
+   * instance. Eligible fields are instance fields declared by the analyzed class that are neither transient nor already
+   * initialized at their declaration. Active methods are the methods in the current call chain; they are tracked to
+   * detect recursive calls. An analysis is complete only when every followed initialization path can be resolved.
+   */
   private static final class AssignmentCollector extends BaseTreeVisitor {
     private final Symbol.TypeSymbol owner;
     private final Set eligibleFields;
@@ -224,7 +234,7 @@ private void mergeResolvedTarget(Symbol.MethodSymbol method) {
         complete = false;
         return;
       }
-      AnalysisResult nested = analyze(declaration, owner, eligibleFields, activeMethods);
+      AnalysisResult nested = analyzeMethod(declaration, owner, eligibleFields, activeMethods);
       assignedFields.addAll(nested.assignedFields);
       complete &= nested.complete;
     }