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 index 55cf48f661..4a38bd63f1 100644 --- 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 index 1e599d23e9..5ca2390fa8 100644 --- 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 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 index 4114300ec9..155843246d 100644 --- 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 index c922976c82..d4cd2d725e 100644 --- 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 index 0de696a401..a2199c0976 100644 --- 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 index 4d8eb0ff82..da19075750 100644 --- 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 index 5d71999e0b..51d3024338 100644 --- 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) {