Cleanup more @return annotations
#42499
Conversation
d828b2c to
8461acc
Compare
|
You removed all |
correct :) |
|
If psalm errors make sense to anyone, suggestions welcome to fix them :) |
src/Symfony/Component/Form/Extension/Core/DataTransformer/WeekToArrayTransformer.php
Outdated
Show resolved
Hide resolved
8461acc to
28af20f
Compare
28af20f to
6e53649
Compare
|
psalm should be happy now, thanks for the help! |
There was a problem hiding this comment.
A very superficial review (only looking at the PHPdoc and method name).
Stopped somewhere around the HttpKernel component, when I realized my scrollbar wasn't functioning as a progress bar (due to GitHub collapsing all other diffs) and I just passed hallway point. I also can't deny the fact that I left more comments at the start :)
src/Symfony/Bundle/FrameworkBundle/CacheWarmer/AbstractPhpFileCacheWarmer.php
Outdated
Show resolved
Hide resolved
| @@ -40,7 +40,7 @@ class XmlDumper extends Dumper | ||
| /** | ||
| * Dumps the service container as an XML string. |
There was a problem hiding this comment.
All these "dumps" are more "returns" right?
There was a problem hiding this comment.
yep, leaving as is for now
| @@ -572,7 +572,7 @@ public function children(string $selector = null) | ||
| /** | ||
| * Returns the attribute value of the first node of the list. | ||
| * | ||
| * @return string|null The attribute value or null if the attribute does not exist |
There was a problem hiding this comment.
quite important, as this indicates that null is a special case and does not have to mean the varabile is set to null?
There was a problem hiding this comment.
I don't see the need to highlight that: there is no difference between both situations if the variable can be set to null.
| @@ -43,7 +43,7 @@ interface EventSubscriberInterface | ||
| * The code must not depend on runtime state as it will only be called at compile time. | ||
| * All logic depending on runtime state must be put into the individual methods handling the events. | ||
| * | ||
| * @return array<string, mixed> The event names to listen to | ||
| * @return array<string, mixed> |
There was a problem hiding this comment.
| * @return array<string, mixed> | |
| * @return array<string, (string|list<string,int>|list<list<string,int>>)> |
There was a problem hiding this comment.
Added. but I'm wondering how useful such descriptions are. Looks quite cryptic.
There was a problem hiding this comment.
It's by definition a win for static analysers to have the most precise type information possible. I agree that it's no longer that human readable (although it can be helpful as I always forget if it's ['eventName', prio] or the other way around)
| @@ -1488,7 +1488,7 @@ public function isMethodIdempotent() | ||
| * | ||
| * @see https://tools.ietf.org/html/rfc7231#section-4.2.3 | ||
| * | ||
| * @return bool True for GET and HEAD, false otherwise |
There was a problem hiding this comment.
quite useful information to know which methods are considered "cacheable" by this method? (should probably be added to the normal description)
There was a problem hiding this comment.
The other similar methods don't have it. The implementation should know what cacheable is, not the description.
src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcherInterface.php
Show resolved
Hide resolved
|
All the adding of return types in 6.0 like #42504 should probably be done after this one has been merged upwards. |
6e53649 to
08d6964
Compare
|
Thanks for the reviews, PR updated accordingly.
I'll deal with the conflicts. I prefer decoupling PRs. |
08d6964 to
be47afc
Compare
be47afc to
8f36bbd
Compare
…kas) This PR was merged into the 5.4 branch. Discussion ---------- Cleanup more ``@return`` annotations | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | no | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Follow up of symfony#42495, same motivation. Automated edits, manual selection, happy review :P Commits ------- 8f36bbd Cleanup more ``@return`` annotations
Follow up of #42495, same motivation.
Automated edits, manual selection, happy review :P