php-cs-fixer is not properly pinned

[erayd]

php-cs-fixer is still trying to apply new, breaking changes to the source - in this case it's removing parts of the documentation comments - despite #537 which was explicitly intended to prevent this.

/CC @localheinz

[erayd]

/CC @SignpostMarv also, as you were the last person to touch this in #575.

[SignpostMarv]

Is this regarding that annoying habit php-cs-fixer has of removing non-starred lines in multi-line docblock tags ?

[erayd]

@SignpostMarv Based on a quick look at what it's doing, it seems to be removing argument lines with a technically invalid type - which are deliberately the way they are to be more descriptive (e.g. string|null).

[localheinz]

@erayd

Can you give an example?

[SignpostMarv]

@erayd this seems to be fine. There'll be rules to configure to change the behaviour. However....

@param tags with no description that are directly match the code are redundant.

  10) tests\Constraints\TypeTest.php (no_superfluous_phpdoc_tags, phpdoc_align)
      ---------- begin diff ----------
--- Original
+++ New
@@ -69,8 +69,7 @@
     /**
      * Helper to assert an error message
      *
-     * @param string         $expected
-     * @param TypeConstraint $actual
+     * @param string $expected
      */
     private function assertTypeConstraintError($expected, TypeConstraint $actual)
     {

mid-line indentation is a matter of taste, although additionally I'm going to hazard a guess most of these should be more specific than mixed.

   5) src\JsonSchema\Constraints\UndefinedConstraint.php (no_superfluous_phpdoc_tags, phpdoc_align)
      ---------- begin diff ----------
--- Original
+++ New
@@ -55,10 +55,9 @@
     /**
      * Validates the value against the types
      *
-     * @param mixed       $value
-     * @param mixed       $schema
-     * @param JsonPointer $path
-     * @param string      $i
+     * @param mixed  $value
+     * @param mixed  $schema
+     * @param string $i
      */

As with the single-typed redundant line removal, T|null is a redundant line when the actual parameter matches the specified type, i.e. JsonPointer|null $path

   2) src\JsonSchema\Constraints\Constraint.php (no_superfluous_phpdoc_tags, phpdoc_trim, phpdoc_align)
      ---------- begin diff ----------
--- Original
+++ New
@@ -56,10 +56,9 @@
     /**
      * Validates an array
      *
-     * @param mixed            $value
-     * @param mixed            $schema
-     * @param JsonPointer|null $path
-     * @param mixed            $i
+     * @param mixed $value
+     * @param mixed $schema
+     * @param mixed $i
      */

[erayd]

@SignpostMarv I don't care that they are redundant; I don't want them removed. They will eventually have descriptions (when somebody has the time to finish cleaning up the docs), but in the meantime I still don't want php-cs-fixer removing parameters. Indentation isn't a problem; it seems to be complying with our existing scheme for them.

My main annoyance with this is php-cs-fixer is yet again changing its behavior without warning. That was the whole reason we pinned it in the first place, as in the past some of those changes included logical changes to the code, not just cleanup (i.e. it was making assumptions and changing things in a way that would have caused the validation logic to change and no longer comply properly with the spec). It might be changing "just comments" this time, but it's still making assumptions and changing stuff that it previously left alone - in this case, including the deletion of lines that are there deliberately.

[DannyvdSluijs]

With the current support of PHP: ^7.2 || ^8.0 we can upgrade to friendsofphp/php-cs-fixer version 3.3.0 which still has support for PHP 7.1 as the next minor release bumps the required PHP version to 7.2.5