Support advanced PHPStan data types (general, arrays)

Thanks, makes sense to me! test cases are missing, can you add some?

Please also file a pull request against https://github.com/pfrenssen/coder so that we see the tests run.

As a workaround you can simply disable the Coder sniff that checks for types if you are using PHPStan in your project and it checks the types for you.

See #3309010: Support PHPDoc Types in @param @var @return annotations in the coding standards issue queue. https://git.drupalcode.org/project/drupal/-/merge_requests/1126/diffs for an example how usage of @phpstan-type annotations would help with array shapes in the context of Drupal's usage of arrays for database schema.

#7

As a workaround you can simply disable the Coder sniff that checks for types if you are using PHPStan in your project and it checks the types for you.

Maybe since PHPStan is now checking Drupal core at each test run, it's worth thinking to do so overall for core?

Coming from #3347758: [PP-1] Support the recommended phpdoc syntax for documenting Iterator return types which I had opened before finding this issue (which would solve a superset of the problem I was worried about).

Looks like this is now being handled at https://github.com/pfrenssen/coder/pull/158

Thanks for all the work on the pull request!

The problem I see here is that you are trying support all the possible magic type strings from PHPStan all at once. That makes the pull request big and full of contradictions. Instead, I think we need to split this into smaller chunks and work on this iteratively. Support the most basic array shapes first and then go from there. I think the test cases are very useful, thanks for that.

One consequence of this change is that we will have to remove the check for equality of the data types with type hints from Coder. That is a bit of an unfortunate regression, but with all the possibilities in PHPStan Coder will not be able to keep up. For example the doc data type "sstring" (typo double s) with type hint "string" will not throw an error anymore. I think that is acceptable if we make life for PHPStan users easier as tradeoff.

I will check if I can get simple array shapes working now.

Before starting with array shapes I found that we do not support all basic types by PHPStan, pushed a change for that with test cases. https://github.com/pfrenssen/coder/pull/191

Next on the PHPStan page is Integer ranges, but I will do General arrays first as it seems more important for people to use.

I don't want to allow arbitrary characters before the dollar sign yet, because that would hurt new contributors to Drupal. They make lots of tiny data type mistakes that Coder detects right now. Coder is a big help for them learning to write proper doc data types.

Now also pushed support for PHPStan general array notation.

Next: supporting complex array shapes of phpstan. Then I think we can call this issue fixed and follow-up with support for more PHPstan data types. We can then also discuss again if we should simply drop any data type validation in Coder.

I agree that it makes sense to research what PHPCS upstream is validating for doc data types, maybe we can use similar approaches.

+1 for #17: don’t try overcharging a linter with static anaysis; the best you’ll get is a paler dupe…

Fully agreed with you both - Coder cannot and will not do static analysis. We still need to do Coding standards checks on data types, for example that developers write "int" instead of "integer". That is the job of a linter.

That means we cannot drop data type validation completely from Coder. Instead we will adapt Coder to not contradict PHPStan and limit the validation to very basic things.

And pushed advanced array shape support tests (it already worked), plus integer range support.

Closing this issue is fixed now, but we are not done with all the possible things at https://phpstan.org/writing-php-code/phpdoc-types . So some further relaxing of validation will be necessary, please open new issues as you see fit.

We were previously running the MR as a patch and have updated to 8.3.18 but am now getting unexpected errors, it seems that commas are not accepted?

22 | ERROR | [x] Expected "array<intarray<intint>>" but found
    |       |     "array<int,array<int,int>>" for @var tag in member
    |       |     variable comment

Looks like @var docs aren't handled.

Thanks for reporting! Created #3355543: Allow arbitrary data types for PHPStan compatibility in @var class properties as follow-up for that.

It looks like #24 addressed @var annotations, but (at least) not @return ones: https://www.drupal.org/pift-ci-job/2730350, #3364706: Refactor transactions

Filed #3378528: Allow arbitrary data types for PHPStan compatibility in @return class methods