audit/2 api conventions - #1027
Draft
mglaman wants to merge 3 commits into
Draft
Conversation
mglaman
force-pushed
the
audit/2-api-conventions
branch
from
August 5, 2026 16:20
11c49d2 to
e8cd1dd
Compare
mglaman
changed the base branch from
audit/1-correctness-fixes
to
audit/1d-autoloader-cleanup
August 5, 2026 16:44
mglaman
force-pushed
the
audit/2-api-conventions
branch
from
August 5, 2026 16:44
e8cd1dd to
99903cc
Compare
mglaman
force-pushed
the
audit/2-api-conventions
branch
from
August 5, 2026 17:15
99903cc to
40887b7
Compare
mglaman
force-pushed
the
audit/2-api-conventions
branch
from
August 5, 2026 19:13
40887b7 to
6ae8385
Compare
Return null to fall back to the declared return type instead of eagerly computing it via ParametersAcceptorSelector, resolve every constant string in a union instead of only the first, and drop the Concat and MethodCall AST sniffing that the constant-strings check already subsumes. Covers getStorage($cond ? 'node' : 'user') with a proper union type now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
InClassNode hands the rule its ClassReflection directly, which drops the ReflectionProvider constructor dependency, the anonymous-class namespacedName juggling, and the manual FQN round-trip through ObjectType. Behavior is unchanged; the rule now simply never fires for classes PHPStan cannot reflect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- GlobalDrupalDependencyInjectionRule: resolve the called class through $scope->resolveName() instead of requiring a FullyQualified node, and type-check against the MethodReflection interface instead of ExtendedMethodReflection, which is marked api-do-not-implement. - EntityFieldReflection: guard ReflectionProvider::getClass() with hasClass() so a missing Drupal interface degrades instead of throwing. - EntityFieldsViaMagicReflectionExtension: check interfaces via ClassReflection::implementsInterface() instead of rebuilding an ObjectType from the class name, which discarded generics and allocated on every property lookup; drop the now-unused static helpers. - FieldItemListPropertyReflection: build nullable types with TypeCombinator::addNull() instead of new UnionType. - Drop ->line($node->getStartLine()) calls that restate the default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
mglaman
force-pushed
the
audit/2-api-conventions
branch
from
August 5, 2026 19:33
6ae8385 to
1c6b5bd
Compare
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.
Part 6 of 9 in the legacy-code audit stack (on top of #1036). Brings the oldest type extension, rules, and reflection classes in line with current PHPStan 2.x API conventions and the patterns the newer code in this repo already uses.
What changed
EntityTypeManagerGetStorageDynamicReturnTypeExtensionnow declares?Typeand returnsnullto fall back, computes the declared return type lazily instead of eagerly on every call, and resolves every constant string in a union —getStorage($cond ? 'node' : 'user')now infersNodeStorage|UserStorageinstead of justNodeStorage. TheConcat/MethodCallAST sniffing is gone;Scope::getType()already folds constant expressions. This file is the template for migrating the remaining eager-fallback extensions in a follow-up.PluginManagerInspectionRulemigrates fromNode\Stmt\Class_toInClassNode, which provides theClassReflectiondirectly. TheReflectionProviderconstructor dependency, anonymous-class handling, and manual FQN round-trip all disappear. Behavior is unchanged.GlobalDrupalDependencyInjectionRuleresolves the called class via$scope->resolveName()and checks against theMethodReflectioninterface instead ofExtendedMethodReflection, which PHPStan marks@api-do-not-implement.EntityFieldReflectionguardsReflectionProvider::getClass()withhasClass();EntityFieldsViaMagicReflectionExtensionusesimplementsInterface()instead of rebuilding anObjectTypefrom the class name on every property lookup;FieldItemListPropertyReflectionbuilds nullable types withTypeCombinator::addNull().->line($node->getStartLine())calls removed — that is already the default.Testing
New type-inference fixtures cover the first-class callable (must not crash), union constant-string arguments, and the dynamic-ID fallback. Full suite, self-analysis, and phpcs are green.
🤖 Generated with Claude Code