[DX] Add typeGuardedClasses() to guard against breaking method signature changes#8135
Merged
Merged
Conversation
TomasVotruba
force-pushed
the
tv-type-guarded-classes-feature
branch
from
5e89bca to
f8aa84f
Compare
TomasVotruba
changed the title
TomasVotruba
force-pushed
the
tv-type-guarded-classes-feature
branch
2 times, most recently
from
c3b0c00 to
959be9e
Compare
…method signature changes Introduces ->withTypeGuardedClasses([...]) / RectorConfig::typeGuardedClasses([...]). Listed classes and their non-final descendants are left untouched by type-declaration rules, since adding a return or param type to an extendable class is a breaking change for its child overrides. Final classes are never guarded (cannot be extended). Logic lives in ParentClassMethodTypeOverrideGuard::isTypeGuardedClass() and is applied at the shared choke points, covering the return and param rule families: - ClassMethodReturnTypeOverrideGuard::shouldSkipClassMethod() (return family) - ClassMethodParamVendorLockResolver::isVendorLocked() (param completer path) - AbstractParamTypeByMethodCallTypeRector::shouldSkipClassMethod() (by-method-call param family) - AddReturnTypeDeclarationRector / AddParamTypeDeclarationRector (configurable rules) When no classes are configured the guard short-circuits, so existing behaviour is unchanged.
TomasVotruba
force-pushed
the
tv-type-guarded-classes-feature
branch
from
959be9e to
e2c86fc
Compare
TomasVotruba
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
New config option to protect classes whose method signatures must not change, because doing so would break subclasses in other packages/apps:
Adding a return type or param type to a method is a breaking change for any child class that overrides it. When a class is listed here, type-declaration rules leave its method signatures untouched.
Rules
ClassReflection::is(). Rector can't enumerate children at runtime, but a descendant has the guarded ancestor, so upward resolution covers the whole subtree.finalclass can't be extended, so the change is always safe and still applied.How it is wired
The check lives in
ParentClassMethodTypeOverrideGuard::isTypeGuardedClass()and is applied at the shared choke points so it covers whole rule families, not just individual rules:ClassMethodReturnTypeOverrideGuard::shouldSkipClassMethod()ReturnTypeFromStrict*,Bool/Numeric/StringReturnTypeFrom*, ...)ClassMethodParamVendorLockResolver::isVendorLocked()AddReturnTypeDeclarationRector/AddParamTypeDeclarationRectorThis reuses the existing vendor-lock layer, which already answers "is it unsafe to change this method's type?" — guarded classes are simply one more reason it is unsafe. (The existing guards handle the upward case: don't diverge from a parent contract. This adds the downward case: don't break unknown children.)
Config API
RectorConfig::typeGuardedClasses(string[] $classes)RectorConfigBuilder::withTypeGuardedClasses(string[] $classes)Option::TYPE_GUARDED_CLASSESTest
rules-tests/TypeDeclaration/TypeGuardedClasses/— a non-final child of a guarded class is skipped; a final child still gets the return type added.