Skip to content

Builder demolition + Info Type field removal - #529

Merged
Firehed merged 5 commits into
mainfrom
519/builder-demolition
Sep 11, 2026
Merged

Builder demolition + Info Type field removal#529
Firehed merged 5 commits into
mainfrom
519/builder-demolition

Conversation

@Firehed

@Firehed Firehed commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Continuation of the rescoped #519 migration: demolish the source-context
builders. #527 shipped the reader-side seam and consumer migration;
this PR unpicks the class-info builders.

What lands

  • Delete Repository\ClassInfoFactory interface. One-implementation
    interface on the deletion list.
  • Fold DefaultClassInfoFactory::fromReflection into
    ReflectionSymbolInfoFactory as private helpers;
    ParameterInfo::fromReflection inlined at its one call site.
  • Fold DefaultClassInfoFactory::fromAstNode into
    DeclarationSymbolInfoFactory; FunctionInfo::fromNode,
    ConstantInfo::fromGlobalDeclaration, and ParameterInfo::fromNode
    inlined at their one call sites. DefaultClassInfoFactory and its
    test delete.
  • Inline ReflectionSymbolInfoFactory into BuiltinBackend. The
    standalone class and its wiring at KnowledgeStack::forProject go
    away — source-picking stays inside the backend, invisible above the
    SymbolBackend interface. Its unit coverage moves into
    BuiltinBackendTest, exercised through the backend's lookup
    surface.

DeclarationSymbolInfoFactory keeps its top-level shape for now: two
consumers (FilesystemBackend and DocumentSymbolSink) both give it
AST-derived input, so it's a shared processor rather than source-picking.
Its name still reads source-shaped; a rename or further burying is a
judgment call that Task B's shrinkage naturally reopens.

Baseline shrinks from 185 → 131.

Follow-up (separate PR)

Task B — drop eager ?Type fields from Info, remove
ResolvedSymbol::getType() / ResolvedCallable::getReturnType()
interface obligations, rewrite NativeTypeSource internals to walk
source, migrate the remaining ->type/->returnType readers, drain
remaining Category 1 baseline entries, trim allowlists.

Closes #519 when merged together with the follow-up.

Body drafted by AI; reviewed by human.

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.44%. Comparing base (55e7e3e) to head (77eb641).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #529      +/-   ##
============================================
- Coverage     99.46%   99.44%   -0.02%     
- Complexity     1919     1924       +5     
============================================
  Files           136      134       -2     
  Lines          5031     5071      +40     
============================================
+ Hits           5004     5043      +39     
- Misses           27       28       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@Firehed
Firehed force-pushed the 519/builder-demolition branch from 7253459 to 6c38701 Compare September 11, 2026 05:21
Firehed added a commit that referenced this pull request Sep 11, 2026
Reader-side of the rescoped #519 migration. See
#519 (comment)
for the full plan; the builder demolition and Info Type field removal
land on #529 stacked on top of this.

## What lands

- **`TypeSource` interface at `Resolution\TypeSource\`.** Lateral move
  from Domain: the interface's implementations need `SymbolSource` and
  `MemberResolver`, so it can't live in Domain (no ruleset edge to
  Knowledge/Repository).
- **`NativeTypeSource` implements `TypeSource`.** Class members walk
  the inheritance graph through `MemberResolver`; functions and global
  constants go through `SymbolSource`. Transitional: still reads eager
  `Info` Type fields until #529 drops them and switches to a source
  walk.
- **Wired at `Server::forProject`** and threaded through
  `SymbolResolver` into `ExpressionResolver` + `MemberAccessDetector`.
- **PHPStan allowlists extended** for `MemberResolver::findMethod` /
  `findProperty` and `SymbolSource::lookupConstant` to name
  `NativeTypeSource` as the identity-keyed type authority.

## Consumers migrated

- `ExpressionResolver::resolveLateBoundReturn` reads the late-bound
  return type through `TypeSource::forMethodReturn` (queried by the
  calling class, so trait aliases still resolve).
- `ExpressionResolver::typeOfBinding` Param branch and
  `SymbolResolver::resolveParameter` both dispatch through a shared
  `ParameterTyping` helper: function/method scopes route through
  `TypeSource`; closure and arrow scopes fall through to
  `TypeFactory::fromNode` (the explicit deferral #517 called out for
  positional/AST-only cases).

## Interface shape

`TypeSource::forConstant(ConstantName, ?ClassName)` split into
`forClassConstant(ClassName, ConstantName)` and
`forGlobalConstant(GlobalConstantName)`. The old signature was
stringly-typed for the global case; the split matches how
`SymbolSource` already models class vs global lookups.

Baseline shrunk by one.

Closes #519 when #529 merges after.

Body drafted by AI; reviewed by human.

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Base automatically changed from 519/native-type-source to main September 11, 2026 05:40
Firehed and others added 4 commits September 10, 2026 22:45
The interface had one implementation and its two methods were already
deprecated. Its consumers, DeclarationSymbolInfoFactory and
ReflectionSymbolInfoFactory, hold the concrete DefaultClassInfoFactory
directly - a temporary shape while the factory itself is on the
deletion list; folding its logic into those two consumers is the
follow-up commits.

Rewrites the two matching baseline entries from interface-method to
class-method identifiers (identifier swap, count unchanged). The
allowlist path in phpstan.neon for the deleted file drops - a Tighten.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Every `*FromReflection` helper on `DefaultClassInfoFactory` now lives on
`ReflectionSymbolInfoFactory` as a private method, and the `fromReflection`
body is inlined. `ParameterInfo::fromReflection` is inlined at the one call
site so the deprecated shortcut leaves the factory. Tests that covered
reflection-side extraction move alongside the code; the internal-only gate
means they exercise built-in classes now (`ArrayObject`, `Attribute`,
`Random\IntervalBoundary`, ...).

Baseline drops from 186 to 171.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…ationSymbolInfoFactory

Every AST-side helper on `DefaultClassInfoFactory::fromAstNode` now lives on
`DeclarationSymbolInfoFactory` as a private method. `FunctionInfo::fromNode`,
`ConstantInfo::fromGlobalDeclaration`, and `ParameterInfo::fromNode` are
inlined at their one call site each, so the deprecated shortcuts leave the
factory.

The `DefaultClassInfoFactory $classes` dependency drops from the sink and
reflection factories, and the composition root passes nothing. The Repository
layer's on-disk class-info factory is gone; only `MemberResolver` remains
under `src/Repository/`.

`AstTextAgreementTest` routes its class-like description through the new
factory's public API: scan the tree with `DeclarationScanner`, then hand the
declarations to `DeclarationSymbolInfoFactory::fromDeclarations`.

Baseline drops from 186 to 131.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ReflectionSymbolInfoFactory had one production caller, BuiltinBackend,
and its role name read as source-picking sitting outside the composite
that already picks a source. Fold every method onto BuiltinBackend as a
private helper; the class file and the composition-root wiring go away,
so no test or caller outside the composite names a per-source builder
any more.

BuiltinBackend picks up the disallowed-function and reflection-namespace
allowlist entries, and the KindBranchRule allowlist row. The unit tests
for the deleted class move to BuiltinBackendTest, exercised through the
backend's own lookup surface. The parity test drops the constructor
argument.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Firehed
Firehed force-pushed the 519/builder-demolition branch from 6c38701 to 2e363bc Compare September 11, 2026 05:47
Eliminate branches that PHP's built-in surface cannot reach, and add
real tests where a branch is reachable but was uncovered.

BuiltinBackend: drop the Trait_ kind branch, the extractTraits helper,
the isEnumCase filter in extractEnumCases, and the constant-visibility
mapper - no built-in trait or non-public class constant exists to route
into any of them. DeclarationSymbolInfoFactory::resolveNameToClassName
loses its ternary: TreeAnnotator's NameResolver replaces class-context
Name nodes in place, so toString() already reads the FQN.

Add coverage tests for the branches that are reachable: trait use
adaptations (insteadof, aliases with and without a source trait), the
malformed-parameter guards (via a fixture the parser recovers into
Error nodes), reflection's Private/Protected mappers (Exception,
SplHeap), inherited constant filtering, the null-default formatter,
and the three ExpressionResolver::resolveNew fallback branches
(variable class, anonymous class, unresolvable name, unknown class).

Regenerate the phpstan baseline (shrunk 185 to 130) and recapture the
completion-broken-first-open golden for the new IncompleteCode fixture.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@Firehed
Firehed marked this pull request as ready for review September 11, 2026 18:30
@Firehed
Firehed merged commit 7d052e3 into main Sep 11, 2026
8 of 9 checks passed
@Firehed
Firehed deleted the 519/builder-demolition branch September 11, 2026 18:31
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.

NativeTypeSource + consumer migration

1 participant