Browse Source

NoUnneededCurlyBracesFixer - handle namespaces

SpacePossum 5 years ago
parent
commit
6e35307754

+ 5 - 0
README.rst

@@ -1152,6 +1152,11 @@ Choose from the list of available rules:
   Removes unneeded curly braces that are superfluous and aren't part of a
   control structure's body.
 
+  Configuration options:
+
+  - ``namespaces`` (``bool``): remove unneeded curly braces from bracketed
+    namespaces; defaults to ``false``
+
 * **no_unneeded_final_method** [@Symfony, @PhpCsFixer]
 
   A ``final`` class must not have ``final`` methods and ``private`` method must

+ 1 - 1
src/Fixer/Comment/HeaderCommentFixer.php

@@ -107,7 +107,7 @@ echo 1;
      */
     public function isCandidate(Tokens $tokens)
     {
-        return $tokens[0]->isGivenKind(T_OPEN_TAG) && $tokens->isMonolithicPhp();
+        return isset($tokens[0]) && $tokens[0]->isGivenKind(T_OPEN_TAG) && $tokens->isMonolithicPhp();
     }
 
     /**

+ 69 - 1
src/Fixer/ControlStructure/NoUnneededCurlyBracesFixer.php

@@ -13,14 +13,18 @@
 namespace PhpCsFixer\Fixer\ControlStructure;
 
 use PhpCsFixer\AbstractFixer;
+use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
+use PhpCsFixer\FixerConfiguration\FixerConfigurationResolver;
+use PhpCsFixer\FixerConfiguration\FixerOptionBuilder;
 use PhpCsFixer\FixerDefinition\CodeSample;
 use PhpCsFixer\FixerDefinition\FixerDefinition;
+use PhpCsFixer\Tokenizer\Token;
 use PhpCsFixer\Tokenizer\Tokens;
 
 /**
  * @author SpacePossum
  */
-final class NoUnneededCurlyBracesFixer extends AbstractFixer
+final class NoUnneededCurlyBracesFixer extends AbstractFixer implements ConfigurationDefinitionFixerInterface
 {
     /**
      * {@inheritdoc}
@@ -42,6 +46,14 @@ switch ($b) {
 }
 '
                 ),
+                new CodeSample(
+                    '<?php
+namespace Foo {
+    function Bar(){}
+}
+',
+                    ['namespaces' => true]
+                ),
             ]
         );
     }
@@ -74,6 +86,23 @@ switch ($b) {
                 $this->clearOverCompleteBraces($tokens, $index, $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $index));
             }
         }
+
+        if ($this->configuration['namespaces']) {
+            $this->clearIfIsOverCompleteNamespaceBlock($tokens);
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function createConfigurationDefinition()
+    {
+        return new FixerConfigurationResolver([
+            (new FixerOptionBuilder('namespaces', 'Remove unneeded curly braces from bracketed namespaces.'))
+                ->setAllowedTypes(['bool'])
+                ->setDefault(false)
+                ->getOption(),
+        ]);
     }
 
     /**
@@ -106,4 +135,43 @@ switch ($b) {
 
         return $tokens[$tokens->getPrevMeaningfulToken($index)]->equalsAny($whiteList);
     }
+
+    private function clearIfIsOverCompleteNamespaceBlock(Tokens $tokens)
+    {
+        if (Tokens::isLegacyMode()) {
+            $index = $tokens->getNextTokenOfKind(0, [[T_NAMESPACE]]);
+            $secondNamespaceIndex = $tokens->getNextTokenOfKind($index, [[T_NAMESPACE]]);
+
+            if (null !== $secondNamespaceIndex) {
+                return;
+            }
+        } elseif (1 !== $tokens->countTokenKind(T_NAMESPACE)) {
+            return; // fast check, we never fix if multiple namespaces are defined
+        }
+
+        $index = $tokens->getNextTokenOfKind(0, [[T_NAMESPACE]]);
+
+        do {
+            $index = $tokens->getNextMeaningfulToken($index);
+        } while ($tokens[$index]->isGivenKind([T_STRING, T_NS_SEPARATOR]));
+
+        if (!$tokens[$index]->equals('{')) {
+            return; // `;`
+        }
+
+        $closeIndex = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_CURLY_BRACE, $index);
+        $afterCloseIndex = $tokens->getNextMeaningfulToken($closeIndex);
+
+        if (null !== $afterCloseIndex && (!$tokens[$afterCloseIndex]->isGivenKind(T_CLOSE_TAG) || null !== $tokens->getNextMeaningfulToken($afterCloseIndex))) {
+            return;
+        }
+
+        // clear up
+        $tokens->clearTokenAndMergeSurroundingWhitespace($closeIndex);
+        $tokens[$index] = new Token(';');
+
+        if ($tokens[$index - 1]->isWhitespace(" \t") && !$tokens[$index - 2]->isComment()) {
+            $tokens->clearTokenAndMergeSurroundingWhitespace($index - 1);
+        }
+    }
 }

+ 1 - 1
src/Fixer/Strict/DeclareStrictTypesFixer.php

@@ -59,7 +59,7 @@ final class DeclareStrictTypesFixer extends AbstractFixer implements Whitespaces
      */
     public function isCandidate(Tokens $tokens)
     {
-        return \PHP_VERSION_ID >= 70000 && $tokens[0]->isGivenKind(T_OPEN_TAG);
+        return \PHP_VERSION_ID >= 70000 && isset($tokens[0]) && $tokens[0]->isGivenKind(T_OPEN_TAG);
     }
 
     /**

+ 1 - 3
src/RuleSet.php

@@ -107,7 +107,7 @@ final class RuleSet implements RuleSetInterface
             'no_trailing_comma_in_list_call' => true,
             'no_trailing_comma_in_singleline_array' => true,
             'no_unneeded_control_parentheses' => true,
-            'no_unneeded_curly_braces' => true,
+            'no_unneeded_curly_braces' => ['namespaces' => true],
             'no_unneeded_final_method' => true,
             'no_unused_imports' => true,
             'no_whitespace_before_comma_in_array' => true,
@@ -246,8 +246,6 @@ final class RuleSet implements RuleSetInterface
             'no_null_property_initialization' => true,
             'no_short_echo_tag' => true,
             'no_superfluous_elseif' => true,
-            'no_unneeded_curly_braces' => true,
-            'no_unneeded_final_method' => true,
             'no_unset_cast' => true,
             'no_useless_else' => true,
             'no_useless_return' => true,

+ 19 - 1
tests/AutoReview/FixerTest.php

@@ -12,6 +12,7 @@
 
 namespace PhpCsFixer\Tests\AutoReview;
 
+use PhpCsFixer\Fixer\Comment\HeaderCommentFixer;
 use PhpCsFixer\Fixer\ConfigurationDefinitionFixerInterface;
 use PhpCsFixer\Fixer\DefinedFixerInterface;
 use PhpCsFixer\Fixer\DeprecatedFixerInterface;
@@ -267,10 +268,27 @@ final class FixerTest extends TestCase
      */
     public function testFixersReturnTypes(FixerInterface $fixer)
     {
+        $tokens = Tokens::fromCode('<?php ');
+        $emptyTokens = new Tokens();
+
         static::assertInternalType('int', $fixer->getPriority(), sprintf('Return type for ::getPriority of "%s" is invalid.', $fixer->getName()));
-        static::assertInternalType('bool', $fixer->isCandidate(Tokens::fromCode('<?php ')), sprintf('Return type for ::isCandidate 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()));
+        static::assertFalse($emptyTokens->isChanged());
+
+        static::assertInternalType('bool', $fixer->isCandidate($tokens), sprintf('Return type for ::isCandidate of "%s" is invalid.', $fixer->getName()));
+        static::assertFalse($tokens->isChanged());
+
+        if ($fixer instanceof HeaderCommentFixer) {
+            $fixer->configure(['header' => 'a']);
+        }
+
+        static::assertNull($fixer->fix(new \SplFileInfo(__FILE__), $emptyTokens), sprintf('Return type for ::fix with empty tokens of "%s" is invalid.', $fixer->getName()));
+        static::assertFalse($emptyTokens->isChanged());
+
+        static::assertNull($fixer->fix(new \SplFileInfo(__FILE__), $tokens), sprintf('Return type for ::fix of "%s" is invalid.', $fixer->getName()));
     }
 
     private function getAllFixers()

+ 60 - 0
tests/Fixer/ControlStructure/NoUnneededCurlyBracesFixerTest.php

@@ -149,4 +149,64 @@ final class NoUnneededCurlyBracesFixerTest extends AbstractFixerTestCase
             ],
         ];
     }
+
+    /**
+     * @param string      $expected
+     * @param null|string $input
+     *
+     * @dataProvider provideFixNamespaceCases
+     */
+    public function testFixNamespace($expected, $input = null)
+    {
+        $this->fixer->configure(['namespaces' => true]);
+        $this->doTest($expected, $input);
+    }
+
+    public function provideFixNamespaceCases()
+    {
+        yield [
+            '<?php
+namespace Foo;
+    function Bar(){}
+
+',
+            '<?php
+namespace Foo {
+    function Bar(){}
+}
+',
+        ];
+
+        yield [
+            '<?php
+            namespace A5 {
+                function AA(){}
+            }
+            namespace B6 {
+                function BB(){}
+            }',
+        ];
+
+        yield [
+            '<?php
+            namespace Foo7;
+                function Bar(){}
+            ',
+            '<?php
+            namespace Foo7 {
+                function Bar(){}
+            }',
+        ];
+
+        yield [
+            '<?php
+            namespace Foo8\\A;
+                function Bar(){}
+             ?>',
+            "<?php
+            namespace Foo8\\A\t \t {
+                function Bar(){}
+            } ?>",
+        ];
+    }
 }

+ 99 - 0
tests/RuleSetTest.php

@@ -579,6 +579,105 @@ final class RuleSetTest extends TestCase
         }, $setDefinitionPHPUnitMigrationNames);
     }
 
+    public function testDuplicateRuleConfigurationInSetDefinitions()
+    {
+        $ruleSet = RuleSet::create();
+        $ruleSetReflection = new \ReflectionObject($ruleSet);
+        $setDefinitions = $ruleSetReflection->getProperty('setDefinitions');
+        $setDefinitions->setAccessible(true);
+        $setDefinitions = $setDefinitions->getValue($ruleSet);
+        $resolvedSets = [];
+
+        foreach ($setDefinitions as $setName => $setDefinition) {
+            $resolvedSets[$setName] = ['rules' => [], 'sets' => []];
+
+            foreach ($setDefinition as $name => $value) {
+                if ('@' === $name[0]) {
+                    $resolvedSets[$setName]['sets'][$name] = $this->expendSet($setDefinitions, $resolvedSets, $name, $value);
+                } else {
+                    $resolvedSets[$setName]['rules'][$name] = $value;
+                }
+            }
+        }
+
+        $duplicates = [];
+
+        foreach ($resolvedSets as $name => $resolvedSet) {
+            foreach ($resolvedSet['rules'] as $ruleName => $config) {
+                if (\count($resolvedSet['sets']) < 1) {
+                    continue;
+                }
+
+                $setDuplicates = $this->findInSets($resolvedSet['sets'], $ruleName, $config);
+
+                if (\count($setDuplicates) > 0) {
+                    if (!isset($duplicates[$name])) {
+                        $duplicates[$name] = [];
+                    }
+
+                    $duplicates[$name][$ruleName] = $setDuplicates;
+                }
+            }
+        }
+
+        if (\count($duplicates) > 0) {
+            $message = '';
+
+            foreach ($duplicates as $setName => $r) {
+                $message .= sprintf("\n%s defines rules the same as it extends from:", $setName);
+
+                foreach ($duplicates[$setName] as $ruleName => $otherSets) {
+                    $message .= sprintf("\n- \"%s\" is also in \"%s\"", $ruleName, implode(', ', $otherSets));
+                }
+            }
+
+            static::fail($message);
+        } else {
+            $this->addToAssertionCount(1);
+        }
+    }
+
+    private function findInSets(array $sets, $ruleName, $config)
+    {
+        $duplicates = [];
+
+        foreach ($sets as $setName => $setRules) {
+            if (\array_key_exists($ruleName, $setRules['rules'])) {
+                if ($config === $setRules['rules'][$ruleName]) {
+                    $duplicates[] = $setName;
+                }
+
+                break; // do not check below, config for the rule has been changed
+            }
+
+            if (\count($setRules['sets']) > 0) {
+                $subSetDuplicates = $this->findInSets($setRules['sets'], $ruleName, $config);
+
+                if (\count($subSetDuplicates) > 0) {
+                    $duplicates = array_merge($duplicates, $subSetDuplicates);
+                }
+            }
+        }
+
+        return $duplicates;
+    }
+
+    private function expendSet($setDefinitions, $resolvedSets, $setName, $setValue)
+    {
+        $rules = $setDefinitions[$setName];
+        foreach ($rules as $name => $value) {
+            if ('@' === $name[0]) {
+                $resolvedSets[$setName]['sets'][$name] = $this->expendSet($setDefinitions, $resolvedSets, $name, $setValue);
+            } elseif (!$setValue) {
+                $resolvedSets[$setName]['rules'][$name] = false;
+            } else {
+                $resolvedSets[$setName]['rules'][$name] = $value;
+            }
+        }
+
+        return $resolvedSets[$setName];
+    }
+
     private static function assertSameRules(array $expected, array $actual, $message = '')
     {
         ksort($expected);