PHP-CS-Fixer: PHPUnit annotations for ignoring code coverage are reformatted incorrectly

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

$ php -v
PHP 7.2.7 (cli) (built: Jun 19 2018 14:40:10) ( NTS )
Copyright (c) 1997-2018 The PHP Group
Zend Engine v3.2.0, Copyright (c) 1998-2018 Zend Technologies

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

$ php-cs-fixer -V
PHP CS Fixer 2.12.1 Long Journey by Fabien Potencier and Dariusz Ruminski (beef6cb)

Original

<?php
class C
{
    public function m()
    {
    // @codeCoverageIgnoreStart
    }
    // @codeCoverageIgnoreEnd

    public function n()
    {
    } // @codeCoverageIgnore
}

Output

   1) C.php (class_attributes_separation, braces)
      ---------- begin diff ----------
--- Original
+++ New
@@ -3,11 +3,14 @@
 {
     public function m()
     {
-    // @codeCoverageIgnoreStart
+        // @codeCoverageIgnoreStart
     }
+
     // @codeCoverageIgnoreEnd
 
     public function n()
     {
-    } // @codeCoverageIgnore
+    }
+
+    // @codeCoverageIgnore
 }

      ----------- end diff -----------

Result

<?php
class C
{
    public function m()
    {
        // @codeCoverageIgnoreStart
    }

    // @codeCoverageIgnoreEnd

    public function n()
    {
    }

    // @codeCoverageIgnore
}

While the multi-line annotations @codeCoverageIgnoreStart and @codeCoverageIgnoreEnd are only reformatted so that they look “ugly” (IMO), the single-line annotation @codeCoverageIgnore is reformatted so that it no longer has the desired effect.

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Comments: 16 (16 by maintainers)

Most upvoted comments

@sebastianbergmann , you have opened Pandora’s box…

IMO there is no need for a special treatment here, any comment after a closing brace should not be moved.

I would consider extending fixer class_attributes_separation with option like separate_comments (default to false).

The comment doesn’t belong to the method so it gets separated from it, which is what the rule is designed to do. If you don’t want to make an exception for these types of comments than I would advice disabling the rule.

If we are going to treat @codeCoverageIgnore differently to normal comments, it may be useful to find out if there are any other specific tags we should be ignoring too. I totally agree with @SpacePossum that there needs to be a unified style for ignores. I also agree with @julienfalque, it doesn’t seem like class_attributes_separation should be moving comments as they are not a class attribute.

Adding a blank line between the closing brace and the // @codeCoverageIgnore comment seems to be done by class_attributes_separation rule. I would say this a bug: the comment shouldn’t move at all.