Skip to content

[removal] Replace ForbiddenNodeRule with dedicated ForbiddenTraitRule and ForbiddenSwitchRule - #342

Merged
TomasVotruba merged 1 commit into
mainfrom
forbidden-trait-switch-rules
Oct 9, 2026
Merged

TomasVotruba merged 1 commit into
mainfrom
forbidden-trait-switch-rules

Conversation

@TomasVotruba

Copy link
Copy Markdown
Member

Why

ForbiddenNodeRule was a single configurable rule banning 8 node types (traits, switch, empty(), @ suppression, string interpolation, post inc/dec) under one shared identifier symplify.forbiddenNode. Two problems:

  • No granular opt-out - all 8 bans share one identifier, so ignoreErrors cannot silence a single node type.
  • Coarse toggle - symplify.configurable: false also kills unrelated configurable rules.

It also hid too much behind one opaque message with no reason or alternative.

What

  • Remove ForbiddenNodeRule and its generic node list.
  • Add dedicated ForbiddenTraitRule and ForbiddenSwitchRule, each with its own identifier (symplify.forbiddenTrait, symplify.forbiddenSwitch) and a message that names the alternative.
  • Wire both into configurable-rules.neon (toggled by symplify.configurable) and rector-rules.neon.
  • The other 6 node bans are dropped.

Checks

ECS, PHPStan, Rector and PHPUnit all green.

@TomasVotruba TomasVotruba changed the title Replace ForbiddenNodeRule with dedicated ForbiddenTraitRule and ForbiddenSwitchRule [depre] Replace ForbiddenNodeRule with dedicated ForbiddenTraitRule and ForbiddenSwitchRule Oct 9, 2026
@TomasVotruba
TomasVotruba merged commit d63ef08 into main Oct 9, 2026
9 checks passed
@TomasVotruba
TomasVotruba deleted the forbidden-trait-switch-rules branch October 9, 2026 09:10
@TomasVotruba TomasVotruba changed the title [depre] Replace ForbiddenNodeRule with dedicated ForbiddenTraitRule and ForbiddenSwitchRule [removal] Replace ForbiddenNodeRule with dedicated ForbiddenTraitRule and ForbiddenSwitchRule Oct 9, 2026
@staabm

staabm commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

This PR immediately broke our builds on release, as its a BC break hidden in a minor release

@staabm

staabm commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

the projects readme promotes use of single rules by documentation like

rules:
    - Symplify\PHPStanRules\Rules\CheckRequiredInterfaceInContractNamespaceRule

so I think removing rules should not happen in non-major releases

@TomasVotruba

Copy link
Copy Markdown
Member Author

Let me patch it. Use the old tag for now.

How do you use it?

@staabm

staabm commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

in the relevant case we are using a configurable rule like

        class: Symplify\PHPStanRules\Rules\ForbiddenNodeRule
        tags: [phpstan.rules.rule]
        arguments:
            forbiddenNodes:
                - PhpParser\Node\Expr\Exit_

thank you

@TomasVotruba

Copy link
Copy Markdown
Member Author

Getting on subway, just a sec...

@TomasVotruba

TomasVotruba commented Oct 9, 2026 •

Copy link
Copy Markdown
Member Author

Fixed: #346

Taggec 14.18.1

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