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..cd951db520b --- /dev/null +++ b/java-checks-test-sources/default/src/main/java/checks/CopyConstructorMissesFieldCheckSample.java @@ -0,0 +1,386 @@ +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 IncrementAndDecrement { + private int postfixIncrement, prefixIncrement, postfixDecrement, prefixDecrement; + + IncrementAndDecrement(IncrementAndDecrement other) { + postfixIncrement++; ++prefixIncrement; postfixDecrement--; --prefixDecrement; + } + } + + 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; + } + } + + 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; + } + } + + 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; + } + } + + 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 new file mode 100644 index 00000000000..526117cdbac --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/CopyConstructorMissesFieldCheck.java @@ -0,0 +1,304 @@ +/* + * 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.TypeCastTree; +import org.sonar.plugins.java.api.tree.UnaryExpressionTree; +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 = analyzeInitializers(classTree, owner, eligibleFields.keySet()) + .merge(analyzeMethod(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 -> 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; + } + + /** + * 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)) { + return AnalysisResult.incomplete(); + } + AssignmentCollector collector = new AssignmentCollector(owner, eligibleFields, activeMethods); + method.block().accept(collector); + activeMethods.remove(methodSymbol); + 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; + 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 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)) { + mergeResolvedTarget(tree.methodSymbol()); + } else if (isInvocationOnThis(tree)) { + Symbol.MethodSymbol method = tree.methodSymbol(); + if (method.isUnknown()) { + complete = false; + } else if (!method.isStatic() && owner.equals(method.enclosingClass())) { + 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() || !owner.equals(method.enclosingClass())) { + complete = false; + return; + } + MethodTree declaration = method.declaration(); + if (declaration == null || declaration.block() == null) { + complete = false; + return; + } + AnalysisResult nested = analyzeMethod(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 + && isCurrentInstance(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 boolean isInvocationOnThis(MethodInvocationTree invocation) { + if (invocation.methodSelect() instanceof IdentifierTree identifier) { + return !"super".equals(identifier.name()); + } + return invocation.methodSelect() instanceof MemberSelectExpressionTree memberSelect + && isCurrentInstance(memberSelect.expression()); + } + + 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()); + } + if (!(receiver instanceof MemberSelectExpressionTree qualifiedThis) + || !ExpressionUtils.isThis(qualifiedThis.identifier())) { + return false; + } + Symbol thisSymbol = qualifiedThis.identifier().symbol(); + 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/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..520a935a190 --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S9365.html @@ -0,0 +1,63 @@ +

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. 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

+
+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.

+

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

+ 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