DX: Prevent having deprecated fixers listed as successors of other deprecated fixers (#7967)

Co-authored-by: Kuba Werłos <werlos@gmail.com>

doc/rules/basic/braces.rst

@@ -13,7 +13,7 @@ This rule is deprecated and will be removed in the next major version
 
 You should use ``single_space_around_construct``, ``control_structure_braces``,
 ``control_structure_continuation_position``, ``declare_parentheses``,
-``no_multiple_statements_per_line``, ``curly_braces_position``,
+``no_multiple_statements_per_line``, ``braces_position``,
 ``statement_indentation`` and ``no_extra_blank_lines`` instead.
 
 Configuration

src/Fixer/Basic/BracesFixer.php

@@ -51,9 +51,9 @@ final class BracesFixer extends AbstractProxyFixer implements ConfigurableFixerI
     public const LINE_SAME = 'same';
 
     /**
-     * @var null|CurlyBracesPositionFixer
+     * @var null|BracesPositionFixer
      */
-    private $curlyBracesPositionFixer;
+    private $bracesPositionFixer;
 
     /**
      * @var null|ControlStructureContinuationPositionFixer
@@ -156,7 +156,7 @@ class Foo
     {
         parent::configure($configuration);
 
-        $this->getCurlyBracesPositionFixer()->configure([
+        $this->getBracesPositionFixer()->configure([
             'control_structures_opening_brace' => $this->translatePositionOption($this->configuration['position_after_control_structures']),
             'functions_opening_brace' => $this->translatePositionOption($this->configuration['position_after_functions_and_oop_constructs']),
             'anonymous_functions_opening_brace' => $this->translatePositionOption($this->configuration['position_after_anonymous_constructs']),
@@ -217,7 +217,7 @@ class Foo
             $singleSpaceAroundConstructFixer,
             new ControlStructureBracesFixer(),
             $noExtraBlankLinesFixer,
-            $this->getCurlyBracesPositionFixer(),
+            $this->getBracesPositionFixer(),
             $this->getControlStructureContinuationPositionFixer(),
             new DeclareParenthesesFixer(),
             new NoMultipleStatementsPerLineFixer(),
@@ -225,13 +225,13 @@ class Foo
         ];
     }
 
-    private function getCurlyBracesPositionFixer(): CurlyBracesPositionFixer
+    private function getBracesPositionFixer(): BracesPositionFixer
     {
-        if (null === $this->curlyBracesPositionFixer) {
-            $this->curlyBracesPositionFixer = new CurlyBracesPositionFixer();
+        if (null === $this->bracesPositionFixer) {
+            $this->bracesPositionFixer = new BracesPositionFixer();
         }
 
-        return $this->curlyBracesPositionFixer;
+        return $this->bracesPositionFixer;
     }
 
     private function getControlStructureContinuationPositionFixer(): ControlStructureContinuationPositionFixer
@@ -246,7 +246,7 @@ class Foo
     private function translatePositionOption(string $option): string
     {
         return self::LINE_NEXT === $option
-            ? CurlyBracesPositionFixer::NEXT_LINE_UNLESS_NEWLINE_AT_SIGNATURE_END
-            : CurlyBracesPositionFixer::SAME_LINE;
+            ? BracesPositionFixer::NEXT_LINE_UNLESS_NEWLINE_AT_SIGNATURE_END
+            : BracesPositionFixer::SAME_LINE;
     }
 }

tests/RuleSet/RuleSetTest.php

@@ -23,6 +23,7 @@ use PhpCsFixer\FixerFactory;
 use PhpCsFixer\RuleSet\RuleSet;
 use PhpCsFixer\RuleSet\RuleSetDescriptionInterface;
 use PhpCsFixer\RuleSet\RuleSets;
+use PhpCsFixer\Tests\Test\TestCaseUtils;
 use PhpCsFixer\Tests\TestCase;
 
 /**
@@ -85,11 +86,7 @@ final class RuleSetTest extends TestCase
      */
     public function testThatDefaultConfigIsNotPassed(string $setName, string $ruleName, $ruleConfig): void
     {
-        $factory = new FixerFactory();
-        $factory->registerBuiltInFixers();
-        $factory->useRuleSet(new RuleSet([$ruleName => true]));
-
-        $fixer = current($factory->getFixers());
+        $fixer = TestCaseUtils::getFixerByName($ruleName);
 
         if (!$fixer instanceof ConfigurableFixerInterface || \is_bool($ruleConfig)) {
             $this->expectNotToPerformAssertions();
@@ -119,11 +116,7 @@ final class RuleSetTest extends TestCase
      */
     public function testThatThereIsNoDeprecatedFixerInRuleSet(string $setName, string $ruleName): void
     {
-        $factory = new FixerFactory();
-        $factory->registerBuiltInFixers();
-        $factory->useRuleSet(new RuleSet([$ruleName => true]));
-
-        $fixer = current($factory->getFixers());
+        $fixer = TestCaseUtils::getFixerByName($ruleName);
 
         self::assertNotInstanceOf(DeprecatedFixerInterface::class, $fixer, sprintf('RuleSet "%s" contains deprecated rule "%s".', $setName, $ruleName));
     }

tests/RuleSet/RuleSetsTest.php

@@ -14,12 +14,11 @@ declare(strict_types=1);
 
 namespace PhpCsFixer\Tests\RuleSet;
 
-use PhpCsFixer\AbstractFixer;
 use PhpCsFixer\Fixer\ConfigurableFixerInterface;
 use PhpCsFixer\Fixer\PhpUnit\PhpUnitTargetVersion;
-use PhpCsFixer\FixerFactory;
 use PhpCsFixer\RuleSet\RuleSet;
 use PhpCsFixer\RuleSet\RuleSets;
+use PhpCsFixer\Tests\Test\TestCaseUtils;
 use PhpCsFixer\Tests\TestCase;
 
 /**
@@ -187,7 +186,7 @@ Integration of %s.
     {
         $maximumVersionForRuleset = preg_replace('/^@PHPUnit(\d+)(\d)Migration:risky$/', '$1.$2', $setName);
 
-        $fixer = self::getFixerByName($ruleName);
+        $fixer = TestCaseUtils::getFixerByName($ruleName);
 
         self::assertInstanceOf(ConfigurableFixerInterface::class, $fixer, sprintf('The fixer "%s" shall be configurable.', $fixer->getName()));
 
@@ -285,7 +284,7 @@ Integration of %s.
     private function getDefaultPHPUnitTargetOfRule(string $ruleName): ?string
     {
         $targetVersion = null;
-        $fixer = self::getFixerByName($ruleName);
+        $fixer = TestCaseUtils::getFixerByName($ruleName);
 
         if ($fixer instanceof ConfigurableFixerInterface) {
             foreach ($fixer->getConfigurationDefinition()->getOptions() as $option) {
@@ -299,25 +298,4 @@ Integration of %s.
 
         return $targetVersion;
     }
-
-    private static function getFixerByName(string $name): AbstractFixer
-    {
-        $factory = new FixerFactory();
-        $factory->registerBuiltInFixers();
-        $factory->useRuleSet(new RuleSet([$name => true]));
-
-        $fixers = $factory->getFixers();
-
-        if (0 === \count($fixers)) {
-            throw new \RuntimeException('FixerFactory unexpectedly returned empty array.');
-        }
-
-        $fixer = current($fixers);
-
-        if (!$fixer instanceof AbstractFixer) {
-            throw new \RuntimeException(sprintf('Fixer class for "%s" rule does not extend "%s".', $name, AbstractFixer::class));
-        }
-
-        return $fixer;
-    }
 }

tests/Test/AbstractFixerTestCase.php

@@ -322,6 +322,27 @@ abstract class AbstractFixerTestCase extends TestCase
         }
     }
 
+    final public function testDeprecatedFixersDoNotHaveDeprecatedSuccessor(): void
+    {
+        if (!$this->fixer instanceof DeprecatedFixerInterface || [] === $this->fixer->getSuccessorsNames()) {
+            $this->addToAssertionCount(1);
+
+            return;
+        }
+
+        foreach ($this->fixer->getSuccessorsNames() as $successorName) {
+            self::assertNotInstanceOf(
+                DeprecatedFixerInterface::class,
+                TestCaseUtils::getFixerByName($successorName),
+                sprintf(
+                    'Successor fixer `%s` for deprecated fixer `%s` is deprecated itself.',
+                    $successorName,
+                    $this->fixer->getName(),
+                )
+            );
+        }
+    }
+
     /**
      * Blur filter that find candidate fixer for performance optimization to use only `insertSlices` instead of multiple `insertAt` if there is no other collection manipulation.
      */

tests/Test/TestCaseUtils.php

@@ -14,6 +14,9 @@ declare(strict_types=1);
 
 namespace PhpCsFixer\Tests\Test;
 
+use PhpCsFixer\Fixer\FixerInterface;
+use PhpCsFixer\FixerFactory;
+
 /**
  * @internal
  */
@@ -38,4 +41,25 @@ final class TestCaseUtils
             yield $case;
         }
     }
+
+    public static function getFixerByName(string $name): FixerInterface
+    {
+        static $fixers = null;
+
+        if (null === $fixers) {
+            $factory = new FixerFactory();
+            $factory->registerBuiltInFixers();
+
+            $fixers = [];
+            foreach ($factory->getFixers() as $fixer) {
+                $fixers[$fixer->getName()] = $fixer;
+            }
+        }
+
+        if (!\array_key_exists($name, $fixers)) {
+            throw new \InvalidArgumentException(sprintf('Fixer "%s" does not exist.', $name));
+        }
+
+        return $fixers[$name];
+    }
 }