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

Reproducer for vimeo/psalm#4705 #4747

Closed
wants to merge 1 commit into from

Conversation

weirdan
Copy link
Collaborator

@weirdan weirdan commented Dec 1, 2020

I believe I finally managed to reproduce #4705 in Psalm codebase. The issue now should show up in CI builds.

@weirdan weirdan marked this pull request as ready for review December 1, 2020 03:04
@muglug
Copy link
Collaborator

muglug commented Dec 1, 2020

ok, thanks.

Release 4.2.0 contained this fix:

Inheritance for docblock return and param types

This ticket – #4537 – revealed a slight issue with Psalm's automatic inheritance of docblock param and return types.

Psalm will now only inherit docblock parameter and return types from a single docblock, which means you can't define param types in one parent class method and return types in another. It's a relatively minor change, but it might break some code.

I'm pretty sure that stubs are mucking that up somehow – the param types Psalm sees for that function are

     * @param string $entityName The name of the entity type.
     * @param mixed  $id         The entity identifier.

Which means the templated type T never gets properly inferred.

@weirdan
Copy link
Collaborator Author

weirdan commented Dec 1, 2020

It wasn't until 3dd185e that it surfaced though:

> git bisect good 4.1.1                                                                                                                                                        
You need to start by "git bisect start"

Do you want me to do it for you [Y/n]? y

> git bisect bad 4.2.0                                                                                                                                                         
Bisecting: 64 revisions left to test after this (roughly 6 steps)
[5a62dc5c40ab49314b788dd9b68d3daeda6a2c94] Fix #4540 - use correct method when simulating property setting

> git bisect run bash -c 'git cherry-pick --no-commit 033d2e1645e15a572789a01bd54c2cc9c2d75e5b && php-noxdebug vendor/bin/phpunit tests/EndToEnd/Bug4705Test.php; z=$?; git reset --hard; exit $z'
running bash -c git cherry-pick --no-commit 033d2e1645e15a572789a01bd54c2cc9c2d75e5b && php-noxdebug vendor/bin/phpunit tests/EndToEnd/Bug4705Test.php; z=$?; git reset --hard; exit $z
PHPUnit 9.4.2 by Sebastian Bergmann and contributors.

Runtime:       PHP 7.4.11
Configuration: /home/weirdan/src/psalm/psalm/phpunit.xml.dist
Random Seed:   1606820819
Warning:       No code coverage driver available
Warning:       Your XML configuration validates against a deprecated schema.
Suggestion:    Migrate your XML configuration using "--migrate-configuration"!

.                                                                   1 / 1 (100%)

Time: 00:01.801, Memory: 8.00 MB

OK (1 test, 3 assertions)
HEAD is now at 5a62dc5c4 Fix #4540 - use correct method when simulating property setting
Bisecting: 32 revisions left to test after this (roughly 5 steps)
[f6591e6d0f25dca6bb4fa19e27d125612355cf19] Use resolution that works in multithreaded mode

[.....................]


13b83e61325291fbe9788452686f040299a9ac46 is the first bad commit
commit 13b83e61325291fbe9788452686f040299a9ac46
Author: Matt Brown <github@muglug.com>
Date:   Fri Nov 13 09:43:30 2020 -0500

    Fix #4545 - allow intersections in more places

 src/Psalm/Internal/Codebase/Methods.php     | 14 ++++++--
 src/Psalm/Type.php                          | 16 ++++++++-
 tests/Template/ClassTemplateExtendsTest.php | 52 +++++++++++++++++++++++++++--
 tests/Template/ClassTemplateTest.php        | 12 +++++++
 4 files changed, 88 insertions(+), 6 deletions(-)
bisect run success

@muglug
Copy link
Collaborator

muglug commented Dec 1, 2020

I just added a failing testcase in master to StubTest (where debugging the problem is easier) – closing this

@muglug muglug closed this Dec 1, 2020
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.

None yet

2 participants