Repository: FriendsOfPHP/PHP-CS-Fixer (php)

Issue:
GroupImportFixer severely broken
## Bug report

I experienced some major issues with the new GroupImportFixer.  It's throwing a parse error on some generated code and severely mangling the output diff.  It looks like it's nesting some group statements, eating the class name itself ("TestClassSoftDeletesuse"), inventing a "D" namespace, etc.  This is from a small small testcase I extracted from a much larger class to demonstrate the issue.  It was also conflicting with the OrderedImportsFixer so I ran it with only the GroupImportFixer, but that is likely a secondary issue.

### Code snippet that reproduces the problem

Test case:
```php
<?php

namespace App\Models;

use App\Models\Traits\JsonObjects;
use App\Services\AccountService;
use App\Services\LocationService;
use DateTime;
use DateTimeZone;
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\BelongsTo;
use Illuminate\Database\Eloquent\Relations\HasOne;
use Illuminate\Database\Eloquent\SoftDeletes;
use Illuminate\Database\Query\JoinClause;
use Illuminate\Notifications\Notifiable;
use Illuminate\Notifications\Notification;
use Illuminate\Support\Arr;
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Log;

class TestClass
{
    use Notifiable;
    use SoftDeletes;
}
```

Output:
```
❯ php-cs-fixer fix -vvv --using-cache=no --dry-run --diff --rules=group_import app/Models/TestClass.php
PHP CS Fixer 2.17.3 Desert Beast by Fabien Potencier and Dariusz Ruminski
Runtime: PHP 7.4.13
Loaded config default.
E
Legend: ?-unknown, I-invalid file syntax (file ignored), S-skipped (cached or empty file), .-no changes, F-fixed, E-error

Checked all files in 0.007 seconds, 20.000 MB memory used

Files that were not fixed due to errors reported during linting after fixing:
   1) app/Models/TestClass.php

                                                                                                 
        [PhpCsFixer\Linter\LintingException]                                                     
        Parse error: syntax error, unexpected 'Illuminate' (T_STRING), expecting '}' on line 9.  
                                                                                                 

      Applied fixers: group_import
      ---------- begin diff ----------
--- Original
+++ New
@@ @@
 <?php
 
 namespace App\Models;
 
 use App\Models\Traits\JsonObjects;
-use App\Services\AccountService;
-use App\Services\LocationService;
-use DateTime;
-use DateTimeZone;
-use Illuminate\Database\Eloquent\Model;
-use Illuminate\Database\Eloquent\Relations\BelongsTo;
-use Illuminate\Database\Eloquent\Relations\HasOne;
-use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Database\Query\JoinClause;
-use Illuminate\Notifications\Notifiable;
-use Illuminate\Notifications\Notification;
-use Illuminate\Support\Arr;
-use Illuminate\Support\Collection;
-use Illuminate\Support\Facades\DB;
-use Illuminate\Support\Facades\Log;
-
-class TestClass
-{
-    use Notifiable;
+use App\Services\{AccountService, LocationService};
+use D\{DateTime, DateTimeZone};
+use Illuminate\Database\Eloquent\{classModeluse Illuminate\Database\Eloquent\Relations\{ BelongsTo, HasOne};
+use Illuminate\Database\Eloquent\{TestClassSoftDeletesuse Illuminate\Notifications\{
+Notifiable, Notification};
+use Illuminate\Support\{{Arr, Collection};
+use Illuminate\Support\Facades\{
+    DB, Log};
+use Notifiable;
     use SoftDeletes;
 }
 

      ----------- end diff -----------
```

Pull request:
GroupImportFixer severely broken
Fixes #5408 

Patch:
diff --git a/composer.json b/composer.json
index 40c8849a7..971b425b6 100644
--- a/composer.json
+++ b/composer.json
@@ -18,7 +18,7 @@
         "ext-json": "*",
         "ext-tokenizer": "*",
         "composer/semver": "^1.4 || ^2.0 || ^3.0",
-        "composer/xdebug-handler": "^1.2 || ^2.0",
+        "composer/xdebug-handler": "^1.2",
         "doctrine/annotations": "^1.2",
         "php-cs-fixer/diff": "^1.3",
         "symfony/console": "^3.4.43 || ^4.1.6 || ^5.0",
diff --git a/doc/rules/doctrine_annotation/doctrine_annotation_array_assignment.rst b/doc/rules/doctrine_annotation/doctrine_annotation_array_assignment.rst
index 999dc8bf0..e71dd1f66 100644
--- a/doc/rules/doctrine_annotation/doctrine_annotation_array_assignment.rst
+++ b/doc/rules/doctrine_annotation/doctrine_annotation_array_assignment.rst
@@ -14,7 +14,7 @@ List of tags that must not be treated as Doctrine Annotations.
 
 Allowed types: ``array``
 
-Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'psalm', 'phpstan', 'fix', 'FIXME', 'fixme', 'override']``
+Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'fix', 'FIXME', 'fixme', 'override']``
 
 ``operator``
 ~~~~~~~~~~~~
diff --git a/doc/rules/doctrine_annotation/doctrine_annotation_braces.rst b/doc/rules/doctrine_annotation/doctrine_annotation_braces.rst
index 8852b7798..a0f029be5 100644
--- a/doc/rules/doctrine_annotation/doctrine_annotation_braces.rst
+++ b/doc/rules/doctrine_annotation/doctrine_annotation_braces.rst
@@ -14,7 +14,7 @@ List of tags that must not be treated as Doctrine Annotations.
 
 Allowed types: ``array``
 
-Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'psalm', 'phpstan', 'fix', 'FIXME', 'fixme', 'override']``
+Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'fix', 'FIXME', 'fixme', 'override']``
 
 ``syntax``
 ~~~~~~~~~~
diff --git a/doc/rules/doctrine_annotation/doctrine_annotation_indentation.rst b/doc/rules/doctrine_annotation/doctrine_annotation_indentation.rst
index 62f8d32bf..2e5f774f7 100644
--- a/doc/rules/doctrine_annotation/doctrine_annotation_indentation.rst
+++ b/doc/rules/doctrine_annotation/doctrine_annotation_indentation.rst
@@ -14,7 +14,7 @@ List of tags that must not be treated as Doctrine Annotations.
 
 Allowed types: ``array``
 
-Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'psalm', 'phpstan', 'fix', 'FIXME', 'fixme', 'override']``
+Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'fix', 'FIXME', 'fixme', 'override']``
 
 ``indent_mixed_lines``
 ~~~~~~~~~~~~~~~~~~~~~~
diff --git a/doc/rules/doctrine_annotation/doctrine_annotation_spaces.rst b/doc/rules/doctrine_annotation/doctrine_annotation_spaces.rst
index 046a4c3ee..a253fa1d7 100644
--- a/doc/rules/doctrine_annotation/doctrine_annotation_spaces.rst
+++ b/doc/rules/doctrine_annotation/doctrine_annotation_spaces.rst
@@ -21,7 +21,7 @@ List of tags that must not be treated as Doctrine Annotations.
 
 Allowed types: ``array``
 
-Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'psalm', 'phpstan', 'fix', 'FIXME', 'fixme', 'override']``
+Default value: ``['abstract', 'access', 'code', 'deprec', 'encode', 'exception', 'final', 'ingroup', 'inheritdoc', 'inheritDoc', 'magic', 'name', 'toc', 'tutorial', 'private', 'static', 'staticvar', 'staticVar', 'throw', 'api', 'author', 'category', 'copyright', 'deprecated', 'example', 'filesource', 'global', 'ignore', 'internal', 'license', 'link', 'method', 'package', 'param', 'property', 'property-read', 'property-write', 'return', 'see', 'since', 'source', 'subpackage', 'throws', 'todo', 'TODO', 'usedBy', 'uses', 'var', 'version', 'after', 'afterClass', 'backupGlobals', 'backupStaticAttributes', 'before', 'beforeClass', 'codeCoverageIgnore', 'codeCoverageIgnoreStart', 'codeCoverageIgnoreEnd', 'covers', 'coversDefaultClass', 'coversNothing', 'dataProvider', 'depends', 'expectedException', 'expectedExceptionCode', 'expectedExceptionMessage', 'expectedExceptionMessageRegExp', 'group', 'large', 'medium', 'preserveGlobalState', 'requires', 'runTestsInSeparateProcesses', 'runInSeparateProcess', 'small', 'test', 'testdox', 'ticket', 'uses', 'SuppressWarnings', 'noinspection', 'package_version', 'enduml', 'startuml', 'fix', 'FIXME', 'fixme', 'override']``
 
 ``around_parentheses``
 ~~~~~~~~~~~~~~~~~~~~~~
diff --git a/phpstan.neon b/phpstan.neon
index f6756723c..18dac5cce 100644
--- a/phpstan.neon
+++ b/phpstan.neon
@@ -59,8 +59,5 @@ parameters:
         -
             message: '/^\$this\(PhpCsFixer\\Tokenizer\\Tokens\) does not accept PhpCsFixer\\Tokenizer\\Token\|null\.$/'
             path: src/Tokenizer/Tokens.php
-        -
-            message: '/^Class Test\dConfig not found.$/'
-            path: tests/Console/ConfigurationResolverTest.php
 
     tipsOfTheDay: false
diff --git a/src/AbstractDoctrineAnnotationFixer.php b/src/AbstractDoctrineAnnotationFixer.php
index f1ef7d5b5..a47eb667b 100644
--- a/src/AbstractDoctrineAnnotationFixer.php
+++ b/src/AbstractDoctrineAnnotationFixer.php
@@ -184,12 +184,6 @@ protected function createConfigurationDefinition()
                     'enduml',
                     'startuml',
 
-                    // Psalm
-                    'psalm',
-
-                    // PHPStan
-                    'phpstan',
-
                     // other
                     'fix',
                     'FIXME',
diff --git a/src/Fixer/Import/GroupImportFixer.php b/src/Fixer/Import/GroupImportFixer.php
index 1771f87d4..1c7620d7f 100644
--- a/src/Fixer/Import/GroupImportFixer.php
+++ b/src/Fixer/Import/GroupImportFixer.php
@@ -97,7 +97,12 @@ function (NamespaceUseAnalysis $useDeclaration) {
             return \in_array($namespaceName, $sameNamespaces, true);
         });
 
-        sort($sameNamespaceAnalysis);
+        usort($sameNamespaceAnalysis, function (NamespaceUseAnalysis $a, NamespaceUseAnalysis $b) {
+            $namespaceA = $this->getNamespaceNameWithSlash($a);
+            $namespaceB = $this->getNamespaceNameWithSlash($b);
+
+            return \strlen($namespaceA) - \strlen($namespaceB) ?: strcmp($a->getFullName(), $b->getFullName());
+        });
 
         return $sameNamespaceAnalysis;
     }


Test patch:
diff --git a/tests/Console/ConfigurationResolverTest.php b/tests/Console/ConfigurationResolverTest.php
index d2fe33087..bd04936ab 100644
--- a/tests/Console/ConfigurationResolverTest.php
+++ b/tests/Console/ConfigurationResolverTest.php
@@ -205,7 +205,7 @@ public function testResolveConfigFileByPathOfFile()
         $resolver = $this->createConfigurationResolver(['path' => [$dir.\DIRECTORY_SEPARATOR.'foo.php']]);
 
         static::assertSame($dir.\DIRECTORY_SEPARATOR.'.php_cs.dist', $resolver->getConfigFile());
-        static::assertInstanceOf(\Test1Config::class, $resolver->getConfig());
+        static::assertInstanceOf('Test1Config', $resolver->getConfig());
     }
 
     public function testResolveConfigFileSpecified()
@@ -215,7 +215,7 @@ public function testResolveConfigFileSpecified()
         $resolver = $this->createConfigurationResolver(['config' => $file]);
 
         static::assertSame($file, $resolver->getConfigFile());
-        static::assertInstanceOf(\Test4Config::class, $resolver->getConfig());
+        static::assertInstanceOf('Test4Config', $resolver->getConfig());
     }
 
     /**
diff --git a/tests/Fixer/DoctrineAnnotation/DoctrineAnnotationBracesFixerTest.php b/tests/Fixer/DoctrineAnnotation/DoctrineAnnotationBracesFixerTest.php
index 0268686f5..7a75770c7 100644
--- a/tests/Fixer/DoctrineAnnotation/DoctrineAnnotationBracesFixerTest.php
+++ b/tests/Fixer/DoctrineAnnotation/DoctrineAnnotationBracesFixerTest.php
@@ -253,14 +253,6 @@ public function provideFixWithBracesCases()
  * @enduml
  * @startuml
  *
- * // Psalm
- * @psalm
- * @psalm-param
- *
- * // PHPStan
- * @phpstan
- * @phpstan-param
- *
  * // other
  * @fix
  * @FIXME
@@ -533,15 +525,6 @@ public function provideFixWithoutBracesCases()
  * @enduml()
  * @startuml()
  *
- * // Psalm
- * @psalm()
- * @psalm-param()
- *
- * // PHPStan
- * @phpstan()
- * @psalm-param()
- *
- *
  * // other
  * @fix()
  * @FIXME()
diff --git a/tests/Fixer/Import/GroupImportFixerTest.php b/tests/Fixer/Import/GroupImportFixerTest.php
index ad276e4c7..b38815d61 100644
--- a/tests/Fixer/Import/GroupImportFixerTest.php
+++ b/tests/Fixer/Import/GroupImportFixerTest.php
@@ -247,6 +247,49 @@ public function provideFixCases()
 use Foo\Baz;
 use \ReflectionClass;
 use \ReflectionMethod;
+',
+            ],
+            [
+                '<?php
+
+use Framework\Support\{Arr, Collection};
+use Framework\Database\ORM\{Model, SoftDeletes};
+use Framework\Notifications\{Notifiable, Notification};
+use Framework\Support\Facades\{DB, Log};
+use Framework\Database\ORM\Relations\{BelongsTo, HasOne};
+use Framework\Database\Query\JoinClause;
+',
+                '<?php
+
+use Framework\Database\ORM\Model;
+use Framework\Database\ORM\Relations\BelongsTo;
+use Framework\Database\ORM\Relations\HasOne;
+use Framework\Database\ORM\SoftDeletes;
+use Framework\Database\Query\JoinClause;
+use Framework\Notifications\Notifiable;
+use Framework\Notifications\Notification;
+use Framework\Support\Arr;
+use Framework\Support\Collection;
+use Framework\Support\Facades\DB;
+use Framework\Support\Facades\Log;
+',
+            ],
+            [
+                '<?php
+
+use Framework\Baz\Class6;
+use Framework\Bar\{Class3, Class4, Class5};
+use Framework\Foo\{Class1, Class2, Class7};
+',
+                '<?php
+
+use Framework\Foo\Class1;
+use Framework\Foo\Class2;
+use Framework\Bar\Class3;
+use Framework\Bar\Class4;
+use Framework\Bar\Class5;
+use Framework\Baz\Class6;
+use Framework\Foo\Class7;
 ',
             ],
         ];