minor #3768 Improve deprecation messages (julienfalque, SpacePossum)

This PR was squashed before being merged into the 2.11 branch (closes #3768). Discussion ---------- Improve deprecation messages Extracted from #3699. Commits ------- 515b34b Improve deprecation messages
PHP-CS-Fixer · May 21, 2018 · ffb3953 · ffb3953
2 parents 3a12537 + 515b34b
commit ffb3953
Show file tree

Hide file tree

Showing 34 changed files with 162 additions and 122 deletions.
diff --git a/src/AbstractFixer.php b/src/AbstractFixer.php
@@ -118,14 +118,13 @@ public function configure(array $configuration = null)
         }
 
         if (null === $configuration) {
+            $message = 'Passing NULL to set default configuration is deprecated and will not be supported in 3.0, use an empty array instead.';
+
             if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
-                throw new \InvalidArgumentException('Parameter must not be `null`. This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.');
+                throw new \InvalidArgumentException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
             }
 
-            @trigger_error(
-                'Passing NULL to set default configuration is deprecated and will not be supported in 3.0, use an empty array instead.',
-                E_USER_DEPRECATED
-            );
+            @trigger_error($message, E_USER_DEPRECATED);
 
             $configuration = [];
         }
@@ -137,12 +136,19 @@ public function configure(array $configuration = null)
 
             $name = $option->getName();
             if (array_key_exists($name, $configuration)) {
-                @trigger_error(sprintf(
-                    'Option "%s" is deprecated and will be removed in %d.0. %s',
+                $message = sprintf(
+                    'Option "%s" for rule "%s" is deprecated and will be removed in version %d.0. %s',
                     $name,
-                    Application::VERSION + 1,
+                    $this->getName(),
+                    (int) Application::VERSION + 1,
                     str_replace('`', '"', $option->getDeprecationMessage())
-                ), E_USER_DEPRECATED);
+                );
+
+                if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
+                    throw new \InvalidArgumentException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
+                }
+
+                @trigger_error($message, E_USER_DEPRECATED);
             }
         }
 

diff --git a/src/Console/ConfigurationResolver.php b/src/Console/ConfigurationResolver.php
@@ -345,25 +345,6 @@ static function (FixerInterface $fixer) {
                     throw new InvalidConfigurationException(sprintf('The rules contain risky fixers (%s), but they are not allowed to run. Perhaps you forget to use --allow-risky option?', implode(', ', $riskyFixers)));
                 }
             }
-
-            foreach ($this->fixers as $fixer) {
-                if ($fixer instanceof DeprecatedFixerInterface) {
-                    $successors = $fixer->getSuccessorsNames();
-                    $message = sprintf(
-                        'Fixer `%s` is deprecated%s',
-                        $fixer->getName(),
-                        [] === $successors
-                            ? ' and will be removed on next major version.'
-                            : sprintf(', use %s instead.', Utils::naturalLanguageJoinWithBackticks($successors))
-                    );
-
-                    if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
-                        throw new \RuntimeException($message.' This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.');
-                    }
-
-                    @trigger_error($message, E_USER_DEPRECATED);
-                }
-            }
         }
 
         return $this->fixers;
@@ -445,14 +426,13 @@ public function getProgress()
                         implode('", "', $progressTypes)
                     ));
                 } elseif (in_array($progressType, ['estimating', 'estimating-max', 'run-in'], true)) {
+                    $message = 'Passing `estimating`, `estimating-max` or `run-in` is deprecated and will not be supported in 3.0, use `none` or `dots` instead.';
+
                     if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
-                        throw new \InvalidArgumentException('Passing `estimating`, `estimating-max` or `run-in` is deprecated and will not be supported in 3.0, use `none` or `dots` instead. This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.');
+                        throw new \InvalidArgumentException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
                     }
 
-                    @trigger_error(
-                        'Passing `estimating`, `estimating-max` or `run-in` is deprecated and will not be supported in 3.0, use `none` or `dots` instead.',
-                        E_USER_DEPRECATED
-                    );
+                    @trigger_error($message, E_USER_DEPRECATED);
                 }
 
                 $this->progress = $progressType;
@@ -749,10 +729,12 @@ private function validateRules(array $rules)
         /** @var string[] $configuredFixers */
         $configuredFixers = array_keys($ruleSet->getRules());
 
+        $fixers = $this->createFixerFactory()->getFixers();
+
         /** @var string[] $availableFixers */
         $availableFixers = array_map(static function (FixerInterface $fixer) {
             return $fixer->getName();
-        }, $this->createFixerFactory()->getFixers());
+        }, $fixers);
 
         $unknownFixers = array_diff(
             $configuredFixers,
@@ -774,6 +756,24 @@ private function validateRules(array $rules)
 
             throw new InvalidConfigurationException(substr($message, 0, -2).'.');
         }
+
+        foreach ($fixers as $fixer) {
+            $fixerName = $fixer->getName();
+            if (isset($rules[$fixerName]) && $fixer instanceof DeprecatedFixerInterface) {
+                $successors = $fixer->getSuccessorsNames();
+                $messageEnd = [] === $successors
+                    ? sprintf(' and will be removed in version %d.0.', (int) Application::VERSION + 1)
+                    : sprintf('. Use %s instead.', str_replace('`', '"', Utils::naturalLanguageJoinWithBackticks($successors)));
+
+                $message = "Rule \"{$fixerName}\" is deprecated{$messageEnd}";
+
+                if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
+                    throw new \RuntimeException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
+                }
+
+                @trigger_error($message, E_USER_DEPRECATED);
+            }
+        }
     }
 
     /**
@@ -917,14 +917,13 @@ private function resolveOptionBooleanValue($optionName)
             return false;
         }
 
+        $message = sprintf('Expected "yes" or "no" for option "%s", other values are deprecated and support will be removed in 3.0. Got "%s", this implicitly set the option to "false".', $optionName, $value);
+
         if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
-            throw new InvalidConfigurationException(sprintf('Expected "yes" or "no" for option "%s", got "%s". This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.', $optionName, $value));
+            throw new InvalidConfigurationException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
         }
 
-        @trigger_error(
-            sprintf('Expected "yes" or "no" for option "%s", other values are deprecated and support will be removed in 3.0. Got "%s", this implicitly set the option to "false".', $optionName, $value),
-            E_USER_DEPRECATED
-        );
+        @trigger_error($message, E_USER_DEPRECATED);
 
         return false;
     }

diff --git a/src/Fixer/Alias/RandomApiMigrationFixer.php b/src/Fixer/Alias/RandomApiMigrationFixer.php
@@ -162,6 +162,6 @@ protected function createConfigurationDefinition()
                     'srand' => 'mt_srand',
                 ])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/Fixer/ClassNotation/OrderedClassElementsFixer.php b/src/Fixer/ClassNotation/OrderedClassElementsFixer.php
@@ -276,7 +276,7 @@ protected function createConfigurationDefinition()
                 ->setAllowedValues($this->supportedSortAlgorithms)
                 ->setDefault(self::SORT_NONE)
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/ClassNotation/SingleClassElementPerStatementFixer.php b/src/Fixer/ClassNotation/SingleClassElementPerStatementFixer.php
@@ -104,7 +104,7 @@ protected function createConfigurationDefinition()
                 ->setAllowedTypes(['array'])
                 ->setAllowedValues([new AllowedValueSubset($values)])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/ClassNotation/VisibilityRequiredFixer.php b/src/Fixer/ClassNotation/VisibilityRequiredFixer.php
@@ -119,7 +119,7 @@ protected function createConfigurationDefinition()
                 })
                 ->setDefault(['property', 'method'])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/ControlStructure/NoUnneededControlParenthesesFixer.php b/src/Fixer/ControlStructure/NoUnneededControlParenthesesFixer.php
@@ -184,6 +184,6 @@ protected function createConfigurationDefinition()
                     'yield',
                 ])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/Fixer/Operator/BinaryOperatorSpacesFixer.php b/src/Fixer/Operator/BinaryOperatorSpacesFixer.php
@@ -474,11 +474,17 @@ private function resolveOldConfig(array $configuration)
             }
         }
 
-        @trigger_error(sprintf(
+        $message = sprintf(
             'Given configuration is deprecated and will be removed in 3.0. Use configuration %s as replacement for %s.',
             HelpCommand::toString($newConfig),
             HelpCommand::toString($configuration)
-        ), E_USER_DEPRECATED);
+        );
+
+        if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
+            throw new InvalidFixerConfigurationException($this->getName(), "{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
+        }
+
+        @trigger_error($message, E_USER_DEPRECATED);
 
         return $newConfig;
     }

diff --git a/src/Fixer/PhpUnit/PhpUnitConstructFixer.php b/src/Fixer/PhpUnit/PhpUnitConstructFixer.php
@@ -129,7 +129,7 @@ protected function createConfigurationDefinition()
                     'assertNotSame',
                 ])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/PhpUnit/PhpUnitDedicateAssertFixer.php b/src/Fixer/PhpUnit/PhpUnitDedicateAssertFixer.php
@@ -253,7 +253,7 @@ protected function createConfigurationDefinition()
                 ])
                 ->setDefault(PhpUnitTargetVersion::VERSION_5_0) // @TODO 3.x: change to `VERSION_NEWEST`
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/PhpUnit/PhpUnitStrictFixer.php b/src/Fixer/PhpUnit/PhpUnitStrictFixer.php
@@ -153,6 +153,6 @@ protected function createConfigurationDefinition()
                     'assertNotEquals',
                 ])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/Fixer/Phpdoc/GeneralPhpdocAnnotationRemoveFixer.php b/src/Fixer/Phpdoc/GeneralPhpdocAnnotationRemoveFixer.php
@@ -112,6 +112,6 @@ protected function createConfigurationDefinition()
                 ->setAllowedTypes(['array'])
                 ->setDefault([])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/Fixer/Phpdoc/PhpdocNoAliasTagFixer.php b/src/Fixer/Phpdoc/PhpdocNoAliasTagFixer.php
@@ -162,6 +162,6 @@ protected function createConfigurationDefinition()
                     'link' => 'see',
                 ])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/Fixer/Phpdoc/PhpdocReturnSelfReferenceFixer.php b/src/Fixer/Phpdoc/PhpdocReturnSelfReferenceFixer.php
@@ -161,7 +161,7 @@ protected function createConfigurationDefinition()
                 })
                 ->setDefault($default)
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     /**

diff --git a/src/Fixer/Whitespace/NoExtraBlankLinesFixer.php b/src/Fixer/Whitespace/NoExtraBlankLinesFixer.php
@@ -13,6 +13,7 @@
 namespace PhpCsFixer\Fixer\Whitespace;
 
 use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\ConfigurationException\InvalidConfigurationException;
 use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
 use PhpCsFixer\Fixer\WhitespacesAwareFixerInterface;
 use PhpCsFixer\FixerConfiguration\AllowedValueSubset;
@@ -307,14 +308,22 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
      */
     protected function createConfigurationDefinition()
     {
+        $that = $this;
+
         return new FixerConfigurationResolverRootless('tokens', [
             (new FixerOptionBuilder('tokens', 'List of tokens to fix.'))
                 ->setAllowedTypes(['array'])
                 ->setAllowedValues([new AllowedValueSubset(self::$availableTokens)])
-                ->setNormalizer(static function (Options $options, $tokens) {
+                ->setNormalizer(static function (Options $options, $tokens) use ($that) {
                     foreach ($tokens as &$token) {
                         if ('useTrait' === $token) {
-                            @trigger_error('Token "useTrait" is deprecated and will be removed in 3.0, use "use_trait" instead.', E_USER_DEPRECATED);
+                            $message = "Token \"useTrait\" in option \"tokens\" for rule \"{$that->getName()}\" is deprecated and will be removed in 3.0, use \"use_trait\" instead.";
+
+                            if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
+                                throw new InvalidConfigurationException("{$message} This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
+                            }
+
+                            @trigger_error($message, E_USER_DEPRECATED);
                             $token = 'use_trait';
 
                             break;
@@ -325,7 +334,7 @@ protected function createConfigurationDefinition()
                 })
                 ->setDefault(['extra'])
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 
     private function fixByToken(Token $token, $index)

diff --git a/src/Fixer/Whitespace/NoSpacesAroundOffsetFixer.php b/src/Fixer/Whitespace/NoSpacesAroundOffsetFixer.php
@@ -102,6 +102,6 @@ protected function createConfigurationDefinition()
                 ->setAllowedValues([new AllowedValueSubset($values)])
                 ->setDefault($values)
                 ->getOption(),
-        ]);
+        ], $this->getName());
     }
 }
diff --git a/src/FixerConfiguration/FixerConfigurationResolverRootless.php b/src/FixerConfiguration/FixerConfigurationResolverRootless.php
@@ -29,13 +29,20 @@ final class FixerConfigurationResolverRootless implements FixerConfigurationReso
      */
     private $root;
 
+    /**
+     * @var string
+     */
+    private $fixerName;
+
     /**
      * @param string                         $root
      * @param iterable<FixerOptionInterface> $options
+     * @param string                         $fixerName
      */
-    public function __construct($root, $options)
+    public function __construct($root, $options, $fixerName)
     {
         $this->resolver = new FixerConfigurationResolver($options);
+        $this->fixerName = $fixerName;
 
         $names = array_map(
             static function (FixerOptionInterface $option) {
@@ -75,17 +82,13 @@ function (FixerOptionInterface $option) {
             $passedNames = array_keys($options);
 
             if (!empty(array_diff($passedNames, $names))) {
+                $message = "Passing \"{$this->root}\" at the root of the configuration for rule \"{$this->fixerName}\" is deprecated and will not be supported in 3.0, use \"{$this->root}\" => array(...) option instead.";
+
                 if (getenv('PHP_CS_FIXER_FUTURE_MODE')) {
-                    throw new \RuntimeException(sprintf(
-                        'Passing "%1$s" at the root of the configuration is deprecated and will not be supported in 3.0, use "%1$s" => array(...) option instead. This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.',
-                        $this->root
-                    ));
+                    throw new \RuntimeException("{$message}. This check was performed as `PHP_CS_FIXER_FUTURE_MODE` env var is set.");
                 }
 
-                @trigger_error(sprintf(
-                    'Passing "%1$s" at the root of the configuration is deprecated and will not be supported in 3.0, use "%1$s" => array(...) option instead.',
-                    $this->root
-                ), E_USER_DEPRECATED);
+                @trigger_error($message, E_USER_DEPRECATED);
 
                 $options = [$this->root => $options];
             }

diff --git a/tests/Console/ConfigurationResolverTest.php b/tests/Console/ConfigurationResolverTest.php
@@ -1111,23 +1111,33 @@ public function testWithEmptyRules()
     }
 
     /**
+     * @param array|bool $ruleConfig
+     *
+     * @dataProvider provideDeprecatedFixerConfiguredCases
+     *
      * @group legacy
-     * @expectedDeprecation Fixer `Vendor4/foo` is deprecated, use `testA` and `testB` instead.
+     * @expectedDeprecation Rule "Vendor4/foo" is deprecated. Use "testA" and "testB" instead.
      */
-    public function testDeprecatedFixerConfigured()
+    public function testDeprecatedFixerConfigured($ruleConfig)
     {
         $fixer = new DeprecatedFixer();
         $config = new Config();
         $config->registerCustomFixers([$fixer]);
+        $config->setRules([$fixer->getName() => $ruleConfig]);
 
-        $resolver = $this->createConfigurationResolver(
-            ['rules' => $fixer->getName()],
-            $config
-        );
-
+        $resolver = $this->createConfigurationResolver([], $config);
         $resolver->getFixers();
     }
 
+    public function provideDeprecatedFixerConfiguredCases()
+    {
+        return [
+            [true],
+            [['foo' => true]],
+            [false],
+        ];
+    }
+
     private function assertSameRules(array $expected, array $actual, $message = '')
     {
         ksort($expected);