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 resolving of __DIR__, __NAMESPACE__ in traits #2043

Merged
merged 5 commits into from Jan 11, 2023

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 3, 2022

closes phpstan/phpstan#3019

the test fails without the fix like

There were 2 failures:

1) PHPStan\Analyser\NodeScopeResolverTest::testFileAsserts with data set "/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/data/bug-3019.php:11" ('type', '/Users/staabm/workspace/phpst...19.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Constant\ConstantStringType Object (...), 11)
Expected type string, got type 'Bug3019\\Bar' in /Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/data/bug-3019.php on line 11.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'string'
+''Bug3019\\Bar''

/Users/staabm/workspace/phpstan-src/src/Testing/TypeInferenceTestCase.php:96
/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/NodeScopeResolverTest.php:1150

2) PHPStan\Analyser\NodeScopeResolverTest::testFileAsserts with data set "/Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/data/bug-3019.php:12" ('type', '/Users/staabm/workspace/phpst...19.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Constant\ConstantStringType Object (...), 12)
Expected type string, got type 'Bug3019' in /Users/staabm/workspace/phpstan-src/tests/PHPStan/Analyser/data/bug-3019.php on line 12.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'string'
+''Bug3019''

3) PHPStan\Rules\Comparison\UnreachableTernaryElseBranchRuleTest::testBug3019
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'09: Else branch is unreachable because ternary operator condition is always true.
 '

@staabm staabm marked this pull request as ready for review December 3, 2022 15:32
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

src/Reflection/InitializerExprTypeResolver.php Outdated Show resolved Hide resolved
src/Reflection/InitializerExprTypeResolver.php Outdated Show resolved Hide resolved
{
public function doFoo(): void
{
assertType('string', __CLASS__);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am wondering that we don't get a class-string here

Copy link
Member

Choose a reason for hiding this comment

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

My guess it's not checked because a duplicate trait with the same name exists.

@@ -0,0 +1,21 @@
<?php

namespace Bug3019;
Copy link
Member

Choose a reason for hiding this comment

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

Duplicate namespaces/duplicate classes

Copy link
Member

Choose a reason for hiding this comment

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

Just use a single file used in both RuleTestCase and NodeScopeResolverTest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ahh good catch.

when I have some time I will work on the rule to detect duplicate namespaces :)

{
public function doFoo(): void
{
assertType('string', __CLASS__);
Copy link
Member

Choose a reason for hiding this comment

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

My guess it's not checked because a duplicate trait with the same name exists.

@staabm
Copy link
Contributor Author

staabm commented Jan 9, 2023

should this be re-targeted for 1.10.x or will we still have 1.9.x releases before 1.10 is out?

@ondrejmirtes
Copy link
Member

or will we still have 1.9.x releases before 1.10 is out

Yeah, sure, that's the point of these branches :)

@@ -0,0 +1,35 @@
<?php

namespace Bug3019;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I verified this code still fails without the patch for both Test-cases

@ondrejmirtes
Copy link
Member

Nice, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants