Symbols' casing is changed when there's name collision between imported class and imported function

[soyuka]

Bug report

Description

There's a weird behavior when a variable has the same name as an imported class, and where this class is used via the static operator ::.

Runtime version

❯ php-cs-fixer fix -vvv --dry-run
PHP CS Fixer 3.47.1-DEV Big Changes by Fabien Potencier, Dariusz Ruminski and contributors.
PHP runtime: 8.2.13
Loaded config default from "/home/soyuka/forks/core2/.php-cs-fixer.dist.php".

   1) docs/src/Kernel.php (fully_qualified_strict_types, no_extra_blank_lines, blank_line_between_import_groups)
   2) src/Metadata/Operation/Factory/OperationMetadataFactory.php (fully_qualified_strict_types)
   3) src/Elasticsearch/Tests/Serializer/ItemNormalizerTest.php (fully_qualified_strict_types)
   4) src/Symfony/EventListener/WriteListener.php (no_extra_blank_lines)
   5) tests/Doctrine/Odm/PropertyInfo/DoctrineExtractorTest.php (fully_qualified_strict_types)

Configuration file

https://github.com/api-platform/core/blob/main/.php-cs-fixer.dist.php

Code snippet that reproduces the problem

Before:

<?php
namespace ApiPlatform\Playground;

use Symfony\Component\HttpFoundation\Request;
use function App\Playground\request;

class Kernel
{
  public function request(Request $request = null): Response
  {
      if (null === $request && \function_exists('App\Playground\request')) {
          $request = request();
      }

      $request = $request ?? Request::create('/docs.json');
      $response = $this->handle($request);
      $response->send();
      $this->terminate($request, $response);

      return $response;
  }
}

After:

<?php
namespace ApiPlatform\Playground;

use Symfony\Component\HttpFoundation\Request;

use function App\Playground\request;

class Kernel
{
    public function request(request $request = null): Response
    {
        if (null === $request && \function_exists('App\Playground\request')) {
            $request = request();
        }

        $request = $request ?? request::create('/docs.json');
        $response = $this->handle($request);
        $response->send();
        $this->terminate($request, $response);

        return $response;
    }
}

-    public function request(Request $request = null): Response
+    public function request(request $request = null): Response
     {
         if (null === $request && \function_exists('App\Playground\request')) {
             $request = request();
         }

-        $request = $request ?? Request::create('/docs.json');
+        $request = $request ?? request::create('/docs.json');
         $response = $this->handle($request);
         $response->send();
         $this->terminate($request, $response);

[Wirone]

The problem is in imports (Request class and request function), not in variable name. I believe this will be fixed with #7704 / #7705 (depending on the chosen implementation), because imports will be sorted by kind. Now all the imports are collected into one list, which makes the matching inaccurate (last one is used). However, in terms of ::class it will always be somehow inaccurate, because we're probably going to treat all these usages related to class imports (because Fixer is rather unable to determine proper context).

[dkrnl]

similar bug with namespace -- replaced lower case to upper case

 controllers/AbstractController.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/controllers/AbstractController.php b/controllers/AbstractController.php
index d589486e..7b238d0e 100644
--- a/controllers/AbstractController.php
+++ b/controllers/AbstractController.php
@@ -9,7 +9,7 @@ use Yii;
  *
  */
-abstract class AbstractController extends \yii\web\Controller
+abstract class AbstractController extends Yii\web\Controller
 {
     /**
      * {@inheritdoc}

[Wirone]

@dkrnl your case is rather related to #7721, as FQCN was shortened using relative name based on use Yii;.

[limenet]

A similar issue (though I'm not sure if the root cause is the same) occurs in this case. Note how the property name now starts with a capital letter (like the class name that's being imported).

Foo.php (fully_qualified_strict_types)
---
use Bar\LanguageService;

    public function __construct(
        private readonly LanguageService $languageService,
    ) {
    }

-        $locale = $this->languageService::get(
+        $locale = $this->LanguageService::get(

Apologies for the redacted diff/excerpt.

[soyuka]

HI @Wirone do you want me to test these patches? Note that this is happening in more then one case I took the simplest one, see https://github.com/api-platform/core/actions/runs/7534511533/job/20509019181.

[Wirone]

@soyuka if you're so kind and verify if these PRs fix the problem then would be great 🙂.

[Wirone]

Quick fix was provided, proper support for import kinds will land later.

[soyuka]

Indeed it fixes some of my issues, I ran #7705 and #7704 but I still have issues with uppercases.

Should I open new issues for these?

[kubawerlos]

@soyuka #7749 should fix the 1st change in your example.

[Wirone]

@soyuka as @kubawerlos said, 1st is already fixed.

As for the 2nd: clear bug, fixer should not prefer relative FQNs over short names. As far as I see, OperationMetadataFactoryInterface is from the same namespace as OperationMetadataFactory so it's not in imports and is treated like something that should be compared with the existing imports. Since there's also use ApiPlatform\Metadata\Operation;, which is a class with FQCN which is a substring of OperationMetadataFactoryInterface's FQCN, it was wrongly replaced with relative reference. Fix in #7752.

I'll take a look at 3rd one too 🙂.

[Wirone]

@soyuka the 3rd one looks like an actual fix 😅. In DoctrineExtractorTest you have use Symfony\Component\PropertyInfo\Type; and there are many Type::BUILTIN_TYPE_* usages, but that one TYPE::BUILTIN_TYPE_INT has wrong casing. Was it there on purpose and should remain like this? Because otherwise IMHO it's good that the casing was fixed.

[soyuka]

No way my bad on the third one I thought that php-cs-fixer changed my Type to TYPE I must have then misread. Thanks! I've tested the patch at #7752 and it works.