-
Notifications
You must be signed in to change notification settings - Fork 53
Add support for createMockForIntersectionOfInterfaces #272
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 support for createMockForIntersectionOfInterfaces #272
Conversation
425904d to
465b649
Compare
staabm
left a comment
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.
I would expect a return type extension in addition to be tested in a type inference test with assertType()
47b0d6f to
51521f4
Compare
src/Type/PHPUnit/MockForIntersectionDynamicReturnTypeExtension.php
Outdated
Show resolved
Hide resolved
src/Type/PHPUnit/MockForIntersectionDynamicReturnTypeExtension.php
Outdated
Show resolved
Hide resolved
c782e6d to
0c7b351
Compare
0c7b351 to
c88dbd0
Compare
| ], | ||
| ]; | ||
|
|
||
| if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore-line function.alreadyNarrowedType |
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.
| if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore-line function.alreadyNarrowedType | |
| if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore function.alreadyNarrowedType |
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.
Kinda curious about this feedback.
Isn't @phpstan-ignore-line function.alreadyNarrowedType clearer ?
I understand it as Ignore function.alreadyNarrowedType in the current line
As opposite to phpstan-ignore-next-line
Are phpstan-ignore-line and phpstan-ignore the same ?
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.
my understanding is
@phpstan-ignore-linealways ignores all errors of a single line (does not support a by-identifier filter)@phpstan-ignore-next-linealways ignores all errors of a single line (does not support a by-identifier filter)@phpstan-ignorerequires at least one identifier and ignores only errors of such identifier (on the same line)
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.
Exactly. I suspect people do this mistake often but I don't know how to prevent it.
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.
I didn't know that, and made the mistake a lot then
https://github.com/search?q=repo%3AVincentLanglet%2FTwig-CS-Fixer+phpstan-ignore-next-line&type=code
Especially because I don't like adding @phpstan-ignore on the same line and prefer to ignore before the line (so I use ignore-next-line a lot).
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.
as far as I remember there is a open feature request (cannot find it right now), which goal is to disallow @phpstan-ignore-line and @phpstan-ignore-next-line and allow only ignoring errors by identifier (via opt-in config)
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.
alternatively we could look at @phpstan-ignore-line and @phpstan-ignore-next-line like we do for @phpstan-ignore - but report a error if we find a valid issue-identifier after it
tests/Type/PHPUnit/MockForIntersectionDynamicReturnTypeExtensionTest.php
Outdated
Show resolved
Hide resolved
staabm
left a comment
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.
few nits, lgtm otherwise
|
This PR need the approval of @ondrejmirtes since he requested changes |
|
Thank you! |
No description provided.