Add type declarations to code examples

[derrabus]

The Symfony codebase is being modernized by adding type declarations. I think that we should do the same here as well.

My suggestions:

• Example code that does not override/implement a Symfony method, should be typed.
• Example code that overrides/implements a Symfony method should be updated for 5.0 to match the added parameter type declarations.
• Example code that overrides/implements a Symfony method should turn an upstream @return annotation into an explicit return type declaration if possible. In most cases, we can do this for the 4.4 docs already.

Related:

• [5.0] Add parameter type-hints symfony#32179
• [5.0] Add return type declarations symfony#33228

[javiereguiluz]

I'd love to have this ... but I'd hate to do this. Adding types at once would create millions of conflicts when merging this in all the upper branches. We don't have resources to do that, so that's why we can't do it.

However, we should start adding types to the new code and new examples added in 5.0 and higher branches.

[wouterj]

I don't think this will create merge conflicts: (1) will be updated in all maintained versions, so there are no conflicts. For (2) and (3), it can cause some merge conflicts between 4.3 and 4.4, but this is minimal as method signatures do not change much between Symfony versions (BC promise), thus changes in these lines for 4.3 hardly conflict with change in the same line in 4.4.

I'm happy to start working on this, promoting industry best practices is the best thing we can do in the docs :).

@derrabus you said "In most cases, we can do this for the 4.4 docs already." about the return type hints. Can you explain/show some examples of which cases this is not possible? (I could not find it in symfony/symfony#33228 )

[derrabus]

(1) will be updated in all maintained versions, so there are no conflicts.

By "all maintained versions" you also mean 3.4? Since Symfony 3.4 still supports php 5, I'm not sure if we should have code examples there using syntax exclusive to php 7.

For (2) and (3), it can cause some merge conflicts between 4.3 and 4.4, but this is minimal as method signatures do not change much between Symfony versions (BC promise), thus changes in these lines for 4.3 hardly conflict with change in the same line in 4.4.

(2) can only be done on 5.0 because the added parameter type declarations are not present in 4.4 (mainly because php 7.1 doesn't allow parameter type widening).

I'm happy to start working on this, promoting industry best practices is the best thing we can do in the docs :).

I'd also be happy to help here. Maybe we can find a way to organize/split the work here, in case others want to join as well.

@derrabus you said "In most cases, we can do this for the 4.4 docs already." about the return type hints. Can you explain/show some examples of which cases this is not possible? (I could not find it in symfony/symfony#33228 )

If we add return types to implementations of Symfony interfaces, we would anticipate the return type declaration that Symfony 6 will add eventually. That's a deep dive into the crystal ball, so in cases where we're not sure how a @return will be turned into a return type declraration, we should probably not add it.

• The object return type would be a problem if we want to keep code snippets of the 4.4 docs compatible with php 7.1.
• Especially when extending fluent interfaces, adding self as return type is problematic as long as php does not support return type covariance (has been added in 7.4).
• There were some other cases where the return type was unclear, but by several deprecations have paved the way for us in the meantime (e.g. [Console] Add deprecation message for non-int statusCode symfony#33775, [SecurityGuard] Deprecate returning non-boolean values from checkCredentials() symfony#33308). We might find other cases while working on this issue. 😃

In all other cases, we should be pretty safe now because on the 4.4 and 5.0 branches, Travis runs Symfony's test suite on php 7.4 against a version of Symfony that is patched with return types inferred from @return annotations.

[wouterj]

Alright, I shortly discussed this with the rest of the team and we agreed on keeping PHP 7.1 compatible code in the Symfony 4.x docs (many people are probably still on PHP 7.1, so it's better to avoid confusion).

So this means the work that has to be done is:

1. All types that do not extend Symfony code: Fix in 4.4 and master
2. Parameter types: Fix in master
3. Return types: Fix in 4.4 and master

I would suggest to split this work on the "topics" and to start focussing on the main guides in the root and waiting for the components docs till the very end.

[wouterj]

Fyi, I started working on this issue in #13564 . For now, the changes seems quite minor (but getting started guides mostly discuss the controller).

[wouterj]

Related to (2) #12867

[derrabus]

Great, thanks @atailouloute!

[OskarStark]

I think you meant "@wouterj" instead? 😋