Skip to content

[CodingStyle][Namespace_] Support constant imports#4612

Merged
samsonasik merged 8 commits intorectorphp:mainfrom
natepage:main
Jul 28, 2023
Merged

[CodingStyle][Namespace_] Support constant imports#4612
samsonasik merged 8 commits intorectorphp:mainfrom
natepage:main

Conversation

@natepage
Copy link
Copy Markdown
Contributor

Hi there 👋

In one of our project, we are using constants defined outside of a class, those constants are defined by a PHP extension.
We've been facing issues with Rector trying to replace the usage of those constants by imports incorrectly.

This PR extends the names importing logic to import constants the right way, really similar to the logic for functions imports.

Here is a simplified example of the situation in our project:

namespace App;

final class Pkcs11Encryptor
{
    private function findKey(string $keyName): object
    {
        $objects = $this->session->findObjects([
            \Pkcs11\CKA_LABEL => $keyName, // <-- Rector wants to replace this by an import
        ]);

        // Rest of the logic...
    }
}

The current names import logic will result in the following:

namespace App;

use Pkcs11\CKA_LABEL; // <-- This is broken "Class CKA_LABEL is not defined..."

final class Pkcs11Encryptor
{
    private function findKey(string $keyName): object
    {
        $objects = $this->session->findObjects([
            CKA_LABEL => $keyName, // <-- Replaced by import
        ]);

        // Rest of the logic...
    }
}

The logic from this PR will result in the following:

namespace App;

use const Pkcs11\CKA_LABEL; // <-- Special import for constant

final class Pkcs11Encryptor
{
    private function findKey(string $keyName): object
    {
        $objects = $this->session->findObjects([
            CKA_LABEL => $keyName, // <-- Replaced by import
        ]);

        // Rest of the logic...
    }
}

@natepage natepage requested a review from TomasVotruba as a code owner July 27, 2023 07:09
@samsonasik
Copy link
Copy Markdown
Member

it seems there is infinite loop in test, could you fix it? thank you.

@natepage
Copy link
Copy Markdown
Contributor Author

@samsonasik Thank you for your reply, I fixed it, no sure what happened but my commit didn't contain all the changes... 😢

Comment thread rules/CodingStyle/ClassNameImport/UsedImportsResolver.php Outdated
Comment thread rules/CodingStyle/ClassNameImport/UsedImportsResolver.php Outdated
Comment thread rules/CodingStyle/ClassNameImport/UsedImportsResolver.php Outdated
Comment thread rules/CodingStyle/ClassNameImport/UsedImportsResolver.php Outdated
@samsonasik
Copy link
Copy Markdown
Member

Thank you @natepage 👍

This looks good 👍 @TomasVotruba let's merge it as it is a bug fix due to const is not imported correctly :)

@samsonasik samsonasik merged commit 682df75 into rectorphp:main Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants