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

[dx] log rules registered in both withRules() and sets, keep them once to avoid duplications or invalid use #6761

Merged
merged 3 commits into from
Mar 3, 2025

Conversation

TomasVotruba
Copy link
Member

No description provided.

@samsonasik
Copy link
Member

samsonasik commented Feb 28, 2025

By this, this config will no longer working:

<?php

use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;

return RectorConfig::configure()
    // A. whole set
    ->withPreparedSets(typeDeclarations: true)
    // B. or few rules
    ->withRules([
        TypedPropertyFromAssignsRector::class
    ]);

for default demo page config https://getrector.com/demo

However, when withConfiguredRule() called, it MUST keep working, as override with configurable eg:

<?php

use Rector\Config\RectorConfig;
use Rector\TypeDeclaration\Rector\Property\TypedPropertyFromAssignsRector;

return RectorConfig::configure()
    // A. whole set
    ->withPreparedSets(typeDeclarations: true)
    // B. OVERRIDE RULE
    ->withConfiguredRule(TypedPropertyFromAssignsRector::class, [
        'inline_public' => true,
    ]);

ref https://getrector.com/demo/c74dab61-ff51-47fe-9391-f14bc34fc84a

this also needs test for configurable rule registered multiple times to keep working, eg: multiple config included with configurable rules.

@TomasVotruba
Copy link
Member Author

The first example is mutually exclusive. Either use A, or B. Not together.

For second use case, I think the configured rules are now skipped, as withRules() accepts only non-configured ones, right?

@samsonasik
Copy link
Member

samsonasik commented Feb 28, 2025

No..., withRules() accept configurable rule with default config, so when collect rules, instanceof ConfigurableRectorInterface should be skipped

@TomasVotruba
Copy link
Member Author

I've just tagged a new Rector release https://github.com/rectorphp/rector-src/releases/tag/2.0.10,
so we can test this in the wild separately 👍

Let's ship it :)

@TomasVotruba TomasVotruba merged commit 2bd14ca into main Mar 3, 2025
44 checks passed
@TomasVotruba TomasVotruba deleted the tv-dx-rules-2 branch March 3, 2025 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants