SONARPHP-1583 Constructor promoted properties should generate class m…

…ember symbol (#1330)
SonarSource · Nov 29, 2024 · bcaf88d · bcaf88d
1 parent 00d2dfe
commit bcaf88d
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 11 deletions.
diff --git a/its/ruling/src/integrationTest/resources/expected/PHP_CodeSniffer/php-S1068.json b/its/ruling/src/integrationTest/resources/expected/PHP_CodeSniffer/php-S1068.json
@@ -6,7 +6,8 @@
 61,
 64,
 67,
-73
+73,
+78
 ],
 "PHP_CodeSniffer:src/Standards/PEAR/Tests/NamingConventions/ValidVariableNameUnitTest.inc": [
 19,

diff --git a/php-checks/src/main/java/org/sonar/php/checks/UnusedPrivateFieldCheck.java b/php-checks/src/main/java/org/sonar/php/checks/UnusedPrivateFieldCheck.java
@@ -32,10 +32,9 @@
 import org.sonar.plugins.php.api.tree.expression.NameIdentifierTree;
 import org.sonar.plugins.php.api.visitors.PHPVisitorCheck;
 
-@Rule(key = UnusedPrivateFieldCheck.KEY)
+@Rule(key = "S1068")
 public class UnusedPrivateFieldCheck extends PHPVisitorCheck {
 
-  public static final String KEY = "S1068";
   private static final String MESSAGE = "Remove this unused \"%s\" private field.";
 
   private static final Set<String> constantUsedBeforeInit = new HashSet<>();

diff --git a/php-checks/src/test/java/org/sonar/php/checks/UnusedPrivateFieldCheckTest.java b/php-checks/src/test/java/org/sonar/php/checks/UnusedPrivateFieldCheckTest.java
@@ -22,7 +22,7 @@
 class UnusedPrivateFieldCheckTest {
 
   @Test
-  void test() throws Exception {
+  void shouldRaiseExpectedIssues() {
     CheckVerifier.verify(new UnusedPrivateFieldCheck(), "UnusedPrivateFieldCheck.php");
   }
 }
diff --git a/php-checks/src/test/resources/checks/UnusedPrivateFieldCheck.php b/php-checks/src/test/resources/checks/UnusedPrivateFieldCheck.php
@@ -9,13 +9,20 @@ class C {
   private static $field4;     // OK
   private static $field5;     // OK
 
+  public function __construct(
+    public $promotedPublic,
+    private $promotedPrivateUsed,
+    private $promotedPrivateUnused // Noncompliant
+  ) {}
+
   public function f($field1) {
-    return $field1 + $this->field2;
+    return $field1 + $this->field2 + $this->promotedPrivateUsed;
   }
 
   public function g() {
     return $this->myArray[0] + self::$field4 + static::$field5;
   }
+
 }
 
 class D {

diff --git a/php-frontend/src/main/java/org/sonar/php/tree/symbols/SymbolVisitor.java b/php-frontend/src/main/java/org/sonar/php/tree/symbols/SymbolVisitor.java
@@ -22,7 +22,9 @@
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import org.sonar.api.utils.Preconditions;
 import org.sonar.php.api.PHPKeyword;
@@ -261,8 +263,16 @@ public void visitAnonymousClass(AnonymousClassTree tree) {
   private void createMemberSymbols(ClassTree tree, @Nullable TypeSymbolImpl classSymbol) {
     for (ClassMemberTree member : tree.members()) {
       if (member.is(Kind.METHOD_DECLARATION)) {
-        SymbolImpl symbol = createMemberSymbol(classSymbol, ((MethodDeclarationTree) member).name(), Symbol.Kind.FUNCTION);
-        symbol.addModifiers(((MethodDeclarationTree) member).modifiers());
+        var method = (MethodDeclarationTree) member;
+        var name = method.name();
+
+        SymbolImpl symbol = createMemberSymbol(classSymbol, name, Symbol.Kind.FUNCTION);
+        symbol.addModifiers(method.modifiers());
+
+        if ("__construct".equals(name.text())) {
+          createMemberSymbolsForPromotedProperties(method, classSymbol);
+        }
+
       } else if (member.is(Kind.CLASS_CONSTANT_PROPERTY_DECLARATION, Kind.CLASS_PROPERTY_DECLARATION)) {
         ClassPropertyDeclarationTree classPropertyDeclaration = (ClassPropertyDeclarationTree) member;
         for (VariableDeclarationTree field : classPropertyDeclaration.declarations()) {
@@ -277,6 +287,20 @@ private void createMemberSymbols(ClassTree tree, @Nullable TypeSymbolImpl classS
     }
   }
 
+  private void createMemberSymbolsForPromotedProperties(MethodDeclarationTree method, @Nullable TypeSymbolImpl classSymbol) {
+    method.parameters().parameters().stream()
+      .filter(ParameterTree::isPropertyPromotion)
+      .forEach(param -> {
+        SymbolImpl fieldSymbol = createMemberSymbol(classSymbol, param.variableIdentifier(), Symbol.Kind.FIELD);
+        var modifiers = Stream.of(param.visibility(), param.readonlyToken()).filter(Objects::nonNull).toList();
+        fieldSymbol.addModifiers(modifiers);
+        ExpressionTree initValue = param.initValue();
+        if (initValue != null) {
+          initValue.accept(this);
+        }
+      });
+  }
+
   private SymbolImpl createMemberSymbol(@Nullable TypeSymbolImpl classSymbol, IdentifierTree identifier, Symbol.Kind kind) {
     SymbolImpl symbol;
     if (classSymbol != null) {

diff --git a/php-frontend/src/test/java/org/sonar/php/tree/symbols/SymbolTableImplTest.java b/php-frontend/src/test/java/org/sonar/php/tree/symbols/SymbolTableImplTest.java
@@ -17,6 +17,7 @@
 package org.sonar.php.tree.symbols;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -28,6 +29,7 @@
 import org.sonar.php.tree.impl.PHPTree;
 import org.sonar.php.tree.impl.expression.FunctionCallTreeImpl;
 import org.sonar.plugins.php.api.symbols.MemberSymbol;
+import org.sonar.plugins.php.api.symbols.QualifiedName;
 import org.sonar.plugins.php.api.symbols.Symbol;
 import org.sonar.plugins.php.api.symbols.Symbol.Kind;
 import org.sonar.plugins.php.api.symbols.TypeSymbol;
@@ -100,12 +102,12 @@ void caseSensitivity() {
 
   @Test
   void symbolsFiltering() {
-    assertThat(SYMBOL_MODEL.getSymbols()).hasSize(20);
+    assertThat(SYMBOL_MODEL.getSymbols()).hasSize(26);
 
-    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.FUNCTION)).hasSize(2);
+    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.FUNCTION)).hasSize(3);
     assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.CLASS)).hasSize(1);
-    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.FIELD)).hasSize(3);
-    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.PARAMETER)).hasSize(1);
+    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.FIELD)).hasSize(5);
+    assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.PARAMETER)).hasSize(4);
     assertThat(SYMBOL_MODEL.getSymbols(Symbol.Kind.VARIABLE)).hasSize(13);
 
     assertThat(SYMBOL_MODEL.getSymbols("$a")).hasSize(3);
@@ -137,6 +139,30 @@ void testClassFields() {
     assertThat(constantField.is(Symbol.Kind.FIELD)).isTrue();
   }
 
+  @Test
+  void promotedPropertiesShouldGenerateSymbols() {
+    assertThat(SYMBOL_MODEL.getSymbols())
+      .map(Symbol::qualifiedName)
+      .filteredOn(Objects::nonNull)
+      .map(QualifiedName::toString)
+      .contains("a::$promoted1", "a::$promoted2");
+  }
+
+  @Test
+  void promotedPropertyShouldGenerateFieldAndParamSymbol() {
+    var symbols = SYMBOL_MODEL.getSymbols("$promoted1");
+
+    assertThat(symbols).hasSize(2);
+
+    var fieldSymbol = symbols.get(0);
+    assertThat(fieldSymbol.qualifiedName()).hasToString("a::$promoted1");
+    assertThat(fieldSymbol.kind()).isEqualTo(Kind.FIELD);
+
+    var paramSymbol = symbols.get(1);
+    assertThat(paramSymbol.qualifiedName()).isNull();
+    assertThat(paramSymbol.kind()).isEqualTo(Kind.PARAMETER);
+  }
+
   @Test
   void testGlobalConstant() {
     Symbol constant = SYMBOL_MODEL.getSymbols("CONSTANT").get(0);

diff --git a/php-frontend/src/test/resources/symbols/symbolTable.php b/php-frontend/src/test/resources/symbols/symbolTable.php
@@ -22,6 +22,8 @@ class A {                                     // A
 
   public function f($p = 12) {                     // f, $p
   }
+
+  public function __construct(public $promoted1 = 1, private $promoted2, $param) {}
 }
 
 $x = function () use($a) {
-Original file line number
+Diff line change
@@ Expand Up / @@ -6,7 +6,8 @@ @@
 ,
 ,
 ,
+,
     ],
     "PHP_CodeSniffer:src/Standards/PEAR/Tests/NamingConventions/ValidVariableNameUnitTest.inc": [
 ,
@@ Expand Down @@