SONARPHP-1806 Fix S117 FP and S116 FN on PHP 8 constructor property promotion

php-checks/src/main/java/org/sonar/php/checks/FieldNameCheck.java

@@ -16,14 +16,15 @@
  */
 package org.sonar.php.checks;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.regex.Pattern;
 import org.sonar.check.Rule;
 import org.sonar.check.RuleProperty;
 import org.sonar.plugins.php.api.tree.Tree;
 import org.sonar.plugins.php.api.tree.Tree.Kind;
 import org.sonar.plugins.php.api.tree.declaration.ClassPropertyDeclarationTree;
+import org.sonar.plugins.php.api.tree.declaration.MethodDeclarationTree;
+import org.sonar.plugins.php.api.tree.declaration.ParameterTree;
 import org.sonar.plugins.php.api.tree.declaration.VariableDeclarationTree;
 import org.sonar.plugins.php.api.visitors.PHPSubscriptionCheck;
 
@@ -49,17 +50,39 @@ public void init() {
 
   @Override
   public List<Kind> nodesToVisit() {
-    return Collections.singletonList(Kind.CLASS_PROPERTY_DECLARATION);
+    return List.of(Kind.CLASS_PROPERTY_DECLARATION, Kind.METHOD_DECLARATION);
   }
 
   @Override
   public void visitNode(Tree tree) {
-    ClassPropertyDeclarationTree property = (ClassPropertyDeclarationTree) tree;
+    if (tree.is(Kind.CLASS_PROPERTY_DECLARATION)) {
+      checkClassPropertyDeclaration((ClassPropertyDeclarationTree) tree);
+    } else {
+      checkPromotedConstructorProperties((MethodDeclarationTree) tree);
+    }
+  }
+
+  private void checkClassPropertyDeclaration(ClassPropertyDeclarationTree property) {
     for (VariableDeclarationTree variableDeclarationTree : property.declarations()) {
-      String propertyName = variableDeclarationTree.identifier().text();
-      if (!pattern.matcher(propertyName.replace("$", "")).matches()) {
-        context().newIssue(this, variableDeclarationTree.identifier(), String.format(MESSAGE, propertyName, format));
+      checkPropertyName(variableDeclarationTree.identifier(), variableDeclarationTree.identifier().text());
+    }
+  }
+
+  private void checkPromotedConstructorProperties(MethodDeclarationTree method) {
+    if (!"__construct".equals(method.name().text())) {
+      return;
+    }
+    for (ParameterTree parameter : method.parameters().parameters()) {
+      if (!parameter.isPropertyPromotion()) {
+        continue;
       }
+      checkPropertyName(parameter.variableIdentifier(), parameter.variableIdentifier().variableExpression().text());
+    }
+  }
+
+  private void checkPropertyName(Tree identifier, String propertyName) {
+    if (!pattern.matcher(propertyName.replace("$", "")).matches()) {
+      context().newIssue(this, identifier, String.format(MESSAGE, propertyName, format));
     }
   }
 

php-checks/src/main/java/org/sonar/php/checks/LocalVariableAndParameterNameCheck.java

@@ -105,6 +105,10 @@ private void checkLocalVariable(Tree tree) {
 
   private void checkParameters(FunctionTree functionDec) {
     for (ParameterTree parameter : functionDec.parameters().parameters()) {
+      // Promoted properties are class fields, not parameters; naming is checked by S116 (FieldNameCheck)
+      if (parameter.isPropertyPromotion()) {
+        continue;
+      }
       String paramName = parameter.variableIdentifier().variableExpression().text();
       if (!isCompliant(paramName)) {
         reportIssue("parameter", parameter, paramName);

php-checks/src/test/java/org/sonar/php/checks/FieldNameCheckTest.java

@@ -16,7 +16,7 @@
  */
 package org.sonar.php.checks;
 
-import java.util.Collections;
+import java.util.List;
 import org.junit.jupiter.api.Test;
 import org.sonar.php.utils.PHPCheckTest;
 import org.sonar.plugins.php.CheckVerifier;
@@ -37,8 +37,13 @@ void defaultValue() throws Exception {
   @Test
   void custom() throws Exception {
     check.format = "^[A-Z][a-zA-Z0-9]*$";
-    String message = "Rename this field \"$myVariable\" to match the regular expression " + check.format + ".";
-    PHPCheckTest.check(check, TestUtils.getCheckFile(FILE_NAME),
-      Collections.singletonList(new LineIssue(check, 7, message)));
+    String format = check.format;
+    PHPCheckTest.check(check, TestUtils.getCheckFile(FILE_NAME), List.of(
+      new LineIssue(check, 7, "Rename this field \"$myVariable\" to match the regular expression " + format + "."),
+      new LineIssue(check, 12, "Rename this field \"$myField\" to match the regular expression " + format + "."),
+      new LineIssue(check, 15, "Rename this field \"$my_field\" to match the regular expression " + format + "."),
+      new LineIssue(check, 16, "Rename this field \"$myReadonly\" to match the regular expression " + format + "."),
+      new LineIssue(check, 17, "Rename this field \"$myFinalField\" to match the regular expression " + format + "."),
+      new LineIssue(check, 18, "Rename this field \"$MY_FINAL_FIELD\" to match the regular expression " + format + ".")));
   }
 }

php-checks/src/test/resources/checks/FieldNameCheck.php

@@ -6,3 +6,17 @@ class myClass {
   public $MyVariable = 1;  // Noncompliant
   public $myVariable;      // OK
 }
+
+class ConstructorPropertyPromotion {
+  public function __construct(
+    public string $myField,           // OK
+    protected int $MyField,           // Noncompliant {{Rename this field "$MyField" to match the regular expression ^[a-z][a-zA-Z0-9]*$.}}
+//                ^^^^^^^^
+    private string $my_field,         // Noncompliant {{Rename this field "$my_field" to match the regular expression ^[a-z][a-zA-Z0-9]*$.}}
+    public readonly string $myReadonly, // OK
+    final string $myFinalField,         // OK - final-only promoted property (PHP 8.5)
+    final string $MY_FINAL_FIELD,         // Noncompliant {{Rename this field "$MY_FINAL_FIELD" to match the regular expression ^[a-z][a-zA-Z0-9]*$.}}
+    string $regularParam,             // OK - not a promoted property
+    string $REGULAR_PARAM             // OK - not a promoted property, checked by S117
+  ) {}
+}

php-checks/src/test/resources/checks/LocalVariableAndParameterNameCheck.php

@@ -44,3 +44,14 @@ function f2() {
 
 $f = fn($param, $second_param) => $param * 2;
 $f = fn($PARAM) => $PARAM * 2; // Noncompliant
+
+class ConstructorPropertyPromotion {
+  public function __construct(
+    public string $myField,          // OK - promoted property, checked by S116 not S117
+    protected int $MY_FIELD,         // OK - promoted property, not subject to parameter naming
+    private string $entityTypeManager, // OK - promoted property
+    final string $MY_FINAL_FIELD,    // OK - final-only promoted property (PHP 8.5), not subject to parameter naming
+    string $regularParam,            // OK
+    string $REGULAR_PARAM,           // Noncompliant {{Rename this parameter "$REGULAR_PARAM" to match the regular expression ^[a-z_][a-zA-Z0-9_]*$.}}
+  ) {}
+}