Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The ruleset force to add PHPDoc blocks for all methods #120

Open
Kwadz opened this issue Jan 8, 2018 · 12 comments
Open

The ruleset force to add PHPDoc blocks for all methods #120

Kwadz opened this issue Jan 8, 2018 · 12 comments

Comments

@Kwadz
Copy link

Kwadz commented Jan 8, 2018

The Symfony coding strandard documentation states:

Add PHPDoc blocks for all classes, methods, and functions (though you may be asked to remove PHPDoc that do not add value);

For example the inherited methods should not contain any PHPDoc comments, except if we want to add additional details.

@soullivaneuh
Copy link
Contributor

I agree. Furthermore, the rules should also sniff useless PHPdoc @param and @return lines like this:

class Test {
    /**
     * @param string $a
     * @param int $b with description.
     *
     * @return int
     */
    public function test(string $a, int $b): int
    {
        return 42;
    }
}

Only $b param notation should be kept because of the custom description.

@soullivaneuh
Copy link
Contributor

Maybe including this rule will be enough: https://github.com/slevomat/coding-standard#slevomatcodingstandardtypehintstypehintdeclaration-

@Kwadz
Copy link
Author

Kwadz commented Jan 30, 2018

PHPdoc @param and @return statements without description are not totally useless. It helps in cases without type declaration (PHP 7). Then I would keep the description as optional. What do you think?

@soullivaneuh
Copy link
Contributor

soullivaneuh commented Feb 17, 2018

It helps in cases without type declaration (PHP 7)

The rule I suggested removes the phpdoc only if useless. It wont touch anything if the PHP 7 typing is not used.

@Xen3r0
Copy link

Xen3r0 commented Jun 14, 2019

Hey! No plan for this rules ?

@tifabien
Copy link

tifabien commented Dec 2, 2019

PHP-CS-Fixer released a new version which configure the Symfony rule for removing PHPDocs when not necessary. At the moment we can't use both PHP-CS-Fixer and this project because the first one remove PHPDocs and the second is requiring them.

@wickedOne
Copy link
Contributor

you can add the following to your php-cs-fixer config

 'no_superfluous_phpdoc_tags' => false,

symfony coding standard still states:

Add PHPDoc blocks for all classes, methods, and functions (though you may be asked to remove PHPDoc that do not add value);

https://symfony.com/doc/current/contributing/code/standards.html#documentation

@wickedOne
Copy link
Contributor

additionally you should be able to add the following snippet to your phpcs.xml

<rule ref="Symfony.Commenting.FunctionComment.MissingParamTag">
    <severity>0</severity>
</rule>

@wickedOne
Copy link
Contributor

ah ic, you want the parameter requirement to be dropped raqther than the docblock requirement.
would make sense with php 7.4+ and property typing i guess...

@tifabien
Copy link

tifabien commented Dec 3, 2019

What I would like is to use the php-cs-fixer @Symfony rule (with the 'no_superfluous_phpdoc_tags' => ['allow_mixed' => true, 'allow_unused_params' => true] config). This way superfluous PHPDocs can be removed because they are redundant with typehinted parameters.
I assume that if I add the MissingParamTag config phpcs won't report error if an parameter is not typehinted and no @param is added in PHPDoc isn't it? IMHO it make sense to remove redundant PHPDocs as it will lighten the code.

@wickedOne
Copy link
Contributor

though i understand your sentiment, my problem with this is that it's not part of the coding standard / explicitly stated.

so i'd vote for removal of the paramter annotation as that's not explicitly stated as well, and just require a docblock regardless of it's contents, but would say that any additional / custom requirements for parameter annotations should be part of your private sniffs / config.

@djoos what are your thoughts on this?

@mmoll
Copy link
Contributor

mmoll commented Nov 7, 2020

Please see #183 for a start to use Slevomat sniffs instead of homegrown ones and comment there. Pulling in more sniffs would be super easy then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants