feature #5280 NoUselessSprintfFixer - Introduction (SpacePossum)

This PR was merged into the 2.17-dev branch.

Discussion
----------

NoUselessSprintfFixer - Introduction

Commits
-------

ecd1525 NoUselessSprintfFixer - Introduction

doc/ruleSets/SymfonyRisky.rst

@@ -35,6 +35,7 @@ Rules
 - `no_php4_constructor <./../rules/class_notation/no_php4_constructor.rst>`_
 - `no_trailing_whitespace_in_string <./../rules/string_notation/no_trailing_whitespace_in_string.rst>`_
 - `no_unneeded_final_method <./../rules/class_notation/no_unneeded_final_method.rst>`_
+- `no_useless_sprintf <./../rules/function_notation/no_useless_sprintf.rst>`_
 - `non_printable_character <./../rules/basic/non_printable_character.rst>`_
 - `php_unit_construct <./../rules/php_unit/php_unit_construct.rst>`_
 - `php_unit_mock_short_will_return <./../rules/php_unit/php_unit_mock_short_will_return.rst>`_

doc/rules/function_notation/no_useless_sprintf.rst

@@ -0,0 +1,35 @@
+===========================
+Rule ``no_useless_sprintf``
+===========================
+
+There must be no ``sprintf`` calls with only the first argument.
+
+.. warning:: Using this rule is risky.
+
+   Risky when if the ``sprintf`` function is overridden.
+
+Examples
+--------
+
+Example #1
+~~~~~~~~~~
+
+.. code-block:: diff
+
+   --- Original
+   +++ New
+   @@ -1,2 +1,2 @@
+    <?php
+   -$foo = sprintf('bar');
+   +$foo = 'bar';
+
+Rule sets
+---------
+
+The rule is part of the following rule sets:
+
+@PhpCsFixer:risky
+  Using the `@PhpCsFixer:risky <./../../ruleSets/PhpCsFixerRisky.rst>`_ rule set will enable the ``no_useless_sprintf`` rule.
+
+@Symfony:risky
+  Using the `@Symfony:risky <./../../ruleSets/SymfonyRisky.rst>`_ rule set will enable the ``no_useless_sprintf`` rule.

doc/rules/index.rst

@@ -241,6 +241,8 @@ Function Notation
     When making a method or function call, there MUST NOT be a space between the method or function name and the opening parenthesis.
 - `no_unreachable_default_argument_value <./function_notation/no_unreachable_default_argument_value.rst>`_ *(risky)*
     In function arguments there must not be arguments with default values before non-default ones.
+- `no_useless_sprintf <./function_notation/no_useless_sprintf.rst>`_ *(risky)*
+    There must be no ``sprintf`` calls with only the first argument.
 - `nullable_type_declaration_for_default_null_value <./function_notation/nullable_type_declaration_for_default_null_value.rst>`_
     Adds or removes ``?`` before type declarations for parameters with a default ``null`` value.
 - `phpdoc_to_param_type <./function_notation/phpdoc_to_param_type.rst>`_ *(risky)*

src/Fixer/Casing/NativeFunctionCasingFixer.php

@@ -38,7 +38,7 @@ public function getDefinition()
     /**
      * {@inheritdoc}
      *
-     * Must run after FunctionToConstantFixer, PowToExponentiationFixer.
+     * Must run after FunctionToConstantFixer, NoUselessSprintfFixer, PowToExponentiationFixer.
      */
     public function getPriority()
     {

src/Fixer/Comment/CommentToPhpdocFixer.php

@@ -99,7 +99,7 @@ static function ($tag) {
     protected function createConfigurationDefinition()
     {
         return new FixerConfigurationResolver([
-            (new FixerOptionBuilder('ignored_tags', sprintf('List of ignored tags')))
+            (new FixerOptionBuilder('ignored_tags', 'List of ignored tags'))
                 ->setAllowedTypes(['array'])
                 ->setDefault([])
                 ->getOption(),

src/Fixer/FunctionNotation/MethodArgumentSpaceFixer.php

@@ -129,7 +129,7 @@ public function configure(array $configuration = null)
      * {@inheritdoc}
      *
      * Must run before ArrayIndentationFixer.
-     * Must run after BracesFixer, CombineNestedDirnameFixer, ImplodeCallFixer, MethodChainingIndentationFixer, PowToExponentiationFixer.
+     * Must run after BracesFixer, CombineNestedDirnameFixer, ImplodeCallFixer, MethodChainingIndentationFixer, NoUselessSprintfFixer, PowToExponentiationFixer.
      */
     public function getPriority()
     {

src/Fixer/FunctionNotation/NoUselessSprintfFixer.php

@@ -0,0 +1,118 @@
+<?php
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *     Dariusz Rumiński <[email protected]>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Fixer\FunctionNotation;
+
+use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\FixerDefinition\CodeSample;
+use PhpCsFixer\FixerDefinition\FixerDefinition;
+use PhpCsFixer\Tokenizer\Analyzer\ArgumentsAnalyzer;
+use PhpCsFixer\Tokenizer\Analyzer\FunctionsAnalyzer;
+use PhpCsFixer\Tokenizer\Tokens;
+
+final class NoUselessSprintfFixer extends AbstractFixer
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getDefinition()
+    {
+        return new FixerDefinition(
+            'There must be no `sprintf` calls with only the first argument.',
+            [
+                new CodeSample(
+                    "<?php\n\$foo = sprintf('bar');\n"
+                ),
+            ],
+            null,
+            'Risky when if the `sprintf` function is overridden.'
+        );
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isCandidate(Tokens $tokens)
+    {
+        return $tokens->isTokenKindFound(T_STRING);
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function isRisky()
+    {
+        return true;
+    }
+
+    /**
+     * {@inheritdoc}
+     *
+     * Must run before MethodArgumentSpaceFixer, NativeFunctionCasingFixer, NoExtraBlankLinesFixer, NoSpacesInsideParenthesisFixer.
+     */
+    public function getPriority()
+    {
+        return 3;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function applyFix(\SplFileInfo $file, Tokens $tokens)
+    {
+        $functionAnalyzer = new FunctionsAnalyzer();
+        $argumentsAnalyzer = new ArgumentsAnalyzer();
+
+        for ($index = \count($tokens) - 1; $index > 0; --$index) {
+            if (!$tokens[$index]->isGivenKind(T_STRING)) {
+                continue;
+            }
+
+            if ('sprintf' !== strtolower($tokens[$index]->getContent())) {
+                continue;
+            }
+
+            if (!$functionAnalyzer->isGlobalFunctionCall($tokens, $index)) {
+                continue;
+            }
+
+            $openParenthesisIndex = $tokens->getNextTokenOfKind($index, ['(']);
+
+            if ($tokens[$tokens->getNextMeaningfulToken($openParenthesisIndex)]->isGivenKind(T_ELLIPSIS)) {
+                continue;
+            }
+
+            $closeParenthesisIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $openParenthesisIndex);
+
+            if (1 !== $argumentsAnalyzer->countArguments($tokens, $openParenthesisIndex, $closeParenthesisIndex)) {
+                continue;
+            }
+
+            $tokens->clearTokenAndMergeSurroundingWhitespace($closeParenthesisIndex);
+
+            $prevMeaningfulTokenIndex = $tokens->getPrevMeaningfulToken($closeParenthesisIndex);
+
+            if ($tokens[$prevMeaningfulTokenIndex]->equals(',')) {
+                $tokens->clearTokenAndMergeSurroundingWhitespace($prevMeaningfulTokenIndex);
+            }
+
+            $tokens->clearTokenAndMergeSurroundingWhitespace($openParenthesisIndex);
+            $tokens->clearTokenAndMergeSurroundingWhitespace($index);
+
+            $prevMeaningfulTokenIndex = $tokens->getPrevMeaningfulToken($index);
+
+            if ($tokens[$prevMeaningfulTokenIndex]->isGivenKind(T_NS_SEPARATOR)) {
+                $tokens->clearTokenAndMergeSurroundingWhitespace($prevMeaningfulTokenIndex);
+            }
+        }
+    }
+}

src/Fixer/Whitespace/NoExtraBlankLinesFixer.php

@@ -278,7 +278,7 @@ class Foo
      * {@inheritdoc}
      *
      * Must run before BlankLineBeforeStatementFixer.
-     * Must run after CombineConsecutiveUnsetsFixer, FunctionToConstantFixer, NoEmptyCommentFixer, NoEmptyPhpdocFixer, NoEmptyStatementFixer, NoUnusedImportsFixer, NoUselessElseFixer, NoUselessReturnFixer.
+     * Must run after CombineConsecutiveUnsetsFixer, FunctionToConstantFixer, NoEmptyCommentFixer, NoEmptyPhpdocFixer, NoEmptyStatementFixer, NoUnusedImportsFixer, NoUselessElseFixer, NoUselessReturnFixer, NoUselessSprintfFixer.
      */
     public function getPriority()
     {

src/Fixer/Whitespace/NoSpacesInsideParenthesisFixer.php

@@ -48,7 +48,7 @@ function foo( \$bar, \$baz )
      * {@inheritdoc}
      *
      * Must run before FunctionToConstantFixer.
-     * Must run after CombineConsecutiveIssetsFixer, CombineNestedDirnameFixer, LambdaNotUsedImportFixer, PowToExponentiationFixer.
+     * Must run after CombineConsecutiveIssetsFixer, CombineNestedDirnameFixer, LambdaNotUsedImportFixer, NoUselessSprintfFixer, PowToExponentiationFixer.
      */
     public function getPriority()
     {

src/RuleSet/Sets/SymfonyRiskySet.php

@@ -69,6 +69,7 @@ public function getRules()
             'no_php4_constructor' => true,
             'no_trailing_whitespace_in_string' => true,
             'no_unneeded_final_method' => true,
+            'no_useless_sprintf' => true,
             'non_printable_character' => true,
             'php_unit_construct' => true,
             'php_unit_mock_short_will_return' => true,

tests/AutoReview/FixerFactoryTest.php

@@ -192,6 +192,10 @@ public function provideFixersPriorityCases()
             [$fixers['no_useless_return'], $fixers['blank_line_before_statement']],
             [$fixers['no_useless_return'], $fixers['no_extra_blank_lines']],
             [$fixers['no_useless_return'], $fixers['no_whitespace_in_blank_line']],
+            [$fixers['no_useless_sprintf'], $fixers['method_argument_space']],
+            [$fixers['no_useless_sprintf'], $fixers['native_function_casing']],
+            [$fixers['no_useless_sprintf'], $fixers['no_extra_blank_lines']],
+            [$fixers['no_useless_sprintf'], $fixers['no_spaces_inside_parenthesis']],
             [$fixers['nullable_type_declaration_for_default_null_value'], $fixers['no_unreachable_default_argument_value']],
             [$fixers['ordered_class_elements'], $fixers['class_attributes_separation']],
             [$fixers['ordered_class_elements'], $fixers['method_separation']],
@@ -395,6 +399,7 @@ function (array $case) {
                         'pow_to_exponentiation,native_function_casing.test',
                         'pow_to_exponentiation,no_spaces_after_function_name.test',
                         'pow_to_exponentiation,no_spaces_inside_parenthesis.test',
+                        'no_useless_sprintf,native_function_casing.test',
                     ],
                     true
                 );

tests/Fixer/FunctionNotation/NoUselessSprintfFixerTest.php

@@ -0,0 +1,119 @@
+<?php
+
+/*
+ * This file is part of PHP CS Fixer.
+ *
+ * (c) Fabien Potencier <[email protected]>
+ *     Dariusz Rumiński <[email protected]>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace PhpCsFixer\Tests\Fixer\FunctionNotation;
+
+use PhpCsFixer\Tests\Test\AbstractFixerTestCase;
+
+/**
+ * @internal
+ *
+ * @covers \PhpCsFixer\Fixer\FunctionNotation\NoUselessSprintfFixer
+ */
+final class NoUselessSprintfFixerTest extends AbstractFixerTestCase
+{
+    /**
+     * @dataProvider provideFixCases
+     *
+     * @param string      $expected
+     * @param null|string $input
+     */
+    public function testFix($expected, $input = null)
+    {
+        $this->doTest($expected, $input);
+    }
+
+    public function provideFixCases()
+    {
+        yield 'simple' => [
+            '<?php echo "bar";',
+            '<?php echo sprintf("bar");',
+        ];
+
+        yield 'simple II' => [
+            "<?php echo 'bar' ?>",
+            "<?php echo sprintf('bar') ?>",
+        ];
+
+        yield 'simple III' => [
+            '<?php echo $bar;',
+            '<?php echo sprintf($bar);',
+        ];
+
+        yield 'simple IV' => [
+            '<?php echo 1;',
+            '<?php echo sprintf(1);',
+        ];
+
+        yield 'minimal' => [
+            '<?php $foo;',
+            '<?php sprintf($foo);',
+        ];
+
+        yield 'case insensitive' => [
+            '<?php echo "bar";',
+            '<?php echo SPRINTF("bar");',
+        ];
+
+        yield 'nested' => [
+            '<?php echo /* test */"bar";',
+            '<?php echo sprintf(sprintf(sprintf(/* test */sprintf(sprintf(sprintf("bar"))))));',
+        ];
+
+        yield [
+            '<?php namespace Foo {
+                function sprintf($foo) {
+                    echo $foo;
+                }
+            }',
+        ];
+
+        yield [
+            '<?php namespace Foo;
+                function sprintf($foo) {
+                    echo $foo;
+                }
+            ',
+        ];
+
+        yield [
+            '<?php
+                echo \Foo\sprintf("abc");
+                echo $bar->sprintf($foo);
+                echo Bar::sprintf($foo);
+                echo sprint(...$foo);
+                echo sprint("%d", 1);
+                echo sprint("%d%d%d", 1, 2, 3);
+                echo sprint();
+                echo sprint[2]("foo");
+            ',
+        ];
+
+        if (\PHP_VERSION_ID > 70300) {
+            yield 'trailing comma' => [
+                '<?php echo "bar";',
+                '<?php echo sprintf("bar",);',
+            ];
+        }
+
+        if (\PHP_VERSION_ID < 80000) {
+            yield [
+                '<?php echo  "bar";',
+                '<?php echo \ sprintf("bar");',
+            ];
+
+            yield [
+                '<?php echo A /* 1 */ \ B \ sprintf("bar");',
+            ];
+        }
+    }
+}

tests/Fixtures/Integration/priority/no_useless_sprintf,method_argument_space.test

@@ -0,0 +1,11 @@
+--TEST--
+Integration of fixers: no_useless_sprintf,method_argument_space.
+--RULESET--
+{"no_useless_sprintf": true, "method_argument_space": true}
+--REQUIREMENTS--
+{"php": 70300}
+--EXPECT--
+<?php $a =   $a  ;
+
+--INPUT--
+<?php $a = sprintf(  $a  ,);