Skip to content

Conversation

calebdw
Copy link
Contributor

@calebdw calebdw commented Sep 14, 2025

Hello!

This fixes some more bugs in the rector:

  1. cases on self were not being renamed
  2. prevents enum constants from being renamed

Thanks!

@calebdw calebdw force-pushed the calebdw/push-utwsxnywtlln branch 4 times, most recently from 2de6166 to ec669ad Compare September 14, 2025 20:06
@calebdw calebdw force-pushed the calebdw/push-utwsxnywtlln branch from ec669ad to 5a7cc94 Compare September 14, 2025 20:17

return RectorConfig::configure()
->withRules([EnumCaseToPascalCaseRector::class]);
->withRules([EnumCaseToPascalCaseRector::class])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like other your PR, Please don't change existing test config, use different class & config on purpose, with use of dynamic provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case there's really no need for a separate config and test case just to test one fixture

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the test include existing Fixture in autoloaded, see my next comment on removed test class

#7272 (review)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you really insist then i can keep both, both the autoload fixture will have to be moved to a separate directory because the test cases were conflicting

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This true 2nd argument is on purpose for autoload existing Fixture directory

@samsonasik
Copy link
Member

Closing is you change existing config/test on purpose, please don't do that.

If you have specific use case to be skipped, see the differ on original PR and it configured like this :)

#6899

@samsonasik samsonasik closed this Sep 14, 2025
@rectorphp rectorphp locked as off-topic and limited conversation to collaborators Sep 14, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants