Skip to content

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

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

Open
lenaorobei opened this issue Mar 4, 2019 · 3 comments
Open

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

lenaorobei opened this issue Mar 4, 2019 · 3 comments
Assignees
Labels
accepted New rule is accepted new rule New feature implementation Progress: dev in progress

Comments

@lenaorobei
Copy link
Contributor

lenaorobei commented Mar 4, 2019

Rule

PHPDoc formatting for functions and methods.

Acceptance Criteria:

Functions and methods should have:

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.

  • Rule covered by unit test.
<severity>5</severity>
<type>warning</type>
@lenaorobei lenaorobei added the accepted New rule is accepted label Mar 4, 2019
@lenaorobei lenaorobei changed the title [New rule] Implement sniffs for missing rule: @inheritdoc formatting [New Rule] Implement sniffs for missing rule: @inheritdoc formatting Mar 4, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniffs for missing rule: @inheritdoc formatting [New Rule] Implement sniffs @inheritdoc formatting Mar 4, 2019
@lenaorobei lenaorobei added the Progress: good first issue Issues is easy to get started with label Mar 4, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniffs @inheritdoc formatting [New Rule] Implement sniff @inheritdoc formatting Mar 5, 2019
@larsroettig larsroettig self-assigned this Mar 8, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniff @inheritdoc formatting [New Rule] Implement sniff functions formatting Mar 8, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniff functions formatting [New Rule] Implement sniff for functions formatting Mar 8, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniff for functions formatting [New Rule] Implement sniff for functions doc comment formatting Mar 8, 2019
@lenaorobei lenaorobei added the enhancement Improvements to existing rules label Mar 8, 2019
@lenaorobei lenaorobei changed the title [New Rule] Implement sniff for functions doc comment formatting [New Rule] Implement sniff for functions PHPDoc formatting Mar 8, 2019
@larsroettig
Copy link
Member

<?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
Copy link
Contributor Author

lenaorobei commented Aug 9, 2019

@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
Copy link
Contributor Author

lenaorobei commented Aug 9, 2019

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.

@sinisa86 sinisa86 self-assigned this Apr 8, 2020
sinisa86 added a commit to sinisa86/magento-coding-standard that referenced this issue Apr 10, 2020
sinisa86 added a commit to sinisa86/magento-coding-standard that referenced this issue Apr 10, 2020
magento-devops-reposync-svc pushed a commit that referenced this issue Sep 4, 2021
…-coding-standard-258

[Imported] AC-1100 Catch badly put newlines in LESS files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted New rule is accepted new rule New feature implementation Progress: dev in progress
Projects
None yet
Development

No branches or pull requests

3 participants