PHP-CS-Fixer / PHP-CS-Fixer

A tool to automatically fix PHP Coding Standards issues
https://cs.symfony.com
MIT License
12.84k stars 1.58k forks source link

global_namespace_import ignores imports for symbols that exist in code but ain't detected #7879

Open javaDeveloperKid opened 7 months ago

javaDeveloperKid commented 7 months ago

Bug report

Description

global_namespace_import ignores classes (and constants?) that exist in code but ain't detected by php-cs-fixer. My case relates to PHPStan array shapes (see example below) but the problem is probably general. I assume php-cs-fixer can understand array shapes because the rule no_unused_imports does not remove the use statement.

Expected behaviour Either analyse PHPStan array shapes or at least report (non zero exit code?) that a use statement for global class/constant exists but it was not found (by php cs fixer) inside the code.

Runtime version

3.51.0

Used command

fix

Configuration file

'global_namespace_import' => ['import_classes' => false, 'import_constants' => false, 'import_functions' => false],
'no_unused_imports' => true,

Code snippet that reproduces the problem

<?php

namespace N;

use DateTimeImmutable;

class Foo
{
    /**
     * @param array{
     *     createdAt: DateTimeImmutable,
     * } $param1
     */
    function f(array $param1)
    {
    }
}

Expected

<?php

namespace N;

-use DateTimeImmutable;

class Foo
{
    /**
     * @param array{
-     *     createdAt: DateTimeImmutable,
+     *     createdAt: \DateTimeImmutable,
     * } $param1
     */
    function f(array $param1)
    {
    }
}
Wirone commented 7 months ago

@javaDeveloperKid please provide expected output, because from the report I just can't tell what's wrong. Used ./php-cs-fixer check --verbose --diff reproducer.php --rules='{"global_namespace_import":{"import_classes":false,"import_constants":false,"import_functions":false},"no_unused_imports":true}' for your snippet and got:

image

Should it be modified to createdAt: \DateTimeImmutable? 🤷‍♂️

Expected behaviour Either analyse PHPStan array shapes or at least report (non zero exit code?) that a use statement for global class/constant exists but it was not found (by php cs fixer) inside the code.

Fixer exits with non-zero code only if dry-run is used and fixable violation is found. The core concept of this tool is to report only what can be fixed, it does not report any kind of "warnings".

mvorisek commented 7 months ago

I think this is duplicate of https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/issues/7619

Wirone commented 7 months ago

Sort of, since it's related to different fixer 🙂.

javaDeveloperKid commented 7 months ago

Updated issue with expected output.

mvorisek commented 7 months ago

So it is related with https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/issues/7674, FQCN fixer is quite complex as it has to parse a lot of different places, global_namespace_import should really reuse that logic.

github-actions[bot] commented 4 months ago

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

javaDeveloperKid commented 4 months ago

keep open

github-actions[bot] commented 4 weeks ago

Since this issue has not had any activity within the last 90 days, I have marked it as stale.

The purpose of this action is to enforce backlog review once in a while. This is mostly for maintainers and helps with keeping repository in good condition, because stale issues and PRs can accumulate over time and make it harder for others to find relevant information. It is also possible that some changes has been made to the repo already, and issue or PR became outdated, but wasn't closed for some reason. This action helps with periodic review and closing of such stale items in automated way.

You may let maintainers handle this or verify current relevancy by yourself, to help with re-triage. Any activity will remove stale label so it won't be automatically closed at this point.

I will close it if no further activity occurs within the next 30 days.

javaDeveloperKid commented 4 weeks ago

keep open