Grav: Question for contributions about code improvements

[belgattitude]

Dear Grav Team,

I'm interested to contribute to the code... I recently introduced phpstan initial support, but before sending more patches, I want to know your opinions about typings issues. I'm aware that you're working fully to release 1.6 and I don't want to make your work harder or introdce bc-breaks. So here's some thoughts, feel free to comment.

(I'm aware and agree that "strict_types=1" could break things, and I feel it's good to not work with strict_types for now.)

1. Method arguments

I would like to replace phpdoc style typings in methods arguments. For example:

/**
 * @param string|null $param1
 */
function test($param1=null) {}

// by 
function test(?string $param1=null) {}

To preserve the idea of 'single source of truth', I'll remove what's obsolete from phpdoc (excluding array, see point 3).

2 Introduce return types

I'm aware that covariance problems can break things, but for simple cases (scalar or arrays) I would like to add return type

function sayHello(): string { return 'hello' }

3. Basic typing of arrays

Where applicable, I would like to replace @param array $languages by @param string[] $languages in phpdoc (the function argument will still be types 'array'). VSCode or phpstorm support this, but some people does not like so much. It makes static analyzers shines.

Hope it makes senses. Let me know you thoughts

[mahagr]

I already tried the same, but introducing strong typing in arguments and return types break things unless you're using PHP 7.2 and in only if the classes aren't extended either by loosening the argument type or tightening the return type.

Unfortunately, as we do support PHP 7.1, I ended up breaking a lot of code by making the type changes, so I just ended up doing it with some newer code which isn't likely to be used anywhere.

Regarding the last point (3), yes, that is what I'm already doing with new code.

---

What do we need the most is to fix the obvious mistakes in the docblocks, improve generated documentation and add unit tests to make sure that the methods work as expected.

In fact, right now the documentation (and testing) is the only thing really blocking the 1.6 release, so in short term, we should just look for improving generated documentation and fixing the obvious bugs.

[rhukster]

I fixed the last couple of level 1 issues (showed up for me in PHP 7.3). I think we should really hold off on the strong typing until Grav 2.0 when we can require PHP 7.2 or greater.

[mahagr]

I fixed most of the level 2 issues, there are some which need a deeper look.

[belgattitude]

You're in lion mode ;)

knew about classes problems, but was not aware of issues with scalar and arrays (https://3v4l.org/WPdCK . Viva PHP 7.2 ! ;) , you're fully right.

Kudos for level 2, level 3 will need fixes in docblocks to get full-power from static analysis. It's generally where we find the little tricky bugs we like so much. I'll have a look when I have time.

Can I suggest to put final on all new 1.6 classes (Flex...) that have no reason to be extended (opening later if needed) Will be safer for next refactorings.

Thanks for all your work !

[mahagr]

Grav\Framework has been fixed up to level 5 now. :) Though I just fixed the issues found by the analysis and the ones I picked up while looking into the code, so there are likely bad types here and there.

Rest of the code is at level 2, they are trickier as there's a lot of older code.

Most of the Flex classes are meant to be extended as you can add custom features and behaviour for your objects and collections.

[belgattitude]

Nice ! I wanted to help but you're too fast ;) I'll find something out, haha.

Idea: add a little badge to the readme

![PHPStan](https://img.shields.io/badge/style-level%205-brightgreen.svg?style=flat-square&label=phpstan)

I like badges ;)

[mahagr]

It's already there, but without level. And we're really at level 2. :)

I stopped working on this, I'm now going through interfaces and documenting them.

[mahagr]

Cleaning up issues, I think this one is sorted out :)