Complete Facade Documenter correctness and drift prevention - #475
Merged
Conversation
The documenter treated PHPStan tags like ordinary PHPDoc, flattened useful parameter conditionals, wrapped top-level unions, stopped after the first lint mismatch, and rendered some string defaults incorrectly. This produced less precise facade metadata and made drift checks harder to fix in one pass. Prefer PHPStan parameter and return tags, preserve conditionals only when their targets can be represented accurately, and flatten unsupported cases to safe unions. Keep native nullability correct, use the parser to escape string defaults, report every lint mismatch, and remove an unreachable constant-array branch. Add regressions for conditional resolution, magic-method unions, string defaults, and aggregated lint failures. Subprocess tests now use the active PHP binary so they run against the same runtime as the suite.
The cache contract inherited seven PSR methods with string-only keys even though Hypervel repositories accept enums and, where supported, arrays. Static analysis therefore rejected calls that the framework handles correctly, and generated Cache metadata exposed a narrower API than the implementation. Redeclare the PSR methods with Hypervel's actual key types and precise iterable shapes. Point CacheManager's mixin at the concrete repository, matching the real facade target, and retain the key type on getMultiple's returned iterable. Regenerate the Cache facade from the corrected declarations so its method signatures now match the runtime API without changing runtime behavior.
Every filesystem implementation accepts an optional separator for prepend and append, but the contract omitted it. Valid calls through the contract failed static analysis even though the implementations handled them. Add the separator to the contract and shared fixture, describe its newline default, and give FilesystemManager the pooled filesystem mixin and accurate when and unless metadata. The Storage facade now documents methods common to every disk plus safe borrow-scoped pooled accessors while leaving driver-specific raw accessors on the resolved disk. Regenerate the Storage facade so the documented surface follows those rules and matches the types developers receive at runtime.
Bus batch identifiers are stored and returned as strings, but the repository contract, database implementation, and test fakes still accepted integers on several paths. That mismatch made the fake more permissive than production and exposed an API that the underlying storage does not provide. Narrow every batch identifier parameter to string across the contract, implementation, and fakes. Correct the PostgreSQL serialization test to use the same string identifier and retain its missing void return type. Regenerate the Bus facade so its batch methods expose the corrected identifier type consistently.
Many Support facade docblocks had drifted from their current targets. They retained redundant parentheses around unions, flattened conditional return types, or omitted methods added since their last generation. Regenerate every remaining affected facade with the corrected documenter. The output now keeps supported parameter conditionals, renders top-level unions consistently, preserves precise defaults and generics, and includes the current public methods without changing facade runtime behavior. Update the Redis metadata regression to assert the canonical unwrapped union emitted from its source declaration.
Facade docblocks can drift when their managers, contracts, or mixed-in implementations change. Individual package tests cover only parts of that generated surface, allowing stale metadata to remain unnoticed. Discover every concrete Support facade with reflection and run the documenter in lint mode against the complete set. Abstract facade bases are excluded by their actual class shape rather than a hard-coded filename list. This turns facade regeneration into a checked repository invariant while keeping the test independent of the current facade inventory.
Package facades depend on generated method metadata for editor support and static analysis, but the documentation did not explain how package authors should define, generate, or verify that surface. Add the canonical workflow to the package guide, including mixins, ignored methods, generation, and linting. Link the contribution guide to that workflow so repository checks are easy to find. Reduce the Facade Documenter README to package metadata and links, leaving the Hypervel documentation as the single source of user guidance.
Three timer tests used a one-millisecond interval and then asserted caller state immediately after scheduling. Under parallel load, the eagerly started child coroutine could reach its deadline before the caller resumed, so the callback legitimately ran before clear or coordinator resume was attempted. Use a long interval for the clear and closing cases so those actions determine the result without slowing the tests. Remove the racy pre-deadline assertion from the elapsed-timer case, which still proves the callback fires once and reports that the coordinator is open. The no-early-fire behavior remains covered by the long-interval closing case and the separate monotonic elapsed-time regression.
Keep fast feedback focused on the files and package being changed. Reserve the complete formatter, analysis, and test pipeline for meaningful checkpoints through composer fix.\n\nDocument how to recover from a failed combined check without rerunning expensive work that has already passed, while requiring agents to inspect the current Composer script instead of relying on remembered steps.\n\nUpdate the framework and porting workflows to use the same verification terminology.
Document the complete design for correcting Facade Documenter output and preventing generated metadata drift. Record the verified parser, reflection, import, filesystem, and Composer behavior behind the implementation. Define the exact rendering rules, failure contracts, test coverage, regeneration workflow, and deliberate limits so the work can be reviewed and maintained from the final design rather than its discovery history.
Track each test-owned top-level fixture path created beneath the disposable Testbench application. Remove only those paths during teardown and always continue through the parent lifecycle. Use the Filesystem boundary for fixture reads, writes, directory creation, and cleanup so repeated and parallel runs cannot inherit files from earlier tests.
Add explicit void return types to the remaining Facade Documenter test methods that needed no behavioral changes. This completes the package typing convention independently from the renderer corrections and keeps the behavioral commits focused on the contracts they protect.
Keep class-level method tags structured until output and render unions, intersections, arrays, nullability, templates, conditionals, relative names, constants, imports, dynamic parameters, and defaults without changing their PHPDoc meaning. Use bounded PHP tokenization for imports, canonical lexical class resolution, parser constant-expression nodes for defaults, and atomic mode-preserving filesystem replacement for generated source. Fail clearly when reflection cannot represent a truthful default or an ignore hook is declared incorrectly. Correct the Support data-accessor Stringable contract while preserving coercion of arbitrary PHP Stringable values. Regenerate the affected Support facades and add focused regressions for every corrected boundary, including line-layout-independent tags, precedence, ownership, import forms, publication, filtering, error behavior, and package dependencies.
Regenerate the Inertia facade against the corrected documenter. Preserve the current proxy union types, include the public state reset hook, and normalize callable and object ordering so the committed facade metadata matches the package source exactly.
Regenerate the Sentry facade from its current proxy signatures. Collapse the redundant mixed and void union on withScope while retaining the precise template-derived integration metadata produced by the corrected renderer.
Regenerate the Socialite facade from the current manager API. Use the existing global Closure import in the generated method signature so the facade remains stable under the repository formatter and accurately reflects its source.
Move facade metadata validation to the package that owns generation and discover production facades from Composer PSR-4 mappings. Filter candidates safely before autoloading, then use reflection to identify concrete facades. Run one lint pass across the complete inventory and parse every generated docblock so stale or syntactically invalid first-party metadata fails the normal test suite without a hardcoded list or separate workflow.
Bring root and split package requirements onto the same supported constraints and declare the external packages and extensions used by component packages. Require sockets where Engine uses socket constants, remove Boost autoload mappings for source directories that do not exist, and normalize repository support metadata for packages that were missing it. Add deterministic package sorting for Prompts and remove the completed Boost documentation porting ledger.
Add one repository-wide invariant for Composer metadata that can be checked uniformly across every split package. Require external dependency constraints to match the root manifest, Hypervel dependencies to be represented by self.version replacements, declared autoload paths to exist, and support metadata to match the repository. Remove duplicated root checks from package-specific tests while retaining their package-owned dependency, provider, extension, and discovery assertions.
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR improves Facade Documenter parsing, type resolution, default rendering, filesystem publication, linting, and validation. It also updates generated facade annotations, API contracts, Composer manifests, package metadata, documentation, tests, and verification guidance. ChangesFacade Documenter correctness
API contracts and facade annotations
Repository metadata and verification
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR substantially strengthens facade metadata generation and adds repository-wide drift checks.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/facade-documenter/facade.php | Reworks facade discovery, PHPDoc resolution, import tokenization, default rendering, lint accumulation, and atomic publication; no follow-up-eligible blocking defect was established. |
| tests/FacadeDocumenter/FacadeDocblocksTest.php | Adds complete first-party facade discovery plus lint and generated-docblock parsing checks. |
| tests/Composer/PackageManifestConsistencyTest.php | Adds repository-wide assertions for package dependency, replacement, autoload-path, and support-metadata consistency. |
| src/contracts/src/Cache/Repository.php | Broadens documented cache key forms to match supported enum and iterable inputs. |
| src/contracts/src/Filesystem/Filesystem.php | Aligns filesystem append and prepend contracts with the separator argument supported by implementations. |
| src/bus/src/BatchRepository.php | Standardizes batch identifiers as strings across the public repository contract. |
| src/support/src/Traits/InteractsWithData.php | Corrects Stringable return metadata while retaining broad native Stringable coercion inputs. |
| composer.json | Aligns root dependency and package metadata used by the new consistency checks. |
Reviews (3): Last reviewed commit: "facade-documenter: type rendered string ..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
tests/FacadeDocumenter/LintExitCodeTest.php (1)
204-204: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the documenter output to the exit-code assertion.
The other exit-code assertions in this file pass a diagnostic message. Lines 50, 54, 103, and 112 all do. This assertion does not. If the documenter fails for an unexpected reason, the failure report shows only the exit codes and hides the cause.
♻️ Proposed change
- $this->assertSame(1, $lint->getExitCode()); + $this->assertSame(1, $lint->getExitCode(), $lint->getErrorOutput() . $lint->getOutput());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/LintExitCodeTest.php` at line 204, Update the exit-code assertion in the relevant test method to pass the documenter output as its diagnostic message, matching the existing assertions in this file and preserving the expected exit code of 1.tests/FacadeDocumenter/PhpstanTagResolutionTest.php (1)
233-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDecide whether generated unions must normalize
object|mixedtomixed.The renderer currently produces
object|mixedfrom broadenedobject : mixedbranches and existing generatedAppfacade annotations. If this output is intentional, the current tests and metadata are consistent. If the style should collapse tomixed, update the renderer and the related expectations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/PhpstanTagResolutionTest.php` around lines 233 - 234, Decide the intended normalization for generated union types containing mixed, then apply it consistently in the renderer responsible for facade annotations and update the related PhpstanTagResolutionTest expectations and metadata. Preserve object|mixed if that output is intentional; otherwise collapse it to mixed wherever these broadened branches are rendered.tests/FacadeDocumenter/ConstFetchResolutionTest.php (1)
234-235: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a title docblock to the new test method.
The neighbouring test at Line 312 has a title docblock. This test does not. Add one for consistency with the repository docblock convention.
♻️ Proposed docblock
+ /** + * Prefer lexical constant owners and keep explicit global class names. + */ public function testConstantOwnersPreferLexicalClassesAndPreserveExplicitGlobalNames(): voidAs per coding guidelines: "Add method title docblocks in imperative Laravel style; add class docblocks only when unusual complexity needs explanation."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/ConstFetchResolutionTest.php` around lines 234 - 235, Add an imperative Laravel-style title docblock immediately above testConstantOwnersPreferLexicalClassesAndPreserveExplicitGlobalNames(), matching the neighboring test’s repository convention.Source: Coding guidelines
tests/FacadeDocumenter/TypePrecedenceTest.php (1)
222-224: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReport every mismatch instead of failing on the first.
This loop asserts 18 rendering rules. If the renderer regresses on several rules, only the first mismatch appears. Collect the missing methods and assert once, so one run shows every regression.
♻️ Proposed change
- foreach ($expectedMethods as $expectedMethod) { - $this->assertStringContainsString($expectedMethod, $contents); - } + $missingMethods = array_values(array_filter( + $expectedMethods, + fn (string $expectedMethod): bool => ! str_contains($contents, $expectedMethod), + )); + + $this->assertSame([], $missingMethods, 'Generated facade is missing expected `@method` lines.');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/TypePrecedenceTest.php` around lines 222 - 224, Update the assertion loop in the TypePrecedence test to collect every expected method absent from contents, then perform one aggregate assertion after the loop. Preserve the existing 18 rendering-rule checks while reporting all mismatches instead of stopping at the first failure.tests/FacadeDocumenter/PackageMetadataTest.php (1)
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse the same repository-root helper as the sibling test.
tests/FacadeDocumenter/FacadeDocblocksTest.phpresolves the repository root withdirname(__DIR__, 2). This test uses a/../../relative path. Use one form in both files.♻️ Proposed change
$composer = json_decode( - file_get_contents(__DIR__ . '/../../src/facade-documenter/composer.json'), + file_get_contents(dirname(__DIR__, 2) . '/src/facade-documenter/composer.json'), true, 512, JSON_THROW_ON_ERROR, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/PackageMetadataTest.php` around lines 19 - 24, Update the composer.json path in the test’s JSON-loading setup to resolve the repository root with the same dirname(__DIR__, 2) helper used by FacadeDocblocksTest.php, keeping the existing file_get_contents and json_decode behavior unchanged.tests/FacadeDocumenter/DefaultValueTest.php (2)
247-247: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
assertSame(1, preg_match(...))does not assert a single docblock. Both tests usepreg_matchto capture the generated docblock and compare the result to1.preg_matchstops after the first match and returns1, so the assertion proves only that a docblock exists. If the intent is to prove the generator emits exactly one docblock, usepreg_match_all.
tests/FacadeDocumenter/DefaultValueTest.php#L247-L247: replacepreg_matchwithpreg_match_alland read the first set from the match array before parsing.tests/FacadeDocumenter/TypePrecedenceTest.php#L226-L226: apply the same replacement, then assign$docCommentfrom the first match set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/DefaultValueTest.php` at line 247, Update the docblock extraction assertions in tests/FacadeDocumenter/DefaultValueTest.php at lines 247-247 and tests/FacadeDocumenter/TypePrecedenceTest.php at lines 226-226 to use preg_match_all, assert that exactly one match is found, and assign the docComment from the first match set before parsing; preserve the existing parsing and validation behavior.
235-237: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueRender
-INFwith the matching PHPDoc-style constant form.
INFandNANrender as named constants, but-INFrenders as-1.0E+999because the renderer intentionally chooses an equivalent float literal. This produces the same value, but keeps the default expression form inconsistent with the other infinite constants. Update the renderer to emit the matching negative infinite form.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/FacadeDocumenter/DefaultValueTest.php` around lines 235 - 237, Update the default-value renderer used by DefaultValueTest so negative infinity is emitted as the PHPDoc-style expression -INF instead of the equivalent -1.0E+999 literal. Preserve the existing named-constant rendering for INF and NAN.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/support/src/Facades/Redis.php`:
- Line 53: Update the Redis facade annotations for lrem(), keys(), and lInsert()
to match phpredis: order lrem() parameters as key, value, count with count
optional; declare keys() as returning matched keys; and declare lInsert() as
returning the insertion result/list size or failure value instead of void.
In `@tests/FacadeDocumenter/FacadeDocumenterTestCase.php`:
- Around line 31-58: Update writeAppFile() to place fixtures under a
worker-unique root beneath APP_PATH, using the platform-appropriate process ID,
and record that root for cleanup instead of the first relative-path segment.
Adjust tearDown() to delete only each test’s recorded unique fixture directory
while preserving parent::tearDown().
---
Nitpick comments:
In `@tests/FacadeDocumenter/ConstFetchResolutionTest.php`:
- Around line 234-235: Add an imperative Laravel-style title docblock
immediately above
testConstantOwnersPreferLexicalClassesAndPreserveExplicitGlobalNames(), matching
the neighboring test’s repository convention.
In `@tests/FacadeDocumenter/DefaultValueTest.php`:
- Line 247: Update the docblock extraction assertions in
tests/FacadeDocumenter/DefaultValueTest.php at lines 247-247 and
tests/FacadeDocumenter/TypePrecedenceTest.php at lines 226-226 to use
preg_match_all, assert that exactly one match is found, and assign the
docComment from the first match set before parsing; preserve the existing
parsing and validation behavior.
- Around line 235-237: Update the default-value renderer used by
DefaultValueTest so negative infinity is emitted as the PHPDoc-style expression
-INF instead of the equivalent -1.0E+999 literal. Preserve the existing
named-constant rendering for INF and NAN.
In `@tests/FacadeDocumenter/LintExitCodeTest.php`:
- Line 204: Update the exit-code assertion in the relevant test method to pass
the documenter output as its diagnostic message, matching the existing
assertions in this file and preserving the expected exit code of 1.
In `@tests/FacadeDocumenter/PackageMetadataTest.php`:
- Around line 19-24: Update the composer.json path in the test’s JSON-loading
setup to resolve the repository root with the same dirname(__DIR__, 2) helper
used by FacadeDocblocksTest.php, keeping the existing file_get_contents and
json_decode behavior unchanged.
In `@tests/FacadeDocumenter/PhpstanTagResolutionTest.php`:
- Around line 233-234: Decide the intended normalization for generated union
types containing mixed, then apply it consistently in the renderer responsible
for facade annotations and update the related PhpstanTagResolutionTest
expectations and metadata. Preserve object|mixed if that output is intentional;
otherwise collapse it to mixed wherever these broadened branches are rendered.
In `@tests/FacadeDocumenter/TypePrecedenceTest.php`:
- Around line 222-224: Update the assertion loop in the TypePrecedence test to
collect every expected method absent from contents, then perform one aggregate
assertion after the loop. Preserve the existing 18 rendering-rule checks while
reporting all mismatches instead of stopping at the first failure.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6b4bd37-e220-41a4-bc8f-ecf0be67c864
📒 Files selected for processing (97)
AGENTS.mdcomposer.jsondocs/plans/2026-08-05-2103-facade-documenter-complete-correctness-and-drift-prevention.mdsrc/boost/composer.jsonsrc/boost/docs-ported.mdsrc/boost/docs/contributions.mdsrc/boost/docs/filesystem.mdsrc/boost/docs/packages.mdsrc/bus/src/BatchRepository.phpsrc/bus/src/DatabaseBatchRepository.phpsrc/cache/src/CacheManager.phpsrc/cache/src/Repository.phpsrc/console/composer.jsonsrc/contracts/src/Cache/Repository.phpsrc/contracts/src/Filesystem/Filesystem.phpsrc/database/composer.jsonsrc/engine/composer.jsonsrc/facade-documenter/README.mdsrc/facade-documenter/composer.jsonsrc/facade-documenter/facade.phpsrc/filesystem/src/FilesystemManager.phpsrc/fortify/composer.jsonsrc/inertia/src/Inertia.phpsrc/passkeys/composer.jsonsrc/prompts/composer.jsonsrc/sentry/src/Facade.phpsrc/socialite/src/Socialite.phpsrc/support/src/Facades/App.phpsrc/support/src/Facades/Artisan.phpsrc/support/src/Facades/Bus.phpsrc/support/src/Facades/Cache.phpsrc/support/src/Facades/Config.phpsrc/support/src/Facades/Context.phpsrc/support/src/Facades/Cookie.phpsrc/support/src/Facades/DB.phpsrc/support/src/Facades/Date.phpsrc/support/src/Facades/Event.phpsrc/support/src/Facades/Exceptions.phpsrc/support/src/Facades/File.phpsrc/support/src/Facades/Gate.phpsrc/support/src/Facades/Grpc.phpsrc/support/src/Facades/Http.phpsrc/support/src/Facades/Lang.phpsrc/support/src/Facades/Log.phpsrc/support/src/Facades/Pipeline.phpsrc/support/src/Facades/Process.phpsrc/support/src/Facades/Redis.phpsrc/support/src/Facades/Request.phpsrc/support/src/Facades/Response.phpsrc/support/src/Facades/Route.phpsrc/support/src/Facades/Schedule.phpsrc/support/src/Facades/Session.phpsrc/support/src/Facades/Storage.phpsrc/support/src/Facades/View.phpsrc/support/src/Facades/Vite.phpsrc/support/src/Testing/Fakes/BatchRepositoryFake.phpsrc/support/src/Testing/Fakes/BusFake.phpsrc/support/src/Traits/InteractsWithData.phpsrc/validation/composer.jsontests/Auth/PackageMetadataTest.phptests/Broadcasting/PackageMetadataTest.phptests/Bus/BusBatchTest.phptests/Cache/Fixtures/ArrayFilesystem.phptests/Composer/PackageManifestConsistencyTest.phptests/Coordinator/TimerTest.phptests/FacadeDocumenter/CaseInsensitiveDedupeTest.phptests/FacadeDocumenter/ClassDocblockMethodFilteringTest.phptests/FacadeDocumenter/ClassDocblockResolutionTest.phptests/FacadeDocumenter/ConditionalDedupeTest.phptests/FacadeDocumenter/ConstFetchResolutionTest.phptests/FacadeDocumenter/DefaultValueTest.phptests/FacadeDocumenter/DocTagParsingTest.phptests/FacadeDocumenter/DocblockNativeNullabilityMergeTest.phptests/FacadeDocumenter/DynamicParameterTest.phptests/FacadeDocumenter/FacadeDocblocksTest.phptests/FacadeDocumenter/FacadeDocumenterTestCase.phptests/FacadeDocumenter/FilePublicationTest.phptests/FacadeDocumenter/GenericPreservationTest.phptests/FacadeDocumenter/GracefulDegradationTest.phptests/FacadeDocumenter/IdempotenceTest.phptests/FacadeDocumenter/IgnoredMethodsTest.phptests/FacadeDocumenter/ImportResolutionTest.phptests/FacadeDocumenter/LintExitCodeTest.phptests/FacadeDocumenter/MixedNativeNullableTest.phptests/FacadeDocumenter/NullableSelfStaticTest.phptests/FacadeDocumenter/PackageMetadataTest.phptests/FacadeDocumenter/PhpstanTagResolutionTest.phptests/FacadeDocumenter/RelativeTypeResolutionTest.phptests/FacadeDocumenter/StaticPrefixGuardTest.phptests/FacadeDocumenter/TraitImportSourceTest.phptests/FacadeDocumenter/TypePrecedenceTest.phptests/FacadeDocumenter/WrapperCollapseTest.phptests/Http/PackageMetadataTest.phptests/Mail/PackageMetadataTest.phptests/Notifications/PackageMetadataTest.phptests/Redis/PackageMetadataTest.phptests/Support/Traits/InteractsWithDataTest.php
💤 Files with no reviewable changes (8)
- tests/Broadcasting/PackageMetadataTest.php
- tests/Http/PackageMetadataTest.php
- src/boost/docs-ported.md
- tests/Mail/PackageMetadataTest.php
- tests/Auth/PackageMetadataTest.php
- tests/FacadeDocumenter/CaseInsensitiveDedupeTest.php
- tests/Notifications/PackageMetadataTest.php
- tests/FacadeDocumenter/NullableSelfStaticTest.php
Collapse flattened conditional branches when an exact mixed member makes the remaining union redundant. Regenerate the App facade so its conditional container methods advertise the canonical mixed result. Correct RedisConnection metadata for keys and lInsert, remove pooled subscription commands that always throw, and regenerate the Redis facade from the owning annotations. Keep the working manager-level subscription API unchanged. Add focused source and generated metadata regressions, improve subprocess failure diagnostics, document the lexical constant regression, and keep the implementation plan aligned with the final contracts and generated output.
Collaborator
Author
|
@coderabbitai Follow-up is pushed in 1c97921. Implemented:
Kept unchanged after verification:
The focused Facade Documenter and Redis suites pass, all generated facades lint cleanly, and the final diff is whitespace-clean. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline --decorate --no-renames 1c9792157
printf '\n--- changed paths ---\n'
git diff-tree --no-commit-id --name-only -r 1c9792157
printf '\n--- relevant diff excerpts ---\n'
git show --format= --no-ext-diff --unified=40 1c9792157 -- \
src/facade-documenter/facade.php \
src/database/src/RedisConnection.php \
src/support/src/Facades/App.php \
src/support/src/Facades/Redis.php \
tests/FacadeDocumenter \
tests/Redis \
| sed -n '1,1800p'Length of output: 32975 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- RedisConnection follow-up diff ---'
git show --format= --no-ext-diff --unified=30 1c9792157 -- src/redis/src/RedisConnection.php
printf '%s\n' '--- Redis facade guard hook and related generated metadata ---'
rg -n -C 8 \
'ignoredFacadeDocumenterMethods|`@method` static .* (keys|lInsert|subscribe|psubscribe|ssubscribe)\(' \
src/support/src/Facades/Redis.php src/redis/src/RedisConnection.php
printf '%s\n' '--- exact conditional regression context ---'
rg -n -C 12 \
'BroadenedGenericConditional|BroadenedBareConditional|array<.*mixed|mixed>' \
tests/FacadeDocumenter/PhpstanTagResolutionTest.phpLength of output: 17744 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '317,375p' src/support/src/Facades/Redis.phpLength of output: 1156
The retained decisions are consistent with the stated contracts. I did not execute the test suites in this inspection environment. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/facade-documenter/facade.php`:
- Around line 896-909: Update flattenConditionalBranches to declare both $if and
$else as string parameters and declare its return type as string, preserving the
existing implementation and PHPDoc contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ba9eee2c-649d-4b89-a38b-5d5d911eced6
📒 Files selected for processing (9)
docs/plans/2026-08-05-2103-facade-documenter-complete-correctness-and-drift-prevention.mdsrc/facade-documenter/facade.phpsrc/redis/src/RedisConnection.phpsrc/support/src/Facades/App.phpsrc/support/src/Facades/Redis.phptests/FacadeDocumenter/ConstFetchResolutionTest.phptests/FacadeDocumenter/LintExitCodeTest.phptests/FacadeDocumenter/PhpstanTagResolutionTest.phptests/Redis/PackageMetadataTest.php
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/FacadeDocumenter/ConstFetchResolutionTest.php
- tests/FacadeDocumenter/PhpstanTagResolutionTest.php
- tests/FacadeDocumenter/LintExitCodeTest.php
- src/support/src/Facades/App.php
- src/support/src/Facades/Redis.php
- docs/plans/2026-08-05-2103-facade-documenter-complete-correctness-and-drift-prevention.md
Declare the exact native contracts for conditional branch flattening and docblock/native nullability merging. Both helpers operate only on rendered type strings, while nested resolution either returns a string or propagates its failure. Remove redundant PHPDoc type annotations while retaining the nullability merge explanation that cannot be expressed by the signature. Keep AST and reflection helpers outside this bounded typing change. Record the complete rendered-string helper boundary in the implementation plan so the source and design remain aligned.
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.
Summary
Facade docblocks are generated API metadata used by editors and static analysis. The previous documenter could emit valid-looking but incorrect types, lose useful PHPDoc information, miss standard import forms, and leave stale metadata outside the Support package unnoticed.
This change makes Facade Documenter a reliable source of truth for every first-party facade. It also corrects the framework contracts that were producing inaccurate metadata and adds repository checks for package metadata that should remain uniform.
Facade Documenter
@methodtags as parser nodes until final rendering instead of recovering method identity from formatted strings.self,parent,static, constants, imported aliases, qualified names, and same-namespace classes using PHP lexical rules.The implementation remains a short-lived procedural CLI. It does not add application, coroutine, worker, cache, registry, or request-time machinery.
Framework contracts and generated facades
The audit exposed several source contracts that did not describe behavior already supported by their implementations:
prependandappendimplementation, and pooled filesystem metadata reflects the safe shared surface.Hypervel\Support\Stringablewhile enum coercion continues to accept any PHPStringableimplementation.All affected Support facades were regenerated. Inertia, Sentry, and Socialite metadata was also refreshed against their current proxy APIs.
Drift prevention
A normal PHPUnit test now discovers concrete first-party facades from the root Composer PSR-4 mappings. It runs one lint process across the complete inventory and parses every generated docblock with phpdoc-parser.
This replaces directory-specific shell discovery and catches both stale output and invalid generated tags without a hardcoded facade list or an additional CI workflow.
Focused regressions cover type precedence, templates, relative ownership, constants, imports, defaults, class-level methods, ignored methods, tag layouts, dynamic parameters, file publication, lint behavior, and deterministic fixture cleanup.
Composer metadata
A repository-wide invariant now checks the structures that should be uniform across split packages:
self.versionreplacements;The existing package metadata was aligned with those rules. This includes direct extension and package requirements, valid Boost mappings, current dependency constraints, and missing support metadata. Duplicated root checks were removed from package-specific tests while their package-owned assertions remain.
Documentation and maintenance
composer fixpipeline runs once at meaningful boundaries.Verification
composer fixpasses, including formatting, both PHPStan configurations, the parallel framework suite, Testbench, and package dogfood. The Facade Documenter package suite, all-facade lint and parse checks, Composer metadata checks, and affected package tests also pass after merging the latest0.4.Summary by CodeRabbit
New Features
Bug Fixes
Documentation