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
No longer verify types, when TypeSpecifyingExtension uses $overwrite=true #1011
Conversation
TypeSpecifierContext $context | ||
): SpecifiedTypes | ||
{ | ||
$newType = new GenericObjectType(\GenericTypeOverride\Foo::class, [\GenericTypeOverride\Bar::class]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't how the constructor should be called. The constructor looks like this:
/**
* @api
* @param array<int, Type> $types
*/
public function __construct(
string $mainType,
private array $types,
?Type $subtractedType = null,
private ?ClassReflection $classReflection = null,
)
You need new ObjectType(\GenericTypeOverride\Bar::class)
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx for catching. with this fix, the unit test works like expected:
$ vendor/bin/phpunit tests/PHPStan/Rules/Comparison/ImpossibleCheckTypeGenericOverwriteRuleTest.php
Xdebug: [Step Debug] Time-out connecting to debugging client, waited: 200 ms. Tried: localhost:9003 (through xdebug.client_host/xdebug.client_port) :-(
PHPUnit 9.5.7 by Sebastian Bergmann and contributors.
Warning: XDEBUG_MODE=coverage or xdebug.mode=coverage has to be set
F 1 / 1 (100%)
Time: 00:00.424, Memory: 52.00 MB
There was 1 failure:
1) PHPStan\Rules\Comparison\ImpossibleCheckTypeGenericOverwriteRuleTest::testNoReportedErrorOnOverwrite
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'13: Call to method GenericTypeOverride\Foo<int>::setFetchMode() will always evaluate to false.
'
C:\dvl\Workspace\phpstan-src-staabm\src\Testing\RuleTestCase.php:140
C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Rules\Comparison\ImpossibleCheckTypeGenericOverwriteRuleTest.php:34
I am wondering whether phpstan should have errored about this when scanning the test-case?
as can be seen in the github actions, it did not catch this problem
I wonder if this is related or will fix cases over on webmozart-assert too. Need to check later :) |
|
||
public function testNoReportedErrorOnOverwrite(): void | ||
{ | ||
$this->treatPhpDocTypesAsCertain = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't need to be a property if you're not testing true/false scenarios. So please put "true" directly in the constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Thank you. |
closes phpstan/phpstan#6620
I am not 100% happy with the test-case.
the test fails without the fix and passes with it - as expected.
but when running the test without the fix I get this error:
it seems there is something missing which makes the test end properly (without the fix), to render a proper repro-case.
the test errors here
phpstan-src/src/Rules/Comparison/ImpossibleCheckTypeHelper.php
Line 195 in 4f72fdc
because it seems it cannot find some part of the involved objects within
$scope
- I guess.