tighten / tlint

Tighten linter for Laravel conventions.
MIT License
521 stars 32 forks source link

`FullyQualifiedFacades` formatter creates duplicate Model imports #345

Closed superbiche closed 10 months ago

superbiche commented 10 months ago

Hi,

When a file uses the Eloquent facade and Illuminate\Database\Eloquent\Model, the FullyQualifiedFacades formatter fixes the Eloquent facade import by importing Illuminate\Database\Eloquent\Model another time, resulting in a duplicate use statement.

Example:
Before fix:

use Eloquent;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;

Run ./vendor/bin/duster fix --using=tlint

After fix:

use Illuminate\Database\Eloquent\Model;
use Exception;
use Illuminate\Database\Eloquent\Builder;
use Illuminate\Database\Eloquent\Model;

I don't know if subsequent tools will fix this, but I suppose it's better if Tlint doesn't create invalid PHP code when formatting.

Maybe just checking if a FQN import of the same class exists in the file would be enough.

Let me know if you want me to give a short at a PR handling this.

Cheers and thanks for your work on this project and Duster <3

driftingly commented 10 months ago

Thanks @superbiche

We might need to add another visitor to clean these up:

    public function leaveNode(Node $node)
    {
        if ($node instanceof Node\Stmt\Use_) {
            foreach ($node->uses as $use) {
                if ($use->name->toString() === $this->className) {
                    // The class has already been imported
                    return NodeTraverser::REMOVE_NODE;
                }
            }
        }
    }

ArrayParametersOverViewWith has an example of how we might create a new visitor with the leaveNode method.