Php-cs-fixer: no_unused_imports no working correct

[DjThossi]

Hey in our project we got some trouble with newest version (2.0.0) of php-cs-fixer and no_unused_imports rule.

If the name of an imported class (e.g. Design) is anywhere else used (e.g. $this->design or //design) the import is not removed even if it should be removed.

PHP version we are using:

$ php -v
PHP 5.6.29-1~dotdeb+7.1 (cli) (built: Dec  9 2016 16:30:46) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.4.0, Copyright (c) 2002-2016, by Derick Rethans

PHP CS Fixer version we are using:

$ php-cs-fixer -V
PHP CS Fixer version 2.0.0 by Fabien Potencier and Dariusz Ruminski

The command we use to run the fixer

$ bin/php-cs-fixer fix -v

The configuration file we are using

<?php

return PhpCsFixer\Config::create()
    ->setRiskyAllowed(true)
    ->setRules(array(
        'no_unused_imports' => true,
    ))
    ->setFinder(
        PhpCsFixer\Finder::create()
            ->in(__DIR__ . '/src')
    );

A minimal sample of valid PHP code that is not fixed correctly

<?php
namespace App\Example;

use App\Test1;
use App\Test2;
use App\Test3;

class Example
{
    protected function getExample($config)
    {
        //test1
        'test2';
        $this->test3;
    }
}

The fixed version of that code after you run the fixer and the version you expected.

<?php
namespace App\Example;

class Example
{
    protected function getExample($config)
    {
        //test1
        'test2';
        $this->test3;
    }
}

[GrahamCampbell]

Are you saying that the test1 import is not removed? If so, that's the expected behaviour, to allow people to reference class names from comments.

[SpacePossum]

Are you saying that the test1 import is not removed?

It seems all imports are left intact.
I agree with @GrahamCampbell that test1 should not be removed. The imports test2 and test3 could be removed. IMO it is not a bug, but a lacking feature (i.e. missed possibility). Love to see a PR for this!

[keradus]

Closed as discussed.

[DjThossi]

Test1:
@GrahamCampbell I get your point that it would be good to reference Classes in comments.
BUT.
In my opinion should the import only stay if casing is matching. In my example I use lower case version in comment. In this case it should be taken out.

Test2 & Test3:
For these 2 cases imports aren't taken out as well.

If this is a feature, would it be good to make this configurable in order to have the possibility of a more strict version.

[SpacePossum]

As stated I would like to see this feature, especially test2 and test3. Hope someone find the time to make it! : D

[keradus]

Sorry, misunderstood your previous comment @SpacePossum , reopening

---

@DjThossi , for test1 problematic thing is that for some frameworks, like phpunit or doctrine, casing is ignored.
test2 and test3 could be removed indeed.
I do not treat it as a bug - we remove as much as we could without breaking a code. It would be a bug if the code would remove too much.
I treat it as an enhancement - nice to have it for sure. Do you want to propose a PR ?

[DjThossi]

@keradus I'd love to do a PR but I have too many things on my plate already. I'm sorry.

[GrahamCampbell]

In my opinion should the import only stay if casing is matching.

php is not case sensitive when resolving class names