RFC/chore: deprecate Tokens::findGivenKind() when searching for multiple kinds #7600
Conversation
keradus
force-pushed
the
depr_aaa
branch
2 times, most recently
from
6e44fbf
to
4d1d344
Compare
| } | ||
|
|
||
| /** | ||
| * @param int|non-empty-list<int> $possibleKind kind or array of kinds |
There was a problem hiding this comment.
| * @param int|non-empty-list<int> $possibleKind kind or array of kinds | |
| * @param int $possibleKind |
to enforce the correct method softly/by static analysis
There was a problem hiding this comment.
not on v3, where both calls are allowed, but one is deprecated.
it can blow-up SCA analysis for repos of custom fixers.
There was a problem hiding this comment.
it can blow-up SCA analysis for repos of custom fixers.
Why is that a problem? It should be actually desired, as static analysis will tell the developers to use the other method, but the fixers will still run without issues as before.
There was a problem hiding this comment.
IMHO, when introducing such change, we shouldn't fail someone CI, but we should give them deprecation notice.
This could break builds for custom fixers on Fixer non-major upgrade. Types of param/return in docblock are considered as BC contract. Imagine method prototype only allowing array $param, and phpdoc changes @parma list<int> $param into @parma list<string> $param
There was a problem hiding this comment.
btw, consider to give opinion on RFC itself, not detail of phpdoc annotation.
|
Since this pull request has not had any activity within the last 90 days, I have marked it as stale. The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way. You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point. I will close it if no further activity occurs within the next 14 days. Please keep your branch up-to-date by rebasing it when main branch is ahead of it, thanks in advance! |
github-actions
bot
added
status/to verify
overall, I consider to replace the approach we introduced in past as shortcut of:
findOneOrMany(needle_oneOrMany)and have following instead
findOne(needle_one)findMany(needle_many)WDYT ?
We can have workaround, like
@return ($possibleKind is int ? array<int, Token> : array<int, array<int, Token>>), but it prevents stronger types at compile level