Support advanced PHPStan data types (general, arrays)

Coming from 📌 [PP-1] Support the recommended phpdoc syntax for documenting Iterator return types Closed: duplicate 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.

At what point do we allow any arbitrary data to be before the dollar-sign, or any data until a new line (in the case of returns)?

Or should we even be validating this?

What are other linting projects doing in this regard? Seems like it should be a solved problem, or one we can ignore.

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.

Another thought: now that we have PHPStan becoming increasingly tied in, make this a responsibility for static analysis to enforce. If static analysis doesn't recognize types, then rely on those tools for errors.

I think it makes sense to defer responsibilities previously held by linters, onto static analysis.

+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.

for example that developers write "int" instead of "integer". That is the job of a linter.

Detecting obvious typos, or detecting and being able to fix (cbf) unapproved variants im totally in favor of.

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 ✨ Allow arbitrary data types for PHPStan compatibility in @var class properties Fixed as follow-up for that.

Automatically closed - issue fixed for 2 weeks with no activity.

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

Filed ✨ Allow arbitrary data types for PHPStan compatibility in @return class methods Active