Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SONARPHP-1570 Symbols should be created for tree inside PropertyHooks #1314

Merged
merged 2 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions its/plugin/projects/php8-features/src/propertyHooks.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

class Test {
public int $prop {
get { return $this->prop * 2; }
set { $this->prop = $value * 2; }
}

// Edge-case where recursion happens via isset().
public int $prop2 {
get { return isset($this->prop2); }
set { }
}

public int $prop3 {
get {
return $this->prop3 + 1;
}
set {
$this->prop3 = $value - 1;
}
}
public $prop4 = 1 {
#[Attr]
&get => $this->prop4;
final set($value) => $value - 1;
}
abstract public $prop5 {
get;
set;
}
// TODO: Force multiline for hooks?
public function __construct(
public $foo {
get => 42;
set => 123;
},
public $bar
) {}
}

$test = new Test;
$test->prop = 10;
var_dump($test->prop);
var_dump(isset($test->prop));
var_dump(isset($test->prop2));
var_dump($test);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I found another two ITS that were not running 😓
Thankfully they were green.

Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,10 @@ private static void assertAnalyzerLogs(String logs) {

// TODO SONARPHP-1466 Replace nested classes in it-php-plugin-tests:Tests with a more elegant solution

@Nested
class NestedCpdTokenTest extends CpdTokenTest {
}

@Nested
class NestedCustomRulesTest extends CustomRulesTest {
}
Expand All @@ -219,6 +223,10 @@ class NestedNoSonarTest extends NoSonarTest {
class NestedPHPIntegrationTest extends PHPIntegrationTest {
}

@Nested
class NestedPhpStanReportTest extends PhpStanReportTest {
}

@Nested
class NestedPHPTest extends PHPTest {
}
Expand All @@ -228,15 +236,15 @@ class NestedPHPUnitTest extends PHPUnitTest {
}

@Nested
class NestedSonarLintTest extends SonarLintTest {
class NestedPsalmReportTest extends PsalmReportTest {
}

@Nested
class NestedPhpStanReportTest extends PhpStanReportTest {
class NestedSonarLintTest extends SonarLintTest {
}

@Nested
class NestedPsalmReportTest extends PsalmReportTest {
class NestedSuppressWarningsTest extends SuppressWarningsTest {
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ public void visitMethodDeclaration(MethodDeclarationTree tree) {
public void visitClassPropertyDeclaration(ClassPropertyDeclarationTree tree) {
scan(tree.attributeGroups());
resolveDeclaredTypeNamespaceName(tree.declaredType());

var propertyHookListTree = tree.propertyHookList();
if (propertyHookListTree != null) {
scan(propertyHookListTree);
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,16 @@ void constantDeclaration() {
assertThat(symbolTable.getSymbol("A1").qualifiedName()).hasToString("a1");
}

@Test
void shouldCreateSymbolForFunctionInPropertyHook() {
SymbolTableImpl symbolTable = symbolTableFor("<?php class A { public int $prop { get { return isset($this->prop2); } } }");

Symbol symbol = symbolTable.getSymbol("isset");
assertThat(symbol).isInstanceOf(UndeclaredSymbol.class);
assertThat(symbol.is(Symbol.Kind.FUNCTION)).isTrue();
assertThat(symbol.usages()).hasSize(1);
}

private static void assertClassSymbols(SymbolTableImpl symbolTable, String... fullyQualifiedNames) {
assertThat(symbolTable.getSymbols(Kind.CLASS)).extracting(s -> s.qualifiedName().toString())
.containsExactly(fullyQualifiedNames);
Expand Down