Skip to content

[Configuration] Skip deprecated rules from active set, warn only instead of crashing - #8358

Open
TomasVotruba wants to merge 1 commit into
mainfrom
fix-deprecated-rules-exit-code
Open

[Configuration] Skip deprecated rules from active set, warn only instead of crashing#8358
TomasVotruba wants to merge 1 commit into
mainfrom
fix-deprecated-rules-exit-code

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Fixes rectorphp/rector#9859

Problem

A registered deprecated rule crashes Rector with exit code 1 instead of printing a warning only.

Every DeprecatedInterface rule has a gutted refactor() that throws:

// rules/Transform/Rector/Class_/AddInterfaceByTraitRector.php
public function refactor(Node $node): ?Node
{
    throw new ShouldNotHappenException(sprintf(
        '"%s" rule is deprecated, as too niche; ...', self::class
    ));
}

DeprecatedRulesReporter prints the deprecation as a non-blocking warning, but the rule was still registered and run. As soon as it matched a node (AddInterfaceByTraitRector subscribes to every Class_), the throw fired and was caught as a SystemError -> exit code 1.

All 43 deprecated rules share this pattern, so any registered deprecated rule breaks the run.

Fix

ConfigurationRuleFilter::filter() now drops DeprecatedInterface rules from the active set unconditionally. They still warn via DeprecatedRulesReporter, but never run, so the throw guard is never reached.

Before

[WARNING] Registered rule "...AddInterfaceByTraitRector" is deprecated ...

[ERROR] "...AddInterfaceByTraitRector" rule is deprecated, as too niche; ...
        (exit code 1)

After

[WARNING] Registered rule "...AddInterfaceByTraitRector" is deprecated ...
        (exit code 0)

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.

Deprecations break rector

1 participant