feat: Configurable phpDoc tags for FQCN processing #7628
Conversation
Wirone
added
kind/enhancement
topic/fqcn
|
|
||
| Allowed types: ``array`` | ||
|
|
||
| Default value: ``['param', 'phpstan-param', 'phpstan-property', 'phpstan-property-read', 'phpstan-property-write', 'phpstan-return', 'phpstan-var', 'property', 'property-read', 'property-write', 'psalm-param', 'psalm-property', 'psalm-property-read', 'psalm-property-write', 'psalm-return', 'psalm-var', 'return', 'see', 'throws', 'var']`` |
There was a problem hiding this comment.
LGTM, I would personally remove @see which is required to be FQCN in some coding standards and very often is not a FQCN alone, it is very often a method name, which is not a "type" which this fixer is about.
There was a problem hiding this comment.
Yes, but as a second condition fixer matches simple FQCNs with a regex, so URLs or methods won't be processed 🙂.
| 'phpstan-param', | ||
| 'phpstan-property', | ||
| 'phpstan-property-read', | ||
| 'phpstan-property-write', |
There was a problem hiding this comment.
what would be the case of phpstan-property-read enabled, but phpstan-property-write disabled?
imagine someone actually use the config, phpstan introducing new annotation - they need to manually add the annotation here.
I would rather have a group of annotations like @phpstan or phpstan* for all phpstan-based annotations
(same for others)
There was a problem hiding this comment.
There are more phpstan-* and psalm-* annotations, but these have different purpose. I only included these related to types, tool-specific variants of regular phpDoc ones. I doubt new annotations will emerge 🙂.
Maybe we can focus on regular phpDoc tags, and automatically add prefixed ones 🤔? Like when someone includes param and automatically phpstan-param and psalm-param are included. It would be more logical to me than wildcards, because you may want to process param but not var (prefixed variants). The other alternative is to ignore tool-specific variants in default config and just let people configure it by themselves, but as far as I remember other fixers handle these too.
There was a problem hiding this comment.
like, tbh I see only 2 cases myself - want to process annotations (any of them) or not. boolean config - do we have request to go granular and support some, but not all annotations?
There was a problem hiding this comment.
For example @see is something that I think can be processed, and there was a voice it shouldn't be. Also, having this configurable makes it possible to clear large codebases step by step (by enabling one tag after another and providing partial diffs). List of tags was previously hardcoded and there was a request to add new tags - having it configurable allows people to do it in a flexible way, without changing the fixer's code.
|
It may be better to provide a list of tags to disallow this for, and then a flag to enable or not fixing at all in phpdoc. There are use cases also for handling names inside phpdoc but outside of the context of a tag. |
|
I am not sure if blacklist is better than whitelist, since these are a lot of phpDoc tags (or userland annotations) that should not be processed. In terms of on/off switch I plan to introduce another config option that will handle this, so here let's focus on the scope of the phpDoc processing. |
Fixes #7620 by providing ability to choose which phpDoc tags should be processed (in that case
@seecan be omitted and than problem is solved). I chose the approach with configurable list of phpDoc tags because it's more flexible - not only it allows disabling phpDoc support completely, but also allows narrowing/widening default scope by providing custom list of tags.