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

Cleanup doc in TypedPropertyFromCreateMockAssignRector to avoid PHPStan reports #6306

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

TomasVotruba
Copy link
Member

  • add fixture
  • Remove var docblock


final class ClearFalseMock extends TestCase
{
public \PHPUnit\Framework\MockObject\MockObject $someMock;
Copy link
Member

Choose a reason for hiding this comment

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

I am thinking if it can use @var MockObject&SomeMockedClass as mock class is mostly intersection typed, or use intersection type if supported (8.1).

/cc @staabm any opinion?

Copy link
Member Author

Choose a reason for hiding this comment

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

That would lead to error:

PHPDoc tag @var for property ... with type  ... is incompatible with native type                                                  PHPUnit\Framework\MockObject\MockObject.    

The only solution is to make MockObject generic, probably via phpstan/phpstan-phpunit package, but that would require having package installed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Another error in the & case is:

PHPDoc tag @var for property ... contains unresolvable type

Which is understandable, as class type is known and it never implements the MockObject interface

Copy link
Member

@samsonasik samsonasik Sep 15, 2024

Choose a reason for hiding this comment

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

what I mean is something like this:

/** @var MockObject&SomeClass */
private MockObject $prop;

intersection should works ok with phpstan subtype, if php 8.1 already, it can be directly typed:

private MockObject&SomeClass $prop;

see PHPUnit doc https://github.com/sebastianbergmann/phpunit/blob/98b9586dbfeed26df628789440cb69eaf5595bdb/src/Framework/TestCase.php#L1338

Copy link
Member

Choose a reason for hiding this comment

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

above is what @ruudk questioning on #6177 (review) which valid type as I see

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you test it? The first case throws me an error

Copy link
Member

Choose a reason for hiding this comment

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

I see, intersection docblock seems somehow can't resolve type when already typed MockObject, but union seems working:

/** @var MockObject|SomeClass */
private MockObject $prop;

Copy link
Member Author

Choose a reason for hiding this comment

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

Yet, that's not true. As it's not A or B, but always A.

Copy link
Contributor

@staabm staabm Sep 16, 2024

Choose a reason for hiding this comment

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

could someone post a phpstan.org/try snippet with the unexpected error?
(or a small repository with a repro in case phpstan.org/try cannot be used)

I am not sure yet, what the "error" is we are talking about.

I guess we are talking about whats described here:
https://github.com/phpstan/phpstan-phpunit?tab=readme-ov-file#how-to-document-mock-objects-in-phpdocs

@TomasVotruba TomasVotruba changed the title tv cleanup doc Cleanup doc in TypedPropertyFromCreateMockAssignRector to avoid false positives PHPSTan reports Sep 15, 2024
@TomasVotruba TomasVotruba changed the title Cleanup doc in TypedPropertyFromCreateMockAssignRector to avoid false positives PHPSTan reports Cleanup doc in TypedPropertyFromCreateMockAssignRector to avoid PHPStan reports Sep 15, 2024
@TomasVotruba
Copy link
Member Author

I'll split this into 2 rules, to make both steps separated and let user decide which to use.

@@ -18,7 +18,7 @@
"composer/semver": "^3.4",
"composer/xdebug-handler": "^3.0.5",
"doctrine/inflector": "^2.0.10",
"illuminate/container": "^11.22.0",
"illuminate/container": "11.22.3",
Copy link
Member

Choose a reason for hiding this comment

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

There is no 11.22.3 version, probably you mean 11.23.5, see https://packagist.org/packages/illuminate/container

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"illuminate/container": "11.22.3",
"illuminate/container": "11.23.5",

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.

3 participants