Browse Source

DX: test "isRisky" method in fixer tests, not as auto review

Kuba Werłos 4 years ago
parent
commit
66b37df450

+ 1 - 1
README.rst

@@ -1596,7 +1596,7 @@ Choose from the list of available rules:
   EXPERIMENTAL: Takes ``@return`` annotation of non-mixed types and adjusts
   accordingly the function signature. Requires PHP >= 7.0.
 
-  *Risky rule: [1] This rule is EXPERIMENTAL and is not covered with backward compatibility promise. [2] ``@return`` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] ``@inheritdocs`` support is under construction.*
+  *Risky rule: this rule is EXPERIMENTAL and [1] is not covered with backward compatibility promise. [2] ``@return`` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] ``@inheritdocs`` support is under construction.*
 
   Configuration options:
 

+ 1 - 1
src/Fixer/FunctionNotation/PhpdocToReturnTypeFixer.php

@@ -112,7 +112,7 @@ function my_foo()
                 ),
             ],
             null,
-            '[1] This rule is EXPERIMENTAL and is not covered with backward compatibility promise. [2] `@return` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] `@inheritdocs` support is under construction.'
+            'This rule is EXPERIMENTAL and [1] is not covered with backward compatibility promise. [2] `@return` annotation is mandatory for the fixer to make changes, signatures of methods without it (no docblock, inheritdocs) will not be fixed. [3] Manual actions are required if inherited signatures are not properly documented. [4] `@inheritdocs` support is under construction.'
         );
     }
 

+ 0 - 8
src/Fixer/Phpdoc/PhpdocAddMissingParamAnnotationFixer.php

@@ -94,14 +94,6 @@ function f9(string $foo, $bar, $baz) {}
         return $tokens->isTokenKindFound(T_DOC_COMMENT);
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function isRisky()
-    {
-        return false;
-    }
-
     /**
      * {@inheritdoc}
      */

+ 0 - 7
tests/AutoReview/FixerTest.php

@@ -170,12 +170,6 @@ final class FixerTest extends TestCase
                 static::assertRegExp('/^[a-z_]+[a-z]$/', $option->getName(), sprintf('[%s] Option %s is not snake_case.', $fixerName, $option->getName()));
             }
         }
-
-        if ($fixer->isRisky()) {
-            self::assertValidDescription($fixerName, 'risky description', $definition->getRiskyDescription());
-        } else {
-            static::assertNull($definition->getRiskyDescription(), sprintf('[%s] Fixer is not risky so no description of it expected.', $fixerName));
-        }
     }
 
     /**
@@ -294,7 +288,6 @@ final class FixerTest extends TestCase
         $emptyTokens = new Tokens();
 
         static::assertInternalType('int', $fixer->getPriority(), sprintf('Return type for ::getPriority of "%s" is invalid.', $fixer->getName()));
-        static::assertInternalType('bool', $fixer->isRisky(), sprintf('Return type for ::isRisky of "%s" is invalid.', $fixer->getName()));
         static::assertInternalType('bool', $fixer->supports(new \SplFileInfo(__FILE__)), sprintf('Return type for ::supports of "%s" is invalid.', $fixer->getName()));
 
         static::assertInternalType('bool', $fixer->isCandidate($emptyTokens), sprintf('Return type for ::isCandidate with empty tokens of "%s" is invalid.', $fixer->getName()));

+ 0 - 7
tests/Fixer/ConstantNotation/NativeConstantInvocationFixerTest.php

@@ -110,13 +110,6 @@ final class NativeConstantInvocationFixerTest extends AbstractFixerTestCase
         $this->doTest($after, $before);
     }
 
-    public function testIsRisky()
-    {
-        $fixer = $this->createFixer();
-
-        static::assertTrue($fixer->isRisky());
-    }
-
     /**
      * @dataProvider provideFixWithDefaultConfigurationCases
      *

+ 0 - 7
tests/Fixer/FunctionNotation/NativeFunctionInvocationFixerTest.php

@@ -167,13 +167,6 @@ PHP;
         $this->doTest($after, $before);
     }
 
-    public function testIsRisky()
-    {
-        $fixer = $this->createFixer();
-
-        static::assertTrue($fixer->isRisky());
-    }
-
     /**
      * @dataProvider provideFixWithDefaultConfigurationCases
      *

+ 49 - 0
tests/Test/AbstractFixerTestCase.php

@@ -13,6 +13,7 @@
 namespace PhpCsFixer\Tests\Test;
 
 use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\AbstractProxyFixer;
 use PhpCsFixer\Linter\CachingLinter;
 use PhpCsFixer\Linter\Linter;
 use PhpCsFixer\Linter\LinterInterface;
@@ -66,6 +67,29 @@ abstract class AbstractFixerTestCase extends TestCase
         Tokens::setLegacyMode(false);
     }
 
+    final public function testIsRisky()
+    {
+        static::assertInternalType('bool', $this->fixer->isRisky(), sprintf('Return type for ::isRisky of "%s" is invalid.', $this->fixer->getName()));
+
+        if ($this->fixer->isRisky()) {
+            self::assertValidDescription($this->fixer->getName(), 'risky description', $this->fixer->getDefinition()->getRiskyDescription());
+        } else {
+            static::assertNull($this->fixer->getDefinition()->getRiskyDescription(), sprintf('[%s] Fixer is not risky so no description of it expected.', $this->fixer->getName()));
+        }
+
+        if ($this->fixer instanceof AbstractProxyFixer) {
+            return;
+        }
+
+        $reflection = new \ReflectionMethod($this->fixer, 'isRisky');
+
+        // If fixer is not risky then the method `isRisky` from `AbstractFixer` must be used
+        static::assertSame(
+            !$this->fixer->isRisky(),
+            AbstractFixer::class === $reflection->getDeclaringClass()->getName()
+        );
+    }
+
     /**
      * @return AbstractFixer
      */
@@ -207,4 +231,29 @@ abstract class AbstractFixerTestCase extends TestCase
 
         return $linter;
     }
+
+    /**
+     * @param string $fixerName
+     * @param string $descriptionType
+     * @param mixed  $description
+     */
+    private static function assertValidDescription($fixerName, $descriptionType, $description)
+    {
+        static::assertInternalType('string', $description);
+        static::assertRegExp('/^[A-Z`][^"]+\.$/', $description, sprintf('[%s] The %s must start with capital letter or a ` and end with dot.', $fixerName, $descriptionType));
+        static::assertNotContains('phpdocs', $description, sprintf('[%s] `PHPDoc` must not be in the plural in %s.', $fixerName, $descriptionType), true);
+        static::assertCorrectCasing($description, 'PHPDoc', sprintf('[%s] `PHPDoc` must be in correct casing in %s.', $fixerName, $descriptionType));
+        static::assertCorrectCasing($description, 'PHPUnit', sprintf('[%s] `PHPUnit` must be in correct casing in %s.', $fixerName, $descriptionType));
+        static::assertFalse(strpos($descriptionType, '``'), sprintf('[%s] The %s must no contain sequential backticks.', $fixerName, $descriptionType));
+    }
+
+    /**
+     * @param string $needle
+     * @param string $haystack
+     * @param string $message
+     */
+    private static function assertCorrectCasing($needle, $haystack, $message)
+    {
+        static::assertSame(substr_count(strtolower($haystack), strtolower($needle)), substr_count($haystack, $needle), $message);
+    }
 }