From 2e72dc7d20b7f911881caf4e212dbcee04106cbe Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Sun, 8 Feb 2026 16:42:14 +0100 Subject: [PATCH 1/7] SONARJAVA-6084 Validation logic should be placed in constructor prologue when possible --- ...eConstructorBodyValidationCheckSample.java | 147 +++++++++ java-checks/pom.xml | 4 + ...thOnlyStaticMethodsInstantiationCheck.java | 8 +- ...lexibleConstructorBodyValidationCheck.java | 286 ++++++++++++++++++ .../checks/ReplaceLambdaByMethodRefCheck.java | 6 +- ...bleConstructorBodyValidationCheckTest.java | 44 +++ .../org/sonar/java/model/ExpressionUtils.java | 4 + .../org/sonar/java/model/JavaVersionImpl.java | 8 +- .../sonar/plugins/java/api/JavaVersion.java | 8 + .../sonar/java/model/JavaVersionImplTest.java | 8 +- pom.xml | 4 +- .../org/sonar/l10n/java/rules/java/S8433.html | 52 ++++ .../org/sonar/l10n/java/rules/java/S8433.json | 24 ++ .../java/rules/java/Sonar_way_profile.json | 3 +- 14 files changed, 588 insertions(+), 18 deletions(-) create mode 100644 java-checks-test-sources/default/src/main/files/non-compiling/checks/FlexibleConstructorBodyValidationCheckSample.java create mode 100644 java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java create mode 100644 java-checks/src/test/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheckTest.java create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.html create mode 100644 sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.json 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 00000000000..3b5ff1562f6 --- /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/pom.xml b/java-checks/pom.xml index c72ccb9dad5..2c91c67f04c 100644 --- a/java-checks/pom.xml +++ b/java-checks/pom.xml @@ -13,6 +13,10 @@ SonarQube Java :: Checks Code Analyzer for Java :: Analyzer's checks + + 21 + + org.sonarsource.api.plugin 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 b5c984d5c37..f2257eb000a 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 00000000000..c20e2c54e26 --- /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 a080ebedc2a..14dbd04eb44 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 00000000000..c7f4327e752 --- /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 3bdcd5660d5..e014a31db98 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 fabb99699c5..e7012bd3813 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 6273048432f..8904279929e 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 f265249420b..2f24cd83698 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 791cfc826b9..91dade0ebef 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 00000000000..05f7ba9b6e0 --- /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 b6bce63ab8f..2661f07bb4a 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" ] } From 2535857ccc92e0f29ffd1c87d49dc6357ceef434 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 9 Feb 2026 09:55:28 +0100 Subject: [PATCH 2/7] SONARJAVA-6084 Validation logic should be placed in constructor prologue when possible --- java-checks/pom.xml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/java-checks/pom.xml b/java-checks/pom.xml index 2c91c67f04c..c72ccb9dad5 100644 --- a/java-checks/pom.xml +++ b/java-checks/pom.xml @@ -13,10 +13,6 @@ SonarQube Java :: Checks Code Analyzer for Java :: Analyzer's checks - - 21 - - org.sonarsource.api.plugin From 73db80a9d74f0e9eb342a919519a822d58c71c73 Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 9 Feb 2026 13:04:11 +0100 Subject: [PATCH 3/7] SONARJAVA-6084 Validation logic should be placed in constructor prologue when possible --- ...eConstructorBodyValidationCheckSample.java | 23 ++++++- ...lexibleConstructorBodyValidationCheck.java | 60 +++---------------- 2 files changed, 31 insertions(+), 52 deletions(-) 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 index 3b5ff1562f6..d4c7e5bb479 100644 --- 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 @@ -29,6 +29,7 @@ public SmallCoffee(int water, int milk) { if (totalVolume > maxCupCoffee) { // Compliant: validation before super() throw new IllegalArgumentException(); } + maxCupCoffee++; super(water, milk); } @@ -48,7 +49,12 @@ static class MediumCoffee extends Coffee { private String flavor; public MediumCoffee(int water, int milk) { + this.isValidFlavor(""); super(water, milk); + if (water + milk > MAX_SIZE) { // Noncompliant + throw new IllegalArgumentException(); + } + this.isValidFlavor(""); this.flavor = "Vanilla"; } @@ -72,6 +78,7 @@ private boolean isValidFlavor(String flavor) { } } + // Test using static members static class LargeCoffee extends Coffee { private String name; private static final int MAX_SIZE = 500; @@ -86,10 +93,14 @@ public LargeCoffee(int water, int milk) { public LargeCoffee(String name) { super(100, 100); this.name = name; - if (name == null) { // Compliant: Not reported - after field assignment + if (!isValidName(name)) { // NonCompliant throw new IllegalArgumentException(); } } + + private static boolean isValidName(String name) { + return name != null && !name.isEmpty(); + } } // Test with different validation libraries @@ -98,6 +109,11 @@ 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.}} } + + public GuavaCoffee(String name) { + com.google.common.base.Preconditions.checkNotNull(name); // Compliant + super(100, 50); + } } static class SpringCoffee extends Coffee { @@ -105,6 +121,11 @@ 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.}} } + + public SpringCoffee(String name) { + org.springframework.util.Assert.notNull(name, "Name must not be null"); // Compliant + super(100, 50); + } } // Test with implicit super() 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 index c20e2c54e26..62fa7a6fd78 100644 --- a/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java +++ b/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java @@ -34,7 +34,6 @@ 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; @@ -106,16 +105,15 @@ public void visitNode(Tree tree) { 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()). + * Find the index of an explicit super() or this() call in the constructor body. + * + * @param body the constructor body to search + * @return the index of the explicit super() or this() call, or -1 if no explicit call is found (implicit super()) */ private static int findConstructorCallIndex(BlockTree body) { List statements = body.body(); @@ -139,13 +137,12 @@ private static int findConstructorCallIndex(BlockTree body) { } /** - * Check if a statement is a validation statement (throws exception or calls validation method). + * Check if a statement is a validation statement (conditionally throws exception or calls validation method). + * + * @param statement the statement to check + * @return true if the statement is a validation statement, false otherwise */ 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()); @@ -180,15 +177,6 @@ private static boolean canBeMovedToPrologue(StatementTree statement, Set 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. */ @@ -212,16 +200,6 @@ private static class InstanceDependencyCheck extends BaseTreeVisitor { 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(); @@ -232,7 +210,7 @@ public void visitIdentifier(IdentifierTree tree) { } // Check if it's an instance field or method being accessed - if (symbol.isVariableSymbol() || symbol.isMethodSymbol()) { + if (symbol.isVariableSymbol()) { // This is likely an instance field accessed without 'this.' hasInstanceDependency = true; return; @@ -263,24 +241,4 @@ 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); - } - } } From b361bf88d2c4b0d34946f4ddecdecd48b1cc226e Mon Sep 17 00:00:00 2001 From: asya-vorobeva Date: Mon, 9 Feb 2026 13:05:14 +0100 Subject: [PATCH 4/7] SONARJAVA-6084 Validation logic should be placed in constructor prologue when possible --- .../main/resources/org/sonar/l10n/java/rules/java/S8433.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.html b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.html index 0536d612ef3..4c438b7604b 100644 --- a/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.html +++ b/sonar-java-plugin/src/main/resources/org/sonar/l10n/java/rules/java/S8433.html @@ -19,6 +19,7 @@

Noncompliant code example

public class SmallCoffee extends Coffee { private static final int MAX_CUP_VOLUME = 100; + int topping; public SmallCoffee(int water, int milk, String topping) { super(water, milk); // Noncompliant: constructing before validation @@ -34,6 +35,7 @@

Compliant solution

 public class SmallCoffee extends Coffee {
     private static final int MAX_CUP_VOLUME = 100;
+    int topping;
 
     public SmallCoffee(int water, int milk, String topping) {
         int totalVolume = water + milk;

From 70c684f1660ff2b31a42df86248f7f28949bc2d7 Mon Sep 17 00:00:00 2001
From: asya-vorobeva 
Date: Mon, 9 Feb 2026 15:21:01 +0100
Subject: [PATCH 5/7] SONARJAVA-6084 Validation logic should be placed in
 constructor prologue when possible

---
 ...lexibleConstructorBodyValidationCheck.java | 28 ++++++-------------
 1 file changed, 9 insertions(+), 19 deletions(-)

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
index 62fa7a6fd78..30dcf1f7154 100644
--- a/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java
+++ b/java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java
@@ -118,18 +118,11 @@ public void visitNode(Tree tree) {
   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;
-        }
+      if (statements.get(i) instanceof ExpressionStatementTree expressionStatementTree
+        && expressionStatementTree.expression() instanceof MethodInvocationTree methodInvocationTree
+        && methodInvocationTree.methodSelect() instanceof IdentifierTree identifierTree
+        && ExpressionUtils.isThisOrSuper(identifierTree.name())){
+        return i;
       }
     }
     // No explicit super() or this() call
@@ -143,16 +136,13 @@ private static int findConstructorCallIndex(BlockTree body) {
    * @return true if the statement is a validation statement, false otherwise
    */
   private static boolean isValidationStatement(StatementTree statement) {
-    if (statement.is(Tree.Kind.IF_STATEMENT)) {
-      IfStatementTree ifStatement = (IfStatementTree) statement;
+    if (statement instanceof IfStatementTree ifStatement) {
       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);
-      }
+    if (statement instanceof ExpressionStatementTree expressionStatementTree
+      && expressionStatementTree.expression() instanceof MethodInvocationTree methodInvocationTree) {
+      return VALIDATION_METHODS.matches(methodInvocationTree);
     }
 
     return false;

From f1e642de11b9b766932858d560cab57512a6b25a Mon Sep 17 00:00:00 2001
From: asya-vorobeva 
Date: Mon, 9 Feb 2026 16:04:05 +0100
Subject: [PATCH 6/7] SONARJAVA-6084 Validation logic should be placed in
 constructor prologue when possible

---
 .../checks/FlexibleConstructorBodyValidationCheckSample.java     | 1 +
 1 file changed, 1 insertion(+)

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
index d4c7e5bb479..c210b1762db 100644
--- 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
@@ -18,6 +18,7 @@ public SmallCoffee(int water, int milk, String topping) {
       int maxCupCoffee = 90;
       int totalVolume = water + milk;
       if (totalVolume > maxCupCoffee) { // Noncompliant {{Move this validation logic before the super() or this() call.}}
+    //^[el=+3;ec=7]
         throw new IllegalArgumentException();
       }
       this.topping = topping;

From 2becb3b875792fb0d236a0031d60a69e697a0cbe Mon Sep 17 00:00:00 2001
From: asya-vorobeva 
Date: Mon, 9 Feb 2026 17:09:15 +0100
Subject: [PATCH 7/7] SONARJAVA-6084 Validation logic should be placed in
 constructor prologue when possible

---
 .../src/test/resources/autoscan/diffs/diff_S8433.json       | 6 ++++++
 1 file changed, 6 insertions(+)
 create mode 100644 its/autoscan/src/test/resources/autoscan/diffs/diff_S8433.json

diff --git a/its/autoscan/src/test/resources/autoscan/diffs/diff_S8433.json b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8433.json
new file mode 100644
index 00000000000..76a9799af83
--- /dev/null
+++ b/its/autoscan/src/test/resources/autoscan/diffs/diff_S8433.json
@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S8433",
+  "hasTruePositives": true,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}
\ No newline at end of file