SONARJAVA-6113 : implement rule S8444 : Excessive logic before super() should not bloat constructor

its/autoscan/src/test/java/org/sonar/java/it/AutoScanTest.java

@@ -199,7 +199,7 @@ public void javaCheckTestSources() throws Exception {
     softly.assertThat(newDiffs).containsExactlyInAnyOrderElementsOf(knownDiffs.values());
     softly.assertThat(newTotal).isEqualTo(knownTotal);
     softly.assertThat(rulesCausingFPs).hasSize(10);
-    softly.assertThat(rulesNotReporting).hasSize(15);
+    softly.assertThat(rulesNotReporting).hasSize(16);
 
     /**
      * 4. Check total number of differences (FPs + FNs)

its/autoscan/src/test/resources/autoscan/diffs/diff_S8444.json

@@ -0,0 +1,6 @@
+{
+  "ruleKey": "S8444",
+  "hasTruePositives": false,
+  "falseNegatives": 0,
+  "falsePositives": 0
+}

java-checks-test-sources/default/src/main/files/non-compiling/checks/PresuperLogicShoudntBloatConstructorSample.java

@@ -0,0 +1,144 @@
+package org.sonar.java.checks;
+
+public class PresuperLogicShoudntBloatConstructorSample {
+  public static class File {
+    public File(String path) {
+      // Simulate file initialization logic
+    }
+  }
+
+  public static class NonCompliantSecureFile extends File {
+    public NonCompliantSecureFile(String path) {
+      if (path == null || path.isBlank()) {  // Noncompliant {{Excessive logic in this "pre-construction" phase makes the code harder to read and maintain.}}
+   // ^[el=+19;ec=7]
+        throw new IllegalArgumentException("Path cannot be empty");
+      }
+      if (path.contains("..")) {
+        throw new IllegalArgumentException("Relative path traversal is forbidden");
+      }
+      if (path.startsWith("/root") || path.startsWith("/etc")) {
+        throw new SecurityException("Access to system directories is restricted");
+      }
+      if (path.length() > 255) {
+        throw new IllegalArgumentException("Path exceeds maximum length");
+      }
+      if (!path.matches("^[a-zA-Z0-9/._-]+$")) {
+        throw new IllegalArgumentException("Path contains illegal characters");
+      }
+      String sanitizedPath = path.trim().replace("//", "/");
+      if (sanitizedPath.endsWith("/")) {
+        sanitizedPath = sanitizedPath.substring(0, sanitizedPath.length() - 1);
+      }
+      super(sanitizedPath);
+    }
+  }
+
+  public static class NonCompliantSecureFileNestedStatements extends File {
+    public NonCompliantSecureFile(String path) {
+      if (true) {  // Noncompliant {{Excessive logic in this "pre-construction" phase makes the code harder to read and maintain.}}
+   // ^[el=+21;ec=7]
+        if (path == null || path.isBlank()) {
+          throw new IllegalArgumentException("Path cannot be empty");
+        }
+        if (path.contains("..")) {
+          throw new IllegalArgumentException("Relative path traversal is forbidden");
+        }
+        if (path.startsWith("/root") || path.startsWith("/etc")) {
+          throw new SecurityException("Access to system directories is restricted");
+        }
+        if (path.length() > 255) {
+          throw new IllegalArgumentException("Path exceeds maximum length");
+        }
+        if (!path.matches("^[a-zA-Z0-9/._-]+$")) {
+          throw new IllegalArgumentException("Path contains illegal characters");
+        }
+        String sanitizedPath = path.trim().replace("//", "/");
+        if (sanitizedPath.endsWith("/")) {
+          sanitizedPath = sanitizedPath.substring(0, sanitizedPath.length() - 1);
+        }
+      }
+      super(sanitizedPath);
+    }
+  }
+
+  public static class CompliantSecureFile extends File {
+    public CompliantSecureFile(String path) {
+      // Compliant: Logic is encapsulated in static helpers
+      validatePathSecurity(path);
+      validatePathFormat(path);
+      String sanitizedPath = normalizePath(path);
+      super(sanitizedPath);
+    }
+  }
+
+  public static class EdgeCaseSecureFile extends File {
+    public EdgeCaseSecureFile(String path) {
+      // Compliant: There are 3 statements before super() : if, throw, var declaration
+      if (path.length() > 255 || !path.matches("^[a-zA-Z0-9/._-]+$")) {
+        throw new IllegalArgumentException("Path format or length is invalid");
+      }
+      String sanitizedPath = normalizePath(path);
+      super(sanitizedPath);
+    }
+
+    public EdgeCaseSecureFile(String path, boolean b) {
+      // Non-compliant: There are 4 statements before super() : if, throw, var declaration, method call
+      validatePathSecurity(path); // Noncompliant {{Excessive logic in this "pre-construction" phase makes the code harder to read and maintain.}}
+   // ^[el=+5;ec=49]
+      if (path.length() > 255 || !path.matches("^[a-zA-Z0-9/._-]+$")) {
+        throw new IllegalArgumentException("Path format or length is invalid");
+      }
+      String sanitizedPath = normalizePath(path);
+      super(sanitizedPath);
+    }
+
+    public EdgeCaseSecureFile(String path, int i) {
+      // Compliant: There are 3 statements before super() : if, if, try-catch block
+      if (true) {
+        if (true) {
+          try {}
+          catch (Exception e) {}
+          finally {}
+        }
+      }
+      super(sanitizedPath);
+    }
+
+
+    public EdgeCaseSecureFile(String path, float f) {
+      // Compliant: There are 4 statements before super() : if, if, try-catch block, if
+      if (true) { // Noncompliant {{Excessive logic in this "pre-construction" phase makes the code harder to read and maintain.}}
+   // ^[el=+9;ec=7]
+        if (true) {
+          try {
+            if (true) {}
+          }
+          catch (Exception e) {}
+          finally {}
+        }
+      }
+      super(sanitizedPath);
+    }
+  }
+
+
+  private static void validatePathSecurity(String path) {
+    if (path == null || path.contains("..")) {
+      throw new IllegalArgumentException("Invalid or dangerous path sequence");
+    }
+    if (path.startsWith("/root") || path.startsWith("/etc")) {
+      throw new SecurityException("Access to system directories is restricted");
+    }
+  }
+
+  private static void validatePathFormat(String path) {
+    if (path.length() > 255 || !path.matches("^[a-zA-Z0-9/._-]+$")) {
+      throw new IllegalArgumentException("Path format or length is invalid");
+    }
+  }
+
+  private static String normalizePath(String path) {
+    String cleaned = path.trim().replace("//", "/");
+    return cleaned.endsWith("/") ? cleaned.substring(0, cleaned.length() - 1) : cleaned;
+  }
+}

java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorBodyValidationCheck.java

@@ -16,21 +16,15 @@
  */
 package org.sonar.java.checks;
 
-import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import java.util.stream.Collectors;
 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.semantic.Type;
 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;
@@ -40,9 +34,10 @@
 import org.sonar.plugins.java.api.tree.StatementTree;
 import org.sonar.plugins.java.api.tree.ThrowStatementTree;
 import org.sonar.plugins.java.api.tree.Tree;
+import org.sonar.plugins.java.api.tree.VariableTree;
 
 @Rule(key = "S8433")
-public class FlexibleConstructorBodyValidationCheck extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
+public class FlexibleConstructorBodyValidationCheck extends FlexibleConstructorVisitor {
 
   private static final MethodMatchers VALIDATION_METHODS = MethodMatchers.or(
     MethodMatchers.create()
@@ -68,69 +63,25 @@ public class FlexibleConstructorBodyValidationCheck extends IssuableSubscription
   );
 
   @Override
-  public List<Tree.Kind> 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<StatementTree> statements = body.body();
-    if (constructorCallIndex == statements.size() - 1
+  void validateConstructor(MethodTree constructor, List<StatementTree> body, int constructorCallIndex) {
+    if (constructorCallIndex == body.size() - 1
       || (constructorCallIndex == -1 && hasNoExplicitSuperClass(constructor))) {
       // No statements after constructor call or no superclass and no constructor call
       return;
     }
-
     // Collect constructor parameters for analysis
-    Set<Symbol> parameters = new HashSet<>();
-    constructor.parameters().forEach(param -> parameters.add(param.symbol()));
+    Set<Symbol> parameters = constructor.parameters().stream().map(VariableTree::symbol).collect(Collectors.toSet());
 
     // Analyze statements after the constructor call for movable validation
-    for (int i = constructorCallIndex + 1; i < statements.size(); i++) {
-      StatementTree statement = statements.get(i);
+    for (int i = constructorCallIndex + 1; i < body.size(); i++) {
+      StatementTree statement = body.get(i);
 
       if (isValidationStatement(statement) && canBeMovedToPrologue(statement, parameters)) {
         reportIssue(statement, "Move this validation logic before the super() or this() call.");
       }
     }
   }
 
-  /**
-   * 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<StatementTree> statements = body.body();
-    for (int i = 0; i < statements.size(); 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
-    return -1;
-  }
-
   private static boolean hasNoExplicitSuperClass(MethodTree constructor) {
     Type superClass = constructor.symbol().enclosingClass().superClass();
     return (superClass == null || superClass.is("java.lang.Object"));
@@ -202,7 +153,7 @@ 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)) {
+      if (symbol.isLocalVariable() || symbol.isStatic() || parameters.contains(symbol)) {
         return;
       }
 

java-checks/src/main/java/org/sonar/java/checks/FlexibleConstructorVisitor.java

@@ -0,0 +1,87 @@
+/*
+ * 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.List;
+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.tree.BlockTree;
+import org.sonar.plugins.java.api.tree.ExpressionStatementTree;
+import org.sonar.plugins.java.api.tree.IdentifierTree;
+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.Tree;
+
+public abstract class FlexibleConstructorVisitor extends IssuableSubscriptionVisitor implements JavaVersionAwareVisitor {
+
+  /**
+   * Validate the constructor body, providing the constructor method tree, the list of statements in the constructor body, and the index of any explicit super() or this() call
+   * (or -1 if no explicit call is found).
+   * @param constructor the constructor method tree being validated
+   * @param body the list of statements in the constructor body
+   * @param constructorCallIndex the index of any explicit super() or this() call in the body, or -1 if no explicit call is found (implicit super())
+   */
+  abstract void validateConstructor(MethodTree constructor, List<StatementTree> body, int constructorCallIndex);
+
+  @Override
+  public final List<Tree.Kind> nodesToVisit() {
+    return Collections.singletonList(Tree.Kind.CONSTRUCTOR);
+  }
+
+  @Override
+  public final boolean isCompatibleWithJavaVersion(JavaVersion version) {
+    return version.isJava25Compatible();
+  }
+
+  @Override
+  public final void visitNode(Tree tree) {
+    MethodTree constructor = (MethodTree) tree;
+    BlockTree block = constructor.block();
+    if (block == null || block.body().isEmpty()) {
+      // No body or empty body, nothing to validate
+      return;
+    }
+    List<StatementTree> body = block.body();
+
+    // Find the super() or this() call
+    int constructorCallIndex = findConstructorCallIndex(body);
+    validateConstructor(constructor, body, constructorCallIndex);
+  }
+
+  /**
+   * 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(List<StatementTree> body) {
+    for (int i = 0; i < body.size(); i++) {
+      if (body.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
+    return -1;
+  }
+}