Add rule for converting Enum cases to PascalCase naming #6890

marcelthole · 2025-05-09T12:16:45Z

Found the idea yesterday from @boesing and didn't find a rector rule for it.

In this PR we would rename the cases of a Enum to PascalCase and add a post rector to change the usages of all changed enums.

I have some open Question about my PR because i'm not very familar with the PostRector stuff.

Do i really need a e2e test to check, that i do not modify a Enum that was not changed in the run by rector already?
How should i optimize the perfomance of the PostRector that this should not run every time and not for every node?

samsonasik · 2025-05-09T12:37:20Z

src/PostRector/Application/PostFileProcessor.php

@@ -126,6 +128,7 @@ private function getPostRectors(): array
            $postRectors[] = $this->unusedImportRemovingPostRector;
        }

+        $postRectors[] = $this->enumReferenceUpdaterPostRector;


Add new PostRector probably need check if RenamedEnumCaseCollector->hasNames() so no need to traverse EnumReferenceUpdaterPostRector when the rule not executed.

Not sure if PostRector actually needed tho, define multiple node possibly enough:

public function getNodeTypes(): array { - return [Enum_::class]; + return [Enum_::class, ClassConstFetch::class]; }

Then, in refactor, verify:

/** - * @param Enum_ $node + * @param Enum_|ClassConstFetch $node */ public function refactor(Node $node): ?Node { + if ($node instanceof ClassConstFetch) { + // process with ClassConstFetch here... + } // process with Enum_ here ... }

PostRectors are limited only for internal use only. Should not be touched, unless fixing something else.

@TomasVotruba that's what I suggest on code diff suggestion above with define multiple nodes ( Enum_::class, ClassConstFetch::class ) in the rector rule itself.

I used that kind of rule also as first idea but would have the problem, that i would change all Enum cases regardingless if its in a 3rd party vendor directory or not? Or i want to use that rule only for a specific module (like the e2e Test) i should not rename all enum cases i will find?

The goal should be, that only that Enum cases are modified in the code that are really touched.

Rector is working file by file, so overlapped file with ClassConstFetch first can happen, so it not changed because Enum not changed first, eg list of files:

- FileWithClassConstFetch.php - TheEnumItself.php

so the Collector won't really working, as it only usefull on "that file" or "after" that file.

The possible solution for your usecase, possibly verify file is on target "paths" range. To get exact paths to process, you may need to save list of "filePaths" into Parameter Provider, this one:

rector-src/src/Application/ApplicationFileProcessor.php

Line 60 in 3be6fd7

$filePaths = $this->filesFinder->findFilesInPaths($configuration->getPaths(), $configuration);

save into :

SimpleParameterProvider::setParameter(Option::FILE_PATHS, $filePaths);

then, you can verify if in on in_array

in_array($targetEnumFile, SimpleParameterProvider::provideArrayParameter(Option::FILE_PATHS), true)

On addition of above, to get Enum target file, you can do via ReflectionProvider, then get file

On summary, when changing ClassConstFetch, verify that the target Enum file ( get via ReflectionProvider) is on target "filePaths" (the files that rector try to change) so safer,

ok, if Target Enum file is 3rd party, then it should only verify renamed ClassConstFetch exists via ClassReflection that you can pull from ReflectionProvider

Thank you very much for these code examples. This helped me a lot :)
I could also remove the e2e test 👍

marcelthole · 2025-05-12T10:39:31Z

Check the windows Build

samsonasik · 2025-05-12T10:40:23Z

src/Application/ApplicationFileProcessor.php

+        if (SimpleParameterProvider::provideArrayParameter(Option::FILE_PATHS) === []) {
+            SimpleParameterProvider::setParameter(Option::FILE_PATHS, $filePaths);
+        }


On parallel process, this won't catch all files, as this only execute collection of files that on current job per process, eg; 16 files per configured parallel job size

rector-src/src/Config/RectorConfig.php

Line 100 in ac3a941

int $jobSize = 16

I am thinking of cache the list of files on very first execution, that's the possible way it can get all files on parallel process on FilesFinder::findInDirectoriesAndFiles()

public function findInDirectoriesAndFiles() { $filePaths = [...$filteredFilePaths, ...$filteredFilePathsInDirectories]; // cache here ... $this->cache->save(key, variableKey, $filePaths); return $this->unchangedFilesFilter->filterFilePaths($filePaths);

at

rector-src/src/FileSystem/FilesFinder.php

Line 104 in ac3a941< 8000 /a>

$filePaths = [...$filteredFilePaths, ...$filteredFilePathsInDirectories];

There is Rector\Caching\Cache service that may can be used for it.

then you pull from cache when exists on processFiles()

@marcelthole I created example of cache usage on PR:

[Draft] Example of cache list of files #6894

feel free to cherry-pick/copy :)

samsonasik · 2025-05-12T20:01:21Z

rules/CodingStyle/Rector/Enum_/EnumCaseToPascalCaseRector.php

+        $targetEnumFile = $this->reflectionProvider->getClass($classConstFetch->class->toString())
+            ->getFileName();
+        if (! in_array($targetEnumFile, SimpleParameterProvider::provideArrayParameter(Option::FILE_PATHS), true)) {
+            return null;
+        }


@marcelthole ok, the another possible solution is by using DynamicSourceLocatorProvider, you can use the following diff:

Inject to __construct

use PHPStan\BetterReflection\Reflector\DefaultReflector; use PHPStan\BetterReflection\Identifier\Identifier; use PHPStan\BetterReflection\Identifier\IdentifierType; use PHPStan\BetterReflection\Reflection\ReflectionClass; use Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider\DynamicSourceLocatorProvider; public function __construct( // ... private readonly DynamicSourceLocatorProvider $dynamicSourceLocatorProvider, )

Check target, it not exists in DynamicSourceLocatorProvider, then skip

$sourceLocator = $this->dynamicSourceLocatorProvider->provide(); $defaultReflector = new DefaultReflector($sourceLocator); $classIdentifier = $sourceLocator->locateIdentifier( $defaultReflector, new Identifier($classConstFetch->class->toString(), new IdentifierType(IdentifierType::IDENTIFIER_CLASS)), ); if (! $classIdentifier instanceof ReflectionClass) { return null; }

that's however, won't really work well on unit test since unit test work file by file

rector-src/src/Testing/PHPUnit/AbstractRectorTestCase.php

Lines 262 to 264 in ce8575f

private function processFilePath(string $filePath): RectorTestResult

{

$this->dynamicSourceLocatorProvider->setFilePath($filePath);

so for another file, the e2e may possibly the way to test that.

Hey, thanks for your input. Played a little bit around and hat sadly some issues with the cache if i use the cache option with multiple runs.

i introduced now a helper class to check if the file is in the paths/source from rector and run the command in that case and fixed 2 bugs with that. First: set the source also in the code and not only in the Unit test and second: use the correct directory separator on windows

samsonasik · 2025-05-12T20:56:58Z

src/Util/FilePath.php

+ */
+final class FilePath
+{
+    public static function fileIsInRectorPathOrSource(string $file): bool


The path logic may need to similar with

rector-src/src/Configuration/ConfigurationFactory.php

Line 140 in ce8575f

private function resolvePaths(InputInterface $input): array

I will try if I can create alternative PR with DynamicSourceLocatorProvider per

#6890 (review)

when I have a chance :)

This case is interesting of how locate identifier can works in the future imo :)

see alternative PR:

[Alternative] Add EnumCaseToPascalCaseRector with use of DynamicSourceLocatorProvider #6899

:)

marcelthole · 2025-05-13T05:32:12Z

use the better solution from @samsonasik in #6899 👍

samsonasik reviewed May 9, 2025

View reviewed changes

marcelthole force-pushed the pascal-case-enum-names branch from 65f2803 to efde06a Compare May 12, 2025 10:35

samsonasik reviewed May 12, 2025

View reviewed changes

samsonasik mentioned this pull request May 12, 2025

[Draft] Example of cache list of files #6894

Closed

samsonasik reviewed May 12, 2025

View reviewed changes

Add rule for converting Enum cases to PascalCase naming

776cf41

marcelthole force-pushed the pascal-case-enum-names branch from efde06a to 776cf41 Compare May 12, 2025 20:43

samsonasik reviewed May 12, 2025

View reviewed changes

samsonasik mentioned this pull request May 12, 2025

[Alternative] Add EnumCaseToPascalCaseRector with use of DynamicSourceLocatorProvider #6899

Open

marcelthole closed this May 13, 2025

marcelthole deleted the pascal-case-enum-names branch May 13, 2025 05:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add rule for converting Enum cases to PascalCase naming #6890

Add rule for converting Enum cases to PascalCase naming #6890

	private function processFilePath(string $filePath): RectorTestResult
	{
	$this->dynamicSourceLocatorProvider->setFilePath($filePath);

Add rule for converting Enum cases to PascalCase naming #6890

Add rule for converting Enum cases to PascalCase naming #6890

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment