Skip to content

Conversation

@yt-catpaw
Copy link

  • Add the reported case to nsrt: tests/PHPStan/Analyser/nsrt/bug-10942.php
  • Wire it into AnalyserIntegrationTest::testBug10942
  • Verified it passes on current 2.1.x (PHP 8.3.19)

closes phpstan/phpstan#10942

If you ran the focused test, you can mention:
vendor/bin/phpunit --filter testBug10942 tests/PHPStan/Analyser/AnalyserIntegrationTest.php

@ondrejmirtes
Copy link
Member

Hey, I don't understand this PR. You're adding a regression test for an issue but the issue isn't fixed. So I'm sorry, but I can't merge this PR in the current state of the code.

@yt-catpaw
Copy link
Author

@ondrejmirtes

I added the reporter’s test case intending to fix the issue, but the error didn’t reproduce, so the PR only adds the test. Is that a problem?

I added the reporter’s case to nsrt and it no longer reproduces on current 2.1.x (PHP 8.3.19) — the test passes. I’d still like to keep the test as a regression safeguard so we catch any future breakage.

@ondrejmirtes
Copy link
Member

The bug is not fixed. If it was fixed then the playground https://phpstan.org/r/819a30a6-5477-448a-a421-24076f8dcd0f would tell you about it.

The bug is happening only if you enable strict rules. In phpstan-src parlance, it will probably happen only with reportMaybesInMethodSignatures: true.

The best way to reproduce it would be in MethodSignatureRuleTest. But it has to be fixed first.

@yt-catpaw
Copy link
Author

Sorry for the confusion — after moving the case into MethodSignatureRuleTest with strict settings, I can reproduce the original “should be contravariant” error. Thanks for pointing me in the right direction. I’ll take a look from here.

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.

Wrong Parameter #3 $x ... of method B::foo() should be contravariant with parameter $x ... of method A::foo()

2 participants