NoUnusedImportsFixer - Fix undetected unused imports when type mismatch

[julienfalque]

Fixes #3020.

[coveralls]

Coverage increased (+0.01%) to 92.293% when pulling 1fc2bda on julienfalque:unused-import-types into 810c3c0 on FriendsOfPHP:3.0.

[GrahamCampbell]

Nice. 👍

[GrahamCampbell]

This incorrectly removes on of the imports for a union catch:

// ...
} catch (InvalidFormatException | ParseErrorException) {

[julienfalque]

@GrahamCampbell I don't reproduce that, could you provide the full snippet please?

[GrahamCampbell]

Oh, I think the issue only occurs when combined with #5685.

[julienfalque]

@GrahamCampbell Actually I didn't notice your snippet uses a non-capturing catch 😐. I do reproduce the issue now, trying to fix it.

[julienfalque]

Fixed in #5685, which this PR now depends on. Thanks @GrahamCampbell!

[GrahamCampbell]

I think this also has an issue with PHP 8 attributes.

<?php

use Symfony\Component\Routing\Annotation\Route;

class Controller
{
    #[Route('/foo', name: 'foo')]
    public function foo(Request $request): Response
    {
    }
}

[julienfalque]

@GrahamCampbell Fixed too, thanks 👍

[GrahamCampbell]

Where there other changes there, or just a rebase?

[julienfalque]

Only a rebase (onto #5741 instead of #5685).

[SpacePossum]

Fixed another issue for goto labels and in detecting goto labels. Please rebase and squash my commits out.
There is a thing I'm not 100% about:

                if ($analyzer->isConstantInvocation($index)) {
                    $type = NamespaceUseAnalysis::TYPE_CONSTANT;
                } elseif ($nextMeaningfulToken->equals('(') && !$prevMeaningfulToken->isGivenKind($tokensNotBeforeFunctionCall)) {
                    $type = NamespaceUseAnalysis::TYPE_FUNCTION;
                } else {
                    $type = NamespaceUseAnalysis::TYPE_CLASS;
                }

First case is 100% constant invocation. The second, function and 3rd (default) class, are not checked on the same detail level. If T_STRING would be used in the future for a new PHP feature we would default here to class, while previous to this PR the default was to assume it was used. Could we improve the testing without adding to much code?

[julienfalque]

If T_STRING would be used in the future for a new PHP feature we would default here to class, while previous to this PR the default was to assume it was used. Could we improve the testing without adding to much code?

I think the current code is fine. If we know for sure that the tested token does not match a function or const import, then the token can only match a class/interface/trait import, which is what NamespaceUseAnalysis::TYPE_CLASS means. This would only fail if PHP introduces a new type of import but then supporting it would be part of the updates we do for each new PHP version.

[SpacePossum]

The goto case was also considered a "class" usage by the code because it is a T_STRING and is not a constant and not a function. I think when PHP would use the T_STRING token in the future for something different or new it would happen again.
For now I cannot think of such a case so we should be good though :)

[GrahamCampbell]

Merge conflicts. 😭

[keradus]

Thank you @julienfalque.

[keradus]

thanks @SpacePossum

[andyrooster]

@julienfalque I think this changed has caused an issue with not detecting PHP 8 attributes usage when there is more than one in a block, for example with the below code the IsGranted and JsonSchema use statements get removed using CS Fixer v3.1.0. This doesn't happen in CS Fixer v3.0.2

use Sensio\Bundle\FrameworkExtraBundle\Configuration\IsGranted;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Core\Security;

...
#[
    Route('/basket/{uuid}/item', name: 'addBasketItem', requirements: ['uuid' => '%regex.uuid%'], methods: ['POST']),
    IsGranted('ROLE_USER'),
    JsonSchema('Public/Basket/addItem.json'),
]

[GrahamCampbell]

@andyrooster can you create a bug ticket for this following the template, so this can be properly tracked?

[andyrooster]

@GrahamCampbell #5940