Skip to content

Conversation

@VincentLanglet
Copy link
Contributor

No description provided.

@VincentLanglet VincentLanglet force-pushed the createMockForIntersectionOfInterfaces branch from 425904d to 465b649 Compare December 16, 2025 14:47
@VincentLanglet VincentLanglet marked this pull request as ready for review December 16, 2025 14:51
Copy link
Contributor

@staabm staabm left a 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()

@VincentLanglet VincentLanglet force-pushed the createMockForIntersectionOfInterfaces branch 2 times, most recently from 47b0d6f to 51521f4 Compare December 17, 2025 08:47
@VincentLanglet VincentLanglet force-pushed the createMockForIntersectionOfInterfaces branch from c782e6d to 0c7b351 Compare February 9, 2026 21:27
@VincentLanglet VincentLanglet force-pushed the createMockForIntersectionOfInterfaces branch from 0c7b351 to c88dbd0 Compare February 9, 2026 21:29
],
];

if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore-line function.alreadyNarrowedType
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore-line function.alreadyNarrowedType
if (method_exists(TestCase::class, 'createMockForIntersectionOfInterfaces')) { // @phpstan-ignore function.alreadyNarrowedType

Copy link
Contributor Author

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 ?

Copy link
Contributor

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-line always ignores all errors of a single line (does not support a by-identifier filter)
  • @phpstan-ignore-next-line always ignores all errors of a single line (does not support a by-identifier filter)
  • @phpstan-ignore requires at least one identifier and ignores only errors of such identifier (on the same line)

Copy link
Member

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.

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 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).

Copy link
Contributor

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)

Copy link
Contributor

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

Copy link
Contributor

@staabm staabm left a 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

@VincentLanglet
Copy link
Contributor Author

This PR need the approval of @ondrejmirtes since he requested changes

@ondrejmirtes ondrejmirtes merged commit f7553d6 into phpstan:2.0.x Feb 10, 2026
88 checks passed
@ondrejmirtes
Copy link
Member

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

Development

Successfully merging this pull request may close these issues.

3 participants