magento / magento-coding-standard

Magento Coding Standard
Open Software License 3.0
349 stars 153 forks source link

[New Rule] Implement sniff for functions PHPDoc formatting #54

Open lenaorobei opened 5 years ago

lenaorobei commented 5 years ago

Rule

PHPDoc formatting for functions and methods.

Acceptance Criteria:

A short description in case it adds meaningful information beyond the method name.

If the purpose of the method is not obvious, a long description that explains the motivation behind the implementation. The comment must describe why method is implemented and not how. For example:

  • If a workaround or hack is implemented, explain why it is necessary and include any other details necessary to understand the algorithm.
  • For non-obvious implementations where the implementation logic is complicated or does not correspond to the Technical Vision or other known best practices, include an explanation in the doc block's description. An implementation is non-obvious if another developer has questions about it. The declaration of all arguments (if any) using @param tag, unless the argument type is indicated in the method signature. All @param annotations must include the appropriate argument type. If any argument requires a @param annotation, all arguments must be listed (all or none).

The @param annotations must be in the same order as the method arguments.

The declaration of the return type using the @return tag must only be added if the method return type signature does not supply all necessary information (see below for more information on return types).

Declaration of possibly thrown exception using @throws tag, if the actual body of function triggers throwing an exception. All occurrences of @throws in a DocBlock must be after any @param and @return annotations.

Exceptions to these rules:

  • Testing methods in Unit tests may have doc blocks to describe the purpose of the test, for example referencing github issues.
  • Test method annotations may include data providers and other testing annotations.
  • Non-testing methods may have a doc block with description according to the rules above.

The @inheritdoc tag SHOULD NOT be used. If a child class method requires a long description to explain its purpose, it may use @inheritdoc to indicate the new description is intended as an addition to the parent method description. In general such method overrides are a code smell and should be used as an incentive to make the code more self-documenting if possible.

<severity>5</severity>
<type>warning</type>
larsroettig commented 5 years ago
<?php

namespace Foo\Bar;

interface TestInterface
{
    /**
     * Test Function with nice Doc
     *
     * @param $int
     * @param $string
     * @return string
     */
    public function test($int, $string);
}

class Test implements TestInterface
{
    /**
     * @inheritDoc
     */
    public function test($int, $string)
    {
        return '';
    }

    /**
     * Test Function with nice Doc.
     *
     * @param $string
     * @return string
     */
    public function test2($string)
    {
        return '';
    }
}

class  TestExtend extends Test implements TestInterface
{
    /**
     * Test Function with a better Function.
     *
     * {@inheritDoc}
     */
    public function test($int, $string)
    {
        return '';
    }

    /**
     * Sniff will find doc error there because you define inherit wrong.
     * @inheritDoc
     */
    public function test2($string)
    {
        return '';
    }
}
lenaorobei commented 5 years ago

@larsroettig, few comments to your example:

The @inheritdoc tag SHOULD NOT be used.

The declaration of the return type using the @return tag must only be added if the method return type signature does not supply all necessary information (see below for more information on return types).

Declaration of possibly thrown exception using @throws tag, if the actual body of function triggers throwing an exception. All occurrences of @throws in a DocBlock must be after any @param and @return annotations.

lenaorobei commented 5 years ago

Test should NOT fail. This example contains negative scenarios:

<?php

class EverythingIsGoodHere
{
    /**
     * @param string $arg1
     * @param bool $arg2
     */
    private function missingArgumentTypeSoParamsNeedToBeAdded($arg1, $arg2): bool
    {

    }

    /**
     * @return bool
     */
    private function presentArgumentTypeSoAnnotationNotNeededButMissingReturnType(string $arg1, bool $arg2)
    {

    }

    private function presentArgumentAndReturnTypes(string $arg1, bool $arg2): bool
    {

    }

    /**
     * @throws Exception
     */
    private function throwsAnException(): void
    {
        throw new \Exception();
    }

    /**
     * We have an additional wonderful message here.
     *
     * @return void
     */
    private function presentGoodDescription(string $arg1, bool $arg2)
    {

    }
}

Test should fail: This example contains positive scenarios:

<?php

class EverythingIsBadHere
{
    /**
     * @param bool $arg2
     * @param string $arg1
     */
    private function wrongParamOrder($arg1, $arg2): bool
    {

    }

    /**
     * @param bool $arg2
     */
    private function missingParamAnnotation($arg1, $arg2): bool
    {

    }

    private function missingArgumentAnnotations($arg1, $arg2): bool
    {

    }

    /**
     * @param string $arg1
     * @param bool $arg2
     * @param bool $arg2
     */
    private function paramDuplication($arg1, $arg2): bool
    {

    }

    /**
     * @inheritdoc
     */
    private function inheritDocShouldNotBeUsed(string $arg1, bool $arg2)
    {

    }

    /**
     * @param $arg1
     * @param $arg2
     */
    private function missingParamType($arg1, $arg2): bool
    {

    }

    /**
     * Useless description
     */
    private function uselessDescription(string $arg1, bool $arg2): bool
    {

    }

    /**
     *
     */
    private function dockBlockIsEmpty(string $arg1, bool $arg2): bool
    {

    }

    /**
     *
     *
     * @param string $arg1
     * @param bool $arg2
     *
     */
    private function redundantLines($arg1, $arg2): bool
    {

    }
}

@Vinai would appreciate much if you could review these examples.