Implement rule from #105

[udovicic]

Implementation of rule from #105

Additionally, to avoid duplicated code, I have refactored ConstantsPHPDocFormattingSniff to make it reusable.

[lenaorobei]

@udovicic thanks for opening this PR! It's going to be an awesome improvement, however I have few comments to make it even better 🙂

1. Let us be more strict with classes and interfaces. What I mean is that Class should have short description AND this description should be different form the class name. Both following examples should raise a warning.

class NotAnErrorHandler
{
}

/**
 * Faulty Handler
 */
class FaultyHandler
{
}

2. Please add case with empty DocBlock.

/**
 *
 */
class FaultyHandler
{
}

3. As per documentation some tags are forbidden. It would be great to include these in this check.
   @author ,@category, @package, and @subpackage MUST NOT be used. Documentation is organized with the use of namespaces.

4. Want to share with you the core rule we are trying to replace (because of some bugs and wrong behaviour), it might help with implementation details (like expected messages) and PHPCS out-of-the-box ClassCommentSniff rule. This one might help with proper formatting and forbidden tags.

[udovicic]

Hi @lenaorobei ,

I am fine with the rest, but regarding the point 1: Definition says that class should have a short description, but that is not a must. Furthermore, in devdocs and the original issue, it actually says:

If the short description adds no additional information beyond what the type name already supplies, the short description must be omitted.

Therefore, I think that parth should remain as is

[lenaorobei]

@udovicic good catch! 🙂 Let me confirm this question with architecture team. My only one concern is that if we will allow no description, nobody will add any description at all.

@paliarush @melnikovi need your input here since you were working on requirements for annotation rules.

[lenaorobei]

@udovicic I raised my concerns regarding missing PHPDoc Blocks magento/architecture#222 but got confirmation that your approach is correct and this is expected behaviour. Sorry for the confusion.

One more thing. I run your branch against Magento2 app/code and found some false-positives.

/**
 * @api
 * @since 100.0.2
 */
class Main extends \Magento\Backend\Block\Template
{}

This code throws a warning, but it shouldn't.

[udovicic]

@lenaorobei Not sure how I missed that one, but it has been added as a test case and code has been updated to reflect that.

Everything else should be in there now. I understand that removing description from those classes is not optimal solution, but may come as future refactoring or something, I didn't consider it to be part of the original issue I am trying to resolve here.

[lenaorobei]

Hm, that's odd, but now these examples raise warnings.

/**
 * @api
 */
interface WysiwygModifierInterface
{}

/**
 * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
 */
class GroupRepositoryTest extends \PHPUnit\Framework\TestCase
{}

It might be related to the string token after the tag.

[udovicic]

@lenaorobei I have added the suggested changes and fixed the latest issue. Tag detection was all wrong previously, it should be fine now (I have also added the test cases for it).