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

Add test for phpstan issue 7396 #1442

Draft
wants to merge 4 commits into
base: 2.0.x
Choose a base branch
from

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Jun 18, 2022

@mvorisek mvorisek force-pushed the test_method_type_specify branch 2 times, most recently from f8326f6 to eee8782 Compare June 18, 2022 11:08
$m = random_int(0, 1) === 0 ? new Model() : new class() extends Model implements Iface {};
assertType('Bug7385\Model', $m);
$m->assertHasIface();
assertType('Bug7385\Iface&Bug7385\Model', $m);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes this assertion is not passing, do you know why?

  • Bug7385MethodTypeSpecifyingExtension::specifyTypes() is correctly reached and $type is (the expected) Bug7385\Iface&Bug7385\Model
  • I am not completely sure what TypeSpecifierContext::createXxx() is used for or if I should use TypeSpecifierContext::createTrue(), but I tried it.
  • I also tried to to set $overwrite param to true in $this->typeSpecifier->create() factory method, but it seems the $m type is still Bug7385\Model as if the MethodTypeSpecifyingExtension result is completely ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes can you please take a look on this issue?

Copy link
Member

Choose a reason for hiding this comment

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

There's 343 open bug reports (https://github.com/phpstan/phpstan/labels/bug) and 323 feature requests (https://github.com/phpstan/phpstan/labels/feature-request). Your issue isn't anything urgent to warrant immediate attention - it's not anything special and doesn't stand out from other 666 open issues.

This PR doesn't even have an associated issue - I closed #7385 because there wasn't anything about type-specifying extensions in the description.

There might even be an open issue about "type-specifying extension doesn't allow me to change the type of the object the method is called on", I'm not sure.

Anyway, if you're already this far, you can take one more step and debug where it actually gets lost, and fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, there is phpstan/phpstan#7396 issue I opened later after a short discussion about an implicit support in phpstan/phpstan#7385

@mvorisek mvorisek changed the title Add test for phpstan issue 7385 Add test for phpstan issue 7396 Jun 24, 2022
@ondrejmirtes ondrejmirtes changed the base branch from 1.7.x to 2.0.x September 26, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants