Php-cs-fixer: NoSuperfluousPhpdocTagsFixer - split overcomplete tag remove to new fixer

[OskarStark]

For configuration or updating questions please read the README and UPGRADE documentation,
or visit: https://gitter.im/PHP-CS-Fixer

When reporting an issue (bug) please provide the following information:

The PHP version you are using ($ php -v):

PHP 7.1.16 (cli) (built: Mar 31 2018 02:59:59) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2018 Zend Technologies

PHP CS Fixer version you are using ($ php-cs-fixer -V):

PHP CS Fixer 2.12.2 Long Journey by Fabien Potencier and Dariusz Ruminski

The command you use to run PHP CS Fixer:

vendor/bin/php-cs-fixer fix

The configuration file you are using, if any:

<?php

$finder = PhpCsFixer\Finder::create()
    ->in('src')
    ->in('tests')
    ->exclude('Migrations')
;

return PhpCsFixer\Config::create()
    ->setRiskyAllowed(true)
    ->setRules([
        '@Symfony' => true,
        'ordered_imports' => true,
        'no_unused_imports' => true,
        'psr4' => true,
        'psr0' => true,
        'array_syntax' => ['syntax' => 'short'],
        'no_superfluous_phpdoc_tags' => true,
    ])
    ->setFinder($finder)
;

If applicable, please provide minimum samples of PHP code (as plain text, not screenshots):

• before running PHP CS Fixer (no changes):

    /**
     * @param Collection|Supplier[] $brands
     */
     public function setSuppliers($suppliers): self
     {
         $this->suppliers = $suppliers;

Note, that the PHP doc is wrong here, as $brands in the PHP doc should be $suppliers (copy paste mistake I guess)

• with unexpected changes applied when running PHP CS Fixer:

     public function setSuppliers($suppliers): self
     {
         $this->suppliers = $suppliers;

• with the changes you expected instead:

    /**
     * @param Collection|Supplier[] $brands
     */
     public function setSuppliers($suppliers): self
     {
         $this->suppliers = $suppliers;

If php doc is wrong, it should be kept, or am I wrong?

[julienfalque]

The rule removes superfluous @param tags but also those related to non-existant parameters. Maybe this is a bit out of the scope of the rule, but why would you keep such invalid tags?

[OskarStark]

Maybe this is a bit out of the scope of the rule

I aggree

but why would you keep such invalid tags?

keep is wrong, I would like to get get it fixed, but I guess the Fixer could not do this!

[SpacePossum]

so IIUC the fixer does what you want it to do, but it should have done so using two rules and not one?

[OskarStark]

Yes, two rules

[OskarStark]

Any news on this @SpacePossum ?

[SpacePossum]

Afraid not, at least I cannot recall someone following up on this ticket

[dmvdbrugge]

I disagree with the premise, because superfluous imho means everything that should not be there. Few thesaurus examples: extra, surplus, needless, unnecessary.

When looked at it that way, @param tags describing something that's not there kinda fall into that category, and should thus be removed. Doing so in a different fixer only causes mental and executional overhead.

[OskarStark]

Me too, that’s why another fixer could be used to achieve this

[dmvdbrugge]

Me too, that’s why another fixer could be used to achieve this

If this is your reply you did not understand what I said.

I'm disagreeing with you, there is absolutely no need to split the fixers and I'm very much against it for the reasons given.

[OskarStark]

Ok sorry then I misunderstood your comment

[SpacePossum]

I think https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/3933#issuecomment-406178481 is the way to go :)
We keep one rule, but allow to set it to not remove invalid-overcomplete tags?

[nicolas-grekas]

From #4504:

In Symfony, it's quite typical to find signatures like:

public function foo($a, $b/*, $c = null*/)

we do this when we want to add an argument in a BC-compatible way.
when we do this, we also add an @param ... $c in the docblock to be explicit.

no_superfluous_phpdoc_tags currently removes this @param, but we don't want it to do that.

[SpacePossum]

closed with https://github.com/FriendsOfPHP/PHP-CS-Fixer/pull/4505