diff --git a/php-checks/src/main/java/org/sonar/php/checks/FieldNameCheck.java b/php-checks/src/main/java/org/sonar/php/checks/FieldNameCheck.java index 1153d688f..a46d1d5f0 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/FieldNameCheck.java +++ b/php-checks/src/main/java/org/sonar/php/checks/FieldNameCheck.java @@ -16,7 +16,6 @@ */ package org.sonar.php.checks; -import java.util.Collections; import java.util.List; import java.util.regex.Pattern; import org.sonar.check.Rule; @@ -24,6 +23,8 @@ 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 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)); } } diff --git a/php-checks/src/main/java/org/sonar/php/checks/LocalVariableAndParameterNameCheck.java b/php-checks/src/main/java/org/sonar/php/checks/LocalVariableAndParameterNameCheck.java index 1eb8f4ce9..cb862a903 100644 --- a/php-checks/src/main/java/org/sonar/php/checks/LocalVariableAndParameterNameCheck.java +++ b/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); diff --git a/php-checks/src/test/java/org/sonar/php/checks/FieldNameCheckTest.java b/php-checks/src/test/java/org/sonar/php/checks/FieldNameCheckTest.java index 687bd8c4e..65d658aa9 100644 --- a/php-checks/src/test/java/org/sonar/php/checks/FieldNameCheckTest.java +++ b/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 + "."))); } } diff --git a/php-checks/src/test/resources/checks/FieldNameCheck.php b/php-checks/src/test/resources/checks/FieldNameCheck.php index b1cac6111..8c64fce29 100644 --- a/php-checks/src/test/resources/checks/FieldNameCheck.php +++ b/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 + ) {} +} diff --git a/php-checks/src/test/resources/checks/LocalVariableAndParameterNameCheck.php b/php-checks/src/test/resources/checks/LocalVariableAndParameterNameCheck.php index 240974418..df5fa038e 100644 --- a/php-checks/src/test/resources/checks/LocalVariableAndParameterNameCheck.php +++ b/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_]*$.}} + ) {} +}