diff --git a/java-checks-test-sources/default/src/main/files/non-compiling/checks/FlexibleConstructorBodyValidationCheckSample.java b/java-checks-test-sources/default/src/main/files/non-compiling/checks/FlexibleConstructorBodyValidationCheckSample.java new file mode 100644 index 0000000000..3b5ff1562f --- /dev/null +++ b/java-checks-test-sources/default/src/main/files/non-compiling/checks/FlexibleConstructorBodyValidationCheckSample.java @@ -0,0 +1,147 @@ +package checks; + +import java.util.Objects; + +class FlexibleConstructorBodyValidationCheckSample { + + static class Coffee { + Coffee() {} + Coffee(int water, int milk) {} + } + + static class SmallCoffee extends Coffee { + private String topping; + private int maxVolume = 100; + + public SmallCoffee(int water, int milk, String topping) { + super(water, milk); + int maxCupCoffee = 90; + int totalVolume = water + milk; + if (totalVolume > maxCupCoffee) { // Noncompliant {{Move this validation logic before the super() or this() call.}} + throw new IllegalArgumentException(); + } + this.topping = topping; + } + + public SmallCoffee(int water, int milk) { + int totalVolume = water + milk; + int maxCupCoffee = 90; + if (totalVolume > maxCupCoffee) { // Compliant: validation before super() + throw new IllegalArgumentException(); + } + super(water, milk); + } + + public SmallCoffee(int water) { + super(water, 0); + if (water > maxVolume) { // Compliant: Uses instance field + throw new IllegalArgumentException(); + } + } + + public SmallCoffee() { + super(0, 0); // Compliant: no validation + } + } + + static class MediumCoffee extends Coffee { + private String flavor; + + public MediumCoffee(int water, int milk) { + super(water, milk); + this.flavor = "Vanilla"; + } + + public MediumCoffee(int water) { + if (water < 0) { // Compliant: throw before this() + throw new IllegalArgumentException("Water cannot be negative"); + } + this(water, 0); + } + + public MediumCoffee(String flavor) { + this(100, 50); + if (!isValidFlavor(flavor)) { // Compliant: validation uses instance method, cannot be moved + throw new IllegalArgumentException(); + } + this.flavor = flavor; + } + + private boolean isValidFlavor(String flavor) { + return flavor != null && !flavor.isEmpty(); + } + } + + static class LargeCoffee extends Coffee { + private String name; + private static final int MAX_SIZE = 500; + + public LargeCoffee(int water, int milk) { + super(water, milk); + if (water + milk > MAX_SIZE) { // Noncompliant + throw new IllegalArgumentException(); + } + } + + public LargeCoffee(String name) { + super(100, 100); + this.name = name; + if (name == null) { // Compliant: Not reported - after field assignment + throw new IllegalArgumentException(); + } + } + } + + // Test with different validation libraries + static class GuavaCoffee extends Coffee { + public GuavaCoffee(String name) { + super(100, 50); + com.google.common.base.Preconditions.checkNotNull(name); // Noncompliant {{Move this validation logic before the super() or this() call.}} + } + } + + static class SpringCoffee extends Coffee { + public SpringCoffee(String name) { + super(100, 50); + org.springframework.util.Assert.notNull(name, "Name must not be null"); // Noncompliant {{Move this validation logic before the super() or this() call.}} + } + } + + // Test with implicit super() + static class ImplicitSuperCoffee extends Coffee { + public ImplicitSuperCoffee(int water) { + // Implicit super() call here + if (water < 0) { // Noncompliant + throw new IllegalArgumentException(); + } + } + } + + // Test multiple validations + static class MultiValidationCoffee extends Coffee { + public MultiValidationCoffee(int water, int milk, String name) { + super(water, milk); + if (water < 0) { // Noncompliant + throw new IllegalArgumentException("Invalid water"); + } + if (milk < 0) { // Noncompliant + throw new IllegalArgumentException("Invalid milk"); + } + Objects.requireNonNull(name); // Noncompliant + } + } + + // Test nested if statements + static class NestedIfCoffee extends Coffee { + public NestedIfCoffee(int water, int milk) { + super(water, milk); + if (water > 0) { // Noncompliant + if (milk > 0) { + if (water + milk > 200) { + throw new IllegalArgumentException(); + } + } + } + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/ClassWithOnlyStaticMethodsInstantiationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ClassWithOnlyStaticMethodsInstantiationCheck.java index b5c984d5c3..f2257eb000 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ClassWithOnlyStaticMethodsInstantiationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ClassWithOnlyStaticMethodsInstantiationCheck.java @@ -17,6 +17,7 @@ package org.sonar.java.checks; import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; import org.sonar.java.model.JUtils; import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; import org.sonar.plugins.java.api.semantic.Symbol; @@ -93,18 +94,13 @@ private static boolean superTypesHaveOnlyStaticMethods(Symbol.TypeSymbol newClas private static Collection filterMethodsAndFields(Collection symbols) { List filtered = new ArrayList<>(); for (Symbol symbol : symbols) { - if ((symbol.isVariableSymbol() && !isThisOrSuper(symbol)) || (symbol.isMethodSymbol() && !isConstructor(symbol))) { + if ((symbol.isVariableSymbol() && !ExpressionUtils.isThisOrSuper(symbol.name())) || (symbol.isMethodSymbol() && !isConstructor(symbol))) { filtered.add(symbol); } } return filtered; } - private static boolean isThisOrSuper(Symbol symbol) { - String name = symbol.name(); - return "this".equals(name) || "super".equals(name); - } - private static boolean isConstructor(Symbol symbol) { return "".equals(symbol.name()); } diff --git a/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java b/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java new file mode 100644 index 0000000000..c20e2c54e2 --- /dev/null +++ b/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java @@ -0,0 +1,286 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * 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.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +import org.sonar.check.Rule; +import org.sonar.java.model.ExpressionUtils; +import org.sonar.plugins.java.api.IssuableSubscriptionVisitor; +import org.sonar.plugins.java.api.JavaVersion; +import org.sonar.plugins.java.api.JavaVersionAwareVisitor; +import org.sonar.plugins.java.api.semantic.MethodMatchers; +import org.sonar.plugins.java.api.semantic.Symbol; +import org.sonar.plugins.java.api.tree.BaseTreeVisitor; +import org.sonar.plugins.java.api.tree.BlockTree; +import org.sonar.plugins.java.api.tree.ExpressionStatementTree; +import org.sonar.plugins.java.api.tree.ExpressionTree; +import org.sonar.plugins.java.api.tree.IdentifierTree; +import org.sonar.plugins.java.api.tree.IfStatementTree; +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.StatementTree; +import org.sonar.plugins.java.api.tree.ThrowStatementTree; +import org.sonar.plugins.java.api.tree.Tree; + +@Rule(key = "S8433") +public class FlexibleConstructorBodyValidationCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor { + + private static final MethodMatchers VALIDATION_METHODS = MethodMatchers.or( + MethodMatchers.create() + .ofTypes("java.util.Objects") + .names("requireNonNull", "requireNonNullElse", "requireNonNullElseGet", "checkIndex", "checkFromToIndex", "checkFromIndexSize") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("com.google.common.base.Preconditions") + .names("checkNotNull", "checkArgument", "checkState") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("org.springframework.util.Assert") + .names("notNull", "isTrue", "state", "hasLength", "hasText", "notEmpty") + .withAnyParameters() + .build(), + MethodMatchers.create() + .ofTypes("org.apache.commons.lang3.Validate") + .names("notNull", "isTrue", "notEmpty", "notBlank") + .withAnyParameters() + .build() + ); + + @Override + public List nodesToVisit() { + return Collections.singletonList(Tree.Kind.CONSTRUCTOR); + } + + @Override + public boolean isCompatibleWithJavaVersion(JavaVersion version) { + return version.isJava25Compatible(); + } + + @Override + public void visitNode(Tree tree) { + MethodTree constructor = (MethodTree) tree; + BlockTree body = constructor.block(); + + if (body == null || body.body().isEmpty()) { + return; + } + + // Find the super() or this() call + int constructorCallIndex = findConstructorCallIndex(body); + + // Get statements after the constructor call + List statements = body.body(); + if (constructorCallIndex == statements.size() - 1) { + // No statements after constructor call + return; + } + + // Collect constructor parameters for analysis + Set parameters = new HashSet<>(); + constructor.parameters().forEach(param -> parameters.add(param.symbol())); + + // Analyze statements after the constructor call for movable validation + for (int i = constructorCallIndex + 1; i < statements.size(); i++) { + StatementTree statement = statements.get(i); + + if (isValidationStatement(statement) && canBeMovedToPrologue(statement, parameters)) { + reportIssue(statement, "Move this validation logic before the super() or this() call."); + } else if (containsFieldAssignment(statement)) { + // Stop analysis if we encounter a field assignment (indicates validation should not be moved) + break; + } + } + } + + /** + * Find the index of an explicit super() / this() call in the constructor body. + * Returns -1 if no explicit call is found (implicit super()). + */ + private static int findConstructorCallIndex(BlockTree body) { + List statements = body.body(); + for (int i = 0; i < statements.size(); i++) { + StatementTree statement = statements.get(i); + if (!statement.is(Tree.Kind.EXPRESSION_STATEMENT) + || !(((ExpressionStatementTree) statement).expression()).is(Tree.Kind.METHOD_INVOCATION)) { + continue; + } + MethodInvocationTree invocation = (MethodInvocationTree) ((ExpressionStatementTree) statement).expression(); + ExpressionTree methodSelect = invocation.methodSelect(); + if (methodSelect.is(Tree.Kind.IDENTIFIER)) { + String methodName = ((IdentifierTree) methodSelect).name(); + if (ExpressionUtils.isThisOrSuper(methodName)) { + return i; + } + } + } + // No explicit super() or this() call + return -1; + } + + /** + * Check if a statement is a validation statement (throws exception or calls validation method). + */ + private static boolean isValidationStatement(StatementTree statement) { + if (statement.is(Tree.Kind.THROW_STATEMENT)) { + return true; + } + + if (statement.is(Tree.Kind.IF_STATEMENT)) { + IfStatementTree ifStatement = (IfStatementTree) statement; + return containsThrow(ifStatement.thenStatement()); + } + + if (statement.is(Tree.Kind.EXPRESSION_STATEMENT)) { + ExpressionTree expression = ((ExpressionStatementTree) statement).expression(); + if (expression.is(Tree.Kind.METHOD_INVOCATION)) { + return VALIDATION_METHODS.matches((MethodInvocationTree) expression); + } + } + + return false; + } + + /** + * Check if a statement contains a throw statement in its body. + */ + private static boolean containsThrow(Tree tree) { + ThrowFinder finder = new ThrowFinder(); + tree.accept(finder); + return finder.foundThrow; + } + + /** + * Check if validation logic can be safely moved to constructor prologue. + * It can be moved if it only uses parameters, local variables, or static members. + */ + private static boolean canBeMovedToPrologue(StatementTree statement, Set parameters) { + InstanceDependencyCheck checker = new InstanceDependencyCheck(parameters); + statement.accept(checker); + return !checker.hasInstanceDependency; + } + + /** + * Check if statement contains field assignment (this.field = value). + */ + private static boolean containsFieldAssignment(StatementTree statement) { + FieldAssignmentFinder finder = new FieldAssignmentFinder(); + statement.accept(finder); + return finder.foundFieldAssignment; + } + + /** + * Visitor to find throw statements. + */ + private static class ThrowFinder extends BaseTreeVisitor { + boolean foundThrow = false; + + @Override + public void visitThrowStatement(ThrowStatementTree tree) { + foundThrow = true; + } + } + + /** + * Visitor to check if code depends on instance members (fields or methods). + */ + private static class InstanceDependencyCheck extends BaseTreeVisitor { + private final Set parameters; + boolean hasInstanceDependency = false; + + InstanceDependencyCheck(Set parameters) { + this.parameters = parameters; + } + + @Override + public void visitMemberSelectExpression(MemberSelectExpressionTree tree) { + if (ExpressionUtils.isSelectOnThisOrSuper((tree))) { + hasInstanceDependency = true; + return; + } + + super.visitMemberSelectExpression(tree); + } + + @Override + public void visitIdentifier(IdentifierTree tree) { + Symbol symbol = tree.symbol(); + + // Allow parameters, local variables and static fields / methods + if (symbol.isLocalVariable() || symbol.isStatic()|| parameters.contains(symbol)) { + return; + } + + // Check if it's an instance field or method being accessed + if (symbol.isVariableSymbol() || symbol.isMethodSymbol()) { + // This is likely an instance field accessed without 'this.' + hasInstanceDependency = true; + return; + } + + super.visitIdentifier(tree); + } + + @Override + public void visitMethodInvocation(MethodInvocationTree tree) { + ExpressionTree methodSelect = tree.methodSelect(); + + // Check for implicit this (method call without any qualifier) + if (methodSelect.is(Tree.Kind.IDENTIFIER)) { + Symbol.MethodSymbol methodSymbol = tree.methodSymbol(); + if (!methodSymbol.isStatic() && !methodSymbol.isUnknown()) { + // This is an instance method call + hasInstanceDependency = true; + return; + } + } + + super.visitMethodInvocation(tree); + } + + @Override + public void visitThrowStatement(ThrowStatementTree tree) { + // skip throw statements + } + } + + /** + * Visitor to find field assignments. + */ + private static class FieldAssignmentFinder extends BaseTreeVisitor { + boolean foundFieldAssignment = false; + + @Override + public void visitMemberSelectExpression(MemberSelectExpressionTree tree) { + // Check if this is a field assignment (this.field = ...) + if (ExpressionUtils.isThis(tree.expression())) { + Tree parent = tree.parent(); + if (parent != null && parent.is(Tree.Kind.ASSIGNMENT)) { + foundFieldAssignment = true; + return; + } + } + super.visitMemberSelectExpression(tree); + } + } +} diff --git a/java-checks/src/main/java/org/sonar/java/checks/ReplaceLambdaByMethodRefCheck.java b/java-checks/src/main/java/org/sonar/java/checks/ReplaceLambdaByMethodRefCheck.java index a080ebedc2..14dbd04eb4 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/ReplaceLambdaByMethodRefCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/ReplaceLambdaByMethodRefCheck.java @@ -356,14 +356,10 @@ private static boolean hasNonFinalFieldInMethodSelect(MethodInvocationTree mit) } return symbol != null && symbol.owner().isTypeSymbol() - && !isThisOrSuper(symbol.name()) + && !ExpressionUtils.isThisOrSuper(symbol.name()) && !symbol.isFinal(); } - private static boolean isThisOrSuper(String name) { - return "this".equals(name) || "super".equals(name); - } - private static boolean matchingParameters(Arguments arguments, List parameters) { return arguments.size() == parameters.size() && IntStream.range(0, arguments.size()).allMatch(i -> { diff --git a/java-checks/src/test/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheckTest.java b/java-checks/src/test/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheckTest.java new file mode 100644 index 0000000000..c7f4327e75 --- /dev/null +++ b/java-checks/src/test/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheckTest.java @@ -0,0 +1,44 @@ +/* + * SonarQube Java + * Copyright (C) 2012-2025 SonarSource Sàrl + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the Sonar Source-Available License Version 1, as published by SonarSource SA. + * + * 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.nonCompilingTestSourcesPath; + +class FlexibleConstructorBodyValidationCheckTest { + + @Test + void test() { + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/FlexibleConstructorBodyValidationCheckSample.java")) + .withCheck(new FlexibleConstructorBodyValidationCheck()) + .withJavaVersion(25) + .verifyIssues(); + } + + @Test + void test_java_24() { + // Should not raise issues on Java 24 and below (feature not available) + CheckVerifier.newVerifier() + .onFile(nonCompilingTestSourcesPath("checks/FlexibleConstructorBodyValidationCheckSample.java")) + .withCheck(new FlexibleConstructorBodyValidationCheck()) + .withJavaVersion(24) + .verifyNoIssues(); + } +} diff --git a/java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java b/java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java index 3bdcd5660d..e014a31db9 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java +++ b/java-frontend/src/main/java/org/sonar/java/model/ExpressionUtils.java @@ -89,6 +89,10 @@ public static boolean isSelectOnThisOrSuper(MemberSelectExpressionTree tree) { return "this".equalsIgnoreCase(selectSourceName) || "super".equalsIgnoreCase(selectSourceName); } + public static boolean isThisOrSuper(String name) { + return "this".equals(name) || "super".equals(name); + } + public static IdentifierTree extractIdentifier(AssignmentExpressionTree tree) { Optional identifier = extractIdentifier(tree.variable()); diff --git a/java-frontend/src/main/java/org/sonar/java/model/JavaVersionImpl.java b/java-frontend/src/main/java/org/sonar/java/model/JavaVersionImpl.java index fabb99699c..e7012bd381 100644 --- a/java-frontend/src/main/java/org/sonar/java/model/JavaVersionImpl.java +++ b/java-frontend/src/main/java/org/sonar/java/model/JavaVersionImpl.java @@ -44,7 +44,8 @@ public class JavaVersionImpl implements JavaVersion { private static final int JAVA_22 = 22; private static final int JAVA_23 = 23; private static final int JAVA_24 = 24; - public static final int MAX_SUPPORTED = JAVA_24; + private static final int JAVA_25 = 25; + public static final int MAX_SUPPORTED = JAVA_25; private final int javaVersion; private final boolean previewFeaturesEnabled; @@ -167,6 +168,11 @@ public boolean isJava24Compatible() { return JAVA_24 <= javaVersion; } + @Override + public boolean isJava25Compatible() { + return JAVA_25 <= javaVersion; + } + private boolean notSetOrAtLeast(int requiredJavaVersion) { return isNotSet() || requiredJavaVersion <= javaVersion; } diff --git a/java-frontend/src/main/java/org/sonar/plugins/java/api/JavaVersion.java b/java-frontend/src/main/java/org/sonar/plugins/java/api/JavaVersion.java index 6273048432..8904279929 100644 --- a/java-frontend/src/main/java/org/sonar/plugins/java/api/JavaVersion.java +++ b/java-frontend/src/main/java/org/sonar/plugins/java/api/JavaVersion.java @@ -165,6 +165,14 @@ public interface JavaVersion { */ boolean isJava24Compatible(); + /** + * Test if java version of the project is greater than or equal to 25. + * Remark - Contrary to other isJava*Compatible methods, this one will NOT return true if version is not set + * @return true if java version used is >= 25 + * @since SonarJava 8.10: Support of Java 25 + */ + boolean isJava25Compatible(); + /** * get java version as integer * @return an int representing the java version diff --git a/java-frontend/src/test/java/org/sonar/java/model/JavaVersionImplTest.java b/java-frontend/src/test/java/org/sonar/java/model/JavaVersionImplTest.java index f265249420..2f24cd8369 100644 --- a/java-frontend/src/test/java/org/sonar/java/model/JavaVersionImplTest.java +++ b/java-frontend/src/test/java/org/sonar/java/model/JavaVersionImplTest.java @@ -53,11 +53,12 @@ void no_version_set() { assertThat(version.isJava22Compatible()).isFalse(); assertThat(version.isJava23Compatible()).isFalse(); assertThat(version.isJava24Compatible()).isFalse(); + assertThat(version.isJava25Compatible()).isFalse(); assertThat(version.asInt()).isEqualTo(-1); } @ParameterizedTest(name = "JavaVersion: \"{0}\"") - @ValueSource(ints = {5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 42}) + @ValueSource(ints = {5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 42}) void java_versions(int javaVersionAsInt) { JavaVersion version = new JavaVersionImpl(javaVersionAsInt); assertThat(version.isSet()).isTrue(); @@ -79,6 +80,7 @@ void java_versions(int javaVersionAsInt) { assertThat(version.isJava22Compatible()).isEqualTo(javaVersionAsInt >= 22); assertThat(version.isJava23Compatible()).isEqualTo(javaVersionAsInt >= 23); assertThat(version.isJava24Compatible()).isEqualTo(javaVersionAsInt >= 24); + assertThat(version.isJava25Compatible()).isEqualTo(javaVersionAsInt >= 25); assertThat(version.asInt()).isEqualTo(javaVersionAsInt); } @@ -99,9 +101,9 @@ void compatibilityMesssages() { @Test void test_effective_java_version() { - assertThat(new JavaVersionImpl().effectiveJavaVersionAsString()).isEqualTo("24"); + assertThat(new JavaVersionImpl().effectiveJavaVersionAsString()).isEqualTo("25"); assertThat(new JavaVersionImpl(10).effectiveJavaVersionAsString()).isEqualTo("10"); - assertThat(new JavaVersionImpl(-1).effectiveJavaVersionAsString()).isEqualTo("24"); + assertThat(new JavaVersionImpl(-1).effectiveJavaVersionAsString()).isEqualTo("25"); } @Test diff --git a/pom.xml b/pom.xml index 791cfc826b..91dade0ebe 100644 --- a/pom.xml +++ b/pom.xml @@ -311,7 +311,7 @@ de.thetaphi forbiddenapis - 3.6 + 3.10 org.apache.maven.plugins @@ -344,7 +344,7 @@ check - false + true Why is this an issue? +

Java 25 introduces Flexible Constructor Bodies, which allow statements to be placed before an explicit constructor invocation - the super(…​) or +this(…​) call. This enables developers to perform validation logic in the prologue, ensuring that the system avoids the overhead of initializing a +superclass or allocating resources for an invalid object.

+

How to fix it

+

Move all the validation logic for parameters in a constructor before super(…​) or this(…​) call.

+

Code examples

+

Noncompliant code example

+
+public class Coffee {
+    int water;
+    int milk;
+
+    public Coffee(int water, int milk) {
+        this.water = water;
+        this.milk = milk;
+    }
+}
+
+public class SmallCoffee extends Coffee {
+    private static final int MAX_CUP_VOLUME = 100;
+
+    public SmallCoffee(int water, int milk, String topping) {
+        super(water, milk); // Noncompliant: constructing before validation
+        int totalVolume = water + milk;
+        if (totalVolume > MAX_CUP_VOLUME) {
+            throw new IllegalArgumentException();
+        }
+        this.topping = topping;
+    }
+}
+
+

Compliant solution

+
+public class SmallCoffee extends Coffee {
+    private static final int MAX_CUP_VOLUME = 100;
+
+    public SmallCoffee(int water, int milk, String topping) {
+        int totalVolume = water + milk;
+        if (totalVolume > MAX_CUP_VOLUME) {
+            throw new IllegalArgumentException();
+        }
+        super(water, milk); // Compliant: validation before construction
+        this.topping = topping;
+    }
+}
+
+

Resources

+ + diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.json new file mode 100644 index 0000000000..05f7ba9b6e --- /dev/null +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.json @@ -0,0 +1,24 @@ +{ + "title": "Validation logic should be placed in constructor prologue when possible", + "type": "CODE_SMELL", + "code": { + "impacts": { + "MAINTAINABILITY": "MEDIUM" + }, + "attribute": "CONVENTIONAL" + }, + "status": "ready", + "remediation": { + "func": "Constant\/Issue", + "constantCost": "5min" + }, + "tags": [ + "java25", + "suspicious" + ], + "defaultSeverity": "Major", + "ruleSpecification": "RSPEC-8433", + "sqKey": "S8433", + "scope": "All", + "quickfix": "unknown" +} diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json index b6bce63ab8..2661f07bb4 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/Sonar_way_profile.json @@ -517,6 +517,7 @@ "S7481", "S7482", "S7629", - "S8346" + "S8346", + "S8433" ] }