nikic / PHP-Parser

A PHP parser written in PHP
BSD 3-Clause "New" or "Revised" License
16.98k stars 1.09k forks source link

Declare more precise phpdoc types #993

Closed staabm closed 5 months ago

staabm commented 5 months ago

goal: ease working in projects which are static analysis covered.

we are doing the same thing in PHPStan with https://github.com/phpstan/phpstan-src/pull/3013#pullrequestreview-1998289070

staabm commented 5 months ago

in the same turn I think about making the $type in Identifier a non-empty-string.. do you think this would work?

nikic commented 5 months ago

Can you please target this at master instead?

in the same turn I think about making the $type in Identifier a non-empty-string.. do you think this would work?

I think that's fine. Empty string identifiers need to be written as something like {''}, in which case they're not Identifiers anymore.

staabm commented 5 months ago

the CI build on php-parser@v5 looks pretty broken. it seems there is some chicken-egg problem regarding deps and v4. I don't think its related to this PR

staabm commented 5 months ago

after a deep dive into the CI problem I found a solution (its still not caused by this PR though). My guess is, that this errors only occur in pull requests from forks.

the last remaining question is https://github.com/nikic/PHP-Parser/pull/993#discussion_r1563768987

Thank you.

ondrejmirtes commented 5 months ago

I’m not a huge fan of this. More precise parameter types will break static analysis in many projects, forcing people to write more precise PHPDoc types.

These types of changes should be done only in a major version IMHO.

staabm commented 5 months ago

I’m not a huge fan of this. More precise parameter types will break static analysis in many projects, forcing people to write more precise PHPDoc types.

Yeah thats why I asked. I am fine with return type improvements only

ondrejmirtes commented 5 months ago

Yeah, return types should be fine, although it's a slight BC break too.

Imagine you're extending a class and typehinting string return type.

If the parent narrows the return type, suddenly the child return type is no longer covariant.