Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
javaDeveloperKid opened this issue Mar 12, 2024 · 5 comments
Labels
kind/bug topic/fqcn Fully Qualified Class Name usage and conversions

Comments

@javaDeveloperKid
Copy link

javaDeveloperKid commented Mar 12, 2024

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
Copy link
Member

Wirone commented Mar 12, 2024

@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".

@Wirone Wirone added the status/input required For the issue to be confirmed or the PR to be process; additional input of the author is required label Mar 12, 2024
@mvorisek
Copy link
Contributor

I think this is duplicate of #7619

@Wirone
Copy link
Member

Wirone commented Mar 13, 2024

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

@javaDeveloperKid
Copy link
Author

Updated issue with expected output.

@mvorisek
Copy link
Contributor

So it is related with #7674, FQCN fixer is quite complex as it has to parse a lot of different places, global_namespace_import should really reuse that logic.

@Wirone Wirone added topic/fqcn Fully Qualified Class Name usage and conversions and removed status/input required For the issue to be confirmed or the PR to be process; additional input of the author is required labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug topic/fqcn Fully Qualified Class Name usage and conversions
Projects
None yet
Development

No branches or pull requests

3 participants