Support advanced data types

[dpi]

Originally https://www.drupal.org/project/coder/issues/3253472

Allow advanced type styles as implemented by PHPStan and Psalm, and recognised by PHPStorm.

Core will be making more use of PHPStan over time. Its going to be necessary to support the types it implements.

For example array shapes:

array{entity_type_id:string,langcode:string}

array<string, string>

array<string, SomethingMoreComplex>

Right now to use these its a frustrating dance of ignores and in some circumstances wholesale disable/modification of sniffs.

[dpi]

Missing tests per comment on d.o

[klausi]

Thanks, tests are failing with this change. Can you check? We probably have to adopt some things that were disallowed so far.

Please also merge in the 8.3.x branch, I made a phpstan fix there.

[dpi]

Tried experimenting by taking the code example from test27 and test29 which were broken by this initial work. PHPStan doesn't complain about them, so I'm thinking PHPStan simply doesnt care if the types dont exist.

This suggests to me we shouldnt bother doing validation of the specific type in this project, so long as there is something present, thats enough.

[dpi]

@klausi

I've added a bunch of tests which need to be fixed. The test are primarily examples of valid types from https://phpstan.org/writing-php-code/phpdoc-types.

I dont think it would make sense to have a giant whitelist of allowed types, perhaps there should just be minimal intelligence about the allowed characters that comprise a type. Then defer any more advanced intelligence to static analysis. Should \Drupal\Sniffs\Commenting\FunctionCommentSniff::suggestType remain or be reworked?

I'm gonna need some direction about reducing existing and increasing existing sniffs/suggests.

[dpi]

The tests in FunctionCommentUnitTest.inc should mostly remain as close to samples in https://phpstan.org/writing-php-code/phpdoc-types, then we adapt from there.

[klausi]

Regarding suggestType(): I think this should stay, because ::$invalidTypes contains a list of very common mistakes people make that should be flagged.

I think we have to bite the bullet and make a huge allow list of PHPStan types with a regex? Or maybe we can allow anything and only flag those simple cases? We could say: if the doc type only contains alphanumeric characters + \ then the typehint must also only contain alphanumeric characters. That way we ensure class/interface names stay in sync between doc comment and type hint, a mistake that people often make.

Otherwise we allow really bad type strings that people not using PHPStan put in.

[dpi]

irt mixed everywhere I was just focussing on the docs since it doesnt really matter that there is a mismatch.

And we cant check that doc and real param match since there are many types that simply dont match eachother. Think any non core-PHP type, like class-string etc.

I'm hoping upstream simply has loosened these checks and we can just undo a bunch of Coder' customizations.

I think we have to bite the bullet and make a huge allow list of PHPStan types with a regex?

I'd really hope a whitelist isnt the solution. Maybe check with upstream? / or phpfixer

suggestType should still check for common mistakes, but it shouldn't always have an answer. It should give up if the type is too foreign-looking.

I dont have time for this at the moment, and I've been meaning to look at viability of phpcsfixer.

This issue might be on-theme for @mglaman at the moment as there is a desire to add PHPStan types to core, which I thoroughly endorse.

[klausi]

class-string doc comment type must always mean string typehint. So that would be a way of mapping for suggestType().

Apart from the @template case are there other examples that don't have a clear mapping like this?

[ultimike]

This would be a big +1 for me.

[eileenmcnaughton]

It would be great to see this resolved - looks like there was a bunch of work done but it went cold

[klausi]

Absolutely, and some parts of this are already committed now! I want to finish the PHPStan array shape support in https://www.drupal.org/project/coder/issues/3253472 (not sure how soon I will get to it, feel free to beat me to it), then I think we can call that one fixed and open new issues for more PHPStan types.

I will close this PR now as we will not merge it as is, but thanks a lot for the inspiration @dpi and others!