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

Fix stale result cache for require-extends/require-implements #2866

Merged
merged 28 commits into from
Jan 14, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,26 @@ jobs:
mv src/Baz.php.orig src/Baz.php
echo -n > phpstan-baseline.neon
../../bin/phpstan -vvv
- script: |
cd e2e/result-cache-6
echo -n > phpstan-baseline.neon
../../bin/phpstan -vvv
patch -b src/Baz.php < patch-1.patch
cat baseline-1.neon > phpstan-baseline.neon
../../bin/phpstan -vvv
mv src/Baz.php.orig src/Baz.php
echo -n > phpstan-baseline.neon
../../bin/phpstan -vvv
- script: |
cd e2e/result-cache-7
echo -n > phpstan-baseline.neon
../../bin/phpstan -vvv
patch -b src/Bar.php < patch-1.patch
cat baseline-1.neon > phpstan-baseline.neon
../../bin/phpstan -vvv
mv src/Bar.php.orig src/Bar.php
echo -n > phpstan-baseline.neon
../../bin/phpstan -vvv
Copy link
Member

Choose a reason for hiding this comment

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

I tried result-cache-8 after removing changes from src/ and it does not fail for me locally. Please first submit a draft PR with just the E2E tests so it's obvious they fail.

Copy link
Contributor Author

@staabm staabm Jan 12, 2024

Choose a reason for hiding this comment

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

after having slept about it, I think the situation is as follows:

we only need to support require-extends for result-cache invalidation because thats the only new part in the reflection hierarchy.

require-implements works with reflection information which was available before, we just added some rules arround it.

see failling test
and the fix

- script: |
cd e2e/bug-9622
echo -n > phpstan-baseline.neon
Expand Down
6 changes: 6 additions & 0 deletions e2e/result-cache-6/baseline-1.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
parameters:
ignoreErrors:
-
message: "#^Access to an undefined property TestResultCache6\\\\Bar\\:\\:\\$s\\.$#"
count: 1
path: src/Foo.php
10 changes: 10 additions & 0 deletions e2e/result-cache-6/patch-1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
diff --git b/e2e/result-cache-6/src/Baz.php a/e2e/result-cache-6/src/Baz.php
index 4a94eb3ae..6fed0b9ec 100644
--- b/e2e/result-cache-6/src/Baz.php
+++ a/e2e/result-cache-6/src/Baz.php
@@ -4,5 +4,4 @@ namespace TestResultCache6;

class Baz
{
- public string $s;
}
Empty file.
7 changes: 7 additions & 0 deletions e2e/result-cache-6/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
includes:
- phpstan-baseline.neon

parameters:
level: 8
paths:
- src
10 changes: 10 additions & 0 deletions e2e/result-cache-6/src/Bar.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

namespace TestResultCache6;

/**
* @phpstan-require-extends Baz
*/
interface Bar
{
}
8 changes: 8 additions & 0 deletions e2e/result-cache-6/src/Baz.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace TestResultCache6;

class Baz
{
public string $s;
}
15 changes: 15 additions & 0 deletions e2e/result-cache-6/src/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<?php

namespace TestResultCache6;

use function PHPStan\Testing\assertType;

class Foo
{

public function doFoo(Bar $b): void
{
echo $b->s;
}

}
6 changes: 6 additions & 0 deletions e2e/result-cache-7/baseline-1.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
parameters:
ignoreErrors:
-
message: "#^PHPDoc tag @phpstan\\-require\\-implements cannot contain non\\-interface type TestResultCache7\\\\Bar\\.$#"
count: 1
path: src/Foo.php
12 changes: 12 additions & 0 deletions e2e/result-cache-7/patch-1.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
diff --git a/e2e/result-cache-7/src/Bar.php b/e2e/result-cache-7/src/Bar.php
index b698e695d..0bbcc3093 100644
--- a/e2e/result-cache-7/src/Bar.php
+++ b/e2e/result-cache-7/src/Bar.php
@@ -2,6 +2,6 @@

namespace TestResultCache7;

-interface Bar
+class Bar
{
}
Empty file.
7 changes: 7 additions & 0 deletions e2e/result-cache-7/phpstan.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
includes:
- phpstan-baseline.neon

parameters:
level: 8
paths:
- src
7 changes: 7 additions & 0 deletions e2e/result-cache-7/src/Bar.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

namespace TestResultCache7;

interface Bar
{
}
8 changes: 8 additions & 0 deletions e2e/result-cache-7/src/Foo.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<?php

namespace TestResultCache7;

/** @phpstan-require-implements Bar */
trait Foo
{
}
25 changes: 25 additions & 0 deletions src/Dependency/DependencyResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,22 @@ public function resolveDependencies(Node $node, Scope $scope): NodeDependencies
&& $node->class instanceof Node\Name
) {
$this->addClassToDependencies($scope->resolveName($node->class), $dependenciesReflections);
} elseif ($node instanceof Node\Stmt\Trait_ && $node->namespacedName !== null) {
try {
$classReflection = $this->reflectionProvider->getClass($node->namespacedName->toString());

foreach ($classReflection->getRequireImplementsTags() as $implementsTag) {
foreach ($implementsTag->getType()->getReferencedClasses() as $referencedClass) {
if (!$this->reflectionProvider->hasClass($referencedClass)) {
continue;
}

$this->addClassToDependencies($referencedClass, $dependenciesReflections);
}
}
} catch (ClassNotFoundException) {
// pass
}
} elseif ($node instanceof Node\Stmt\TraitUse) {
foreach ($node->traits as $traitName) {
$this->addClassToDependencies($traitName->toString(), $dependenciesReflections);
Expand Down Expand Up @@ -495,6 +511,15 @@ private function addClassToDependencies(string $className, array &$dependenciesR
}
}

foreach ($classReflection->getRequireExtendsTags() as $extendsTag) {
foreach ($extendsTag->getType()->getReferencedClasses() as $referencedClass) {
if (!$this->reflectionProvider->hasClass($referencedClass)) {
continue;
}
$dependenciesReflections[] = $this->reflectionProvider->getClass($referencedClass);
}
}

foreach ($classReflection->getTemplateTags() as $templateTag) {
foreach ($templateTag->getBound()->getReferencedClasses() as $referencedClass) {
if (!$this->reflectionProvider->hasClass($referencedClass)) {
Expand Down