Repository navigation
Add PHPStan rules for Hypervel's long-lived workers - #105
binaryfire wants to merge 22 commits into
Conversation
Include optional Unsupported guidance in the main diagnostic so terminal and CI formatters retain it. Specify reflection-based argument reading and focused coverage for bare markers and supplied messages, and update the documentation requirements.
Expose resolvedShared() on the container contract so facades can distinguish worker-held instances from fresh resolutions. Retain worker caching for shared roots, keep fresh roots per coroutine, and leave scoped and uncached facades resolving through the container. Generation changes invalidate cached coroutine roots across live contexts, while copied child contexts resolve their own roots. Register the date factory during foundation bootstrap without replacing earlier bindings. Standalone Date facade fallback no longer writes a container binding. Document the supported Date subclass adaptation and cover concurrent resolution, singleton-selecting factories, copied contexts, invalidation and date binding precedence.
Add return templates to filesystem borrowing, pinned Redis connections and SQS connection callbacks without changing runtime behavior. Let filesystem callback parameter templates accept the concrete client, adapter or driver selected by the caller while preserving useful Flysystem bounds for untyped callbacks. Cover typed SDK callbacks, untyped clients and concrete or inferred Flysystem parameters in the max-level type fixtures.
Record the merged attributes prerequisite and completed runtime section. Describe callback parameter templates and their type coverage, remove stale prerequisite gates and instruction line numbers, and keep the remaining implementation at the shared analysis infrastructure.
…d facades Resolve facade accessors and exact root classes from declarations and booted bindings without executing accessors or factories. Respect overrides and unknown construction paths, and include actionable Unsupported attribute guidance in diagnostic messages. Track constants referenced by method attributes and parameter defaults through the existing source dependency helper. Cover native and nullsafe calls, facade factories and binding chains, supported overrides, and uncertain roots.
Share the collected worker-state model across rules and recognize listener, observer, and server-operation contexts. Preserve per-method receiver evidence through inherited handlers and helpers, distinguishing container-built queued listeners from restored job payloads. Report boot-only and test-only calls with their parameter conditions and proven shared receivers. Merge injection evidence across routes, jobs, and constructors; recognize singleton factories and readonly promoted dependencies; and give configuration writes one diagnostic. Add malformed-attribute checks, isolated enablement coverage, and warm-cache regressions for registrations, facade accessors, and constant-backed defaults. Preserve unknown receiver and binding paths without executing application code.
Record the implemented facade resolution, observer discovery, merged injection evidence, and source dependencies. Clarify the bounded receiver and reference-alias analysis and mark section 3.3 as the next implementation step.
Plan removal of the global mutable-date opt-out while preserving useful customization and explicit interoperability conversions. Include stronger return types and PHPStan diagnostics once runtime immutability is guaranteed.
Align resolver, manager, Capsule, testing and queue accessor return types with the Connection base class already required by Eloquent and migrations. Keep custom non-PDO drivers supported and retain ConnectionInterface at independently supplied connection boundaries. Remove redundant type guards and analysis suppressions. Update resolver-backed test doubles without changing their behavioral assertions, including non-PDO rejection coverage and both queue skip-locked outcomes. Document the custom resolver and mock requirements. Expand the native request API porting guidance and remove the ineffective HTTP status assignment during CLI environment bootstrap. Database, queue, foundation, facade metadata and affected integration tests pass; unavailable database drivers remain skipped.
Default LOG_USER facility configuration to the equivalent user string in the framework and both application skeletons. Monolog resolves named string facilities but rejects numeric strings from the environment, so the default now reflects the supported environment value. Preserve the existing LogManager fallback and facility behavior. Static analysis and formatting pass for the combined change.
Detect handles escaping lending callbacks, premature completion of managed transactions, native request APIs, discarded immutable date results and explicit mutable date creation. Match framework declarations and preserve user overrides; share connection inspection with the queue transaction rule. Add configuration environment-cast checks, typed getter guidance, observer registration conventions and application binding-cycle detection. Keep migration column policy disabled by default. Restrict configuration value inference to proven application repository reads so independent repositories retain their declared types. Reuse runtime dependency tracking and configuration source discovery, remove redundant callable guards, and document legitimate framework exceptions with local reasons. Add behavioral rule fixtures and update type expectations. The PHPStan suite, static analysis and formatting pass.
Record the implemented resolver contract, configuration ownership boundary and declaration-aware rules. Describe supported borrowed-resource and transaction identities, regression coverage and the required consumer mock updates. Mark sections 3.3 through 3.12 complete and retain section 3.13 as the next implementation step.
Move exact-catch inspection into the coroutine helper so cancellation checks can share the same verdict without duplicate reports. Resolve receivers assigned inside try blocks while discarding stale input facts and preserving argument-dependent return types through PHPStan inference. Expose source method lookup for inherited and aliased trait methods while keeping constructor ownership intact. Cover a channel assigned from a typed parameter in the existing exact-catch fixture.
Report swallowed cancellation around recognized synchronous database, Redis, HTTP and wait operations in request, job and operation handlers. Follow forwarded methods, model traits and exception causes while respecting overrides, deferred work and schema defaults that only throw. Track exception constructor files for warm result-cache invalidation. Add default-on checks for raw coroutine spawning, direct Socialite context-property access, Swoole tables allocated in provider bodies and request-dependent BindWhen conditions. Inspect condition callables in their own source namespace and retain explicit reasons for framework-owned raw coroutine use. Add focused rule fixtures for accepted alternatives, distinct dispatch paths and verified regressions. Validation includes formatting, framework analysis, targeted rule and cache tests, and the PHPStan test suite.
Record the implemented receiver and callable analysis, supported synchronous operation families, exception propagation checks and cache dependencies. Describe the verified exclusions and regression coverage, and move remaining work to documentation, final verification and approved guidance updates.
Explain the new checks with corrective examples, configuration switches and supported exceptions. Describe deferred execution and receiver ownership boundaries without promising analysis of unknown callbacks or repositories. Update the rule index, method-attribute guidance and worker-context coverage. Record the config-repository typing difference from Larastan.
Explain that PHPStan checks restricted and unsupported methods. Replace the blanket redundant-singleton example with guidance that preserves intentional binding, optional injection and rebinding behavior, and correct the alias-registration description. Record documentation completion and the remaining benchmark and final review. Hold PR creation until the owner has reviewed completion and discussed next steps.
Capture the vendor directory chosen when creating a disposable skeleton copy and expose it alongside the existing skeleton source path. Preserve its lexical spelling so consumers can observe later symlink changes, and reset it with the runtime ownership state. Extend the runtime-copy test to verify the recorded path matches the generated link and restore the field with the other isolated test state.
Track Composer metadata through the skeleton and working vendor paths instead of the disposable runtime directory. Record skeleton vendor directory presence separately because even an empty directory suppresses the working vendor fallback. Preserve ordinary file and symlink dependency handling. Cover unchanged cache reuse, working vendor symlink changes, empty skeleton vendor transitions and newly supplied skeleton metadata. Targeted cache and bootstrap tests, framework analysis, the PHPStan suite and Testbench suite pass. Complete the rules plan and final integration review. Warm analysis now reuses results across runtime copies; retain the owner discussion gate before PR creation.
Lead new diagnostic sections with recommended framework usage, shorten dense coverage explanations and remove repeated diagnostic transcripts. Preserve configuration switches, inference limits and legitimate exceptions. Clarify facade lifetimes, configuration repository ownership and native request API adaptations. Review the wording against the Processes documentation and verify links, anchors and code fences.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
PR Summary by QodoAdd PHPStan safeguards for long-lived Hypervel workers
AI Description
Diagram
High-Level Assessment
Files changed (196)
|
Code Review by Qodo
1. Callable creation triggers call warnings
|
 [High impact] The PR adds checks and changes runtime behavior across worker requests and shared framework services. The PR appears safe to merge, with three non-blocking fixes recommended for the new analyzer rules.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| foreach ((new NodeFinder)->findInstanceOf($closure->expr, Variable::class) as $variable) { | ||
| if (is_string($variable->name)) { | ||
| $used[$variable->name] = true; | ||
| } | ||
| } | ||
|
|
||
| $captured = array_intersect_key($captured, $used); |
There was a problem hiding this comment.
getCapturedKind() counts variable names inside nested functions without checking whether their parameters hide the outer variable. Inside Redis::withConnection(), go(fn () => array_map(fn ($connection) => $connection, ['safe'])) is therefore reported as capturing the borrowed connection, although it never uses that connection.
Respect nested function scopes when collecting captured names. Otherwise, renaming an unrelated inner parameter changes whether safe code gets a warning.
| if (! $last instanceof Expression || ! $last->expr instanceof Throw_ || $this->leaves($statements, 0)) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
rethrows() accepts a trailing throw $exception even when an earlier branch throws a different exception without keeping the original. For example, a catch containing if (Coroutine::causedByCancellation($exception)) { throw new RuntimeException('Canceled'); } followed by throw $exception passes this check.
The cancellation path never reaches that final rethrow. The new exception loses the cancellation, but the analyzer gives no warning. Check earlier throws before accepting the catch as safe.
| $namespace = $this->getNamespace($function, $file); | ||
| $declarationScope = $this->scopeFactory->create(ScopeContext::create($file)); | ||
|
|
||
| $declarationScope = $namespace === null ? $declarationScope : $declarationScope->enterNamespace($namespace); | ||
|
|
||
| return $this->findRead($function->stmts ?? [], $this->getContainerVariable($function->params), $declarationScope) === null |
There was a problem hiding this comment.
Static method conditions are scanned in a scope containing only their file and namespace. A condition such as Conditions::active(...), whose body calls $app->make(self::REQUEST) with self::REQUEST = 'request', needs its declaring class to resolve that constant.
getRequestedAbstracts() otherwise cannot identify the request key, so the request-dependent condition escapes the check. Enter the method's declaring class before scanning its body, and cover this case in the tests.
This adds a set of PHPStan rules for mistakes that are easy to make in a long-lived Swoole worker and hard to see in review: calling a boot-only method while handling a request, letting a pooled connection escape the callback that borrowed it, swallowing a coroutine cancellation, creating a Swoole table in every process, and similar. It builds on the lifetime and
Unsupportedmethod attributes, which mark the methods these rules check.Laravel code mostly runs unchanged on Hypervel, but a worker serves many requests and coroutines at once, so some patterns that are harmless under PHP-FPM leak state or misbehave here. The framework already documents these. These rules turn that guidance into diagnostics, each with a fix and, where there is one, the legitimate exception that should be ignored with a reason.
A few runtime changes came out of the work, so they're described first. They're in their own commits.
Runtime changes
bind()orTransientservice therefore kept the first request's object for every later request. The container gainsresolvedShared(), which reports whether it holds a worker-shared instance, including auto-singletons. Facades cache those for the worker as before. Other roots are kept per coroutine, so each request gets its own and concurrent requests never share one. Clearing a facade's resolved instance now makes later facade calls resolve again in every coroutine.dateis bound by the framework. TheDatefacade used to callswap()on first use whendatewasn't bound, which registered a container instance in the middle of a request.FoundationServiceProvidernow bindsdatetoDateFactorywithsingletonIf(). ADatefacade subclass'sDEFAULT_FACADEtherefore only applies whiledateis unbound; an application should binddateto its factory instead. This is listed in the Support README's differences.Connection.ConnectionResolverInterface::connection()and the resolvers implementing it now declare the driver-neutralConnection, notConnectionInterface. Every built-in andDB::extend()connection already is one, and Eloquent and the migrator already required it, so a resolver returning anything else was already broken. The change removes several ignores andinstanceofchecks. Laravel's contract documentsConnectionInterface, so this is listed in the Database README and the porting guide.withDriver(),withAdapter()andwithClient(), the RediswithPinnedConnection()methods and the SQS pool'swithConnection()are now generic over their callback's return type. The filesystem methods also accept a callback that narrows its parameter to the disk's concrete driver, adapter or client.logging.syslog.facilityreadLOG_SYSLOG_FACILITYwith the integerLOG_USERas its default. An environment variable is always a string, and Monolog only accepts facility names as strings, so the variable has to hold a name such aslocal0. The default is now'user', which Monolog maps toLOG_USER, in the framework config and both skeletons.LoadEnvironmentVariablesno longer callshttp_response_code(). It was left over from Laravel's FPM bootstrap and does nothing in Hypervel, which bootstraps from the CLI before any response exists.The rules
Each rule has its own section in the static analysis docs, with an example, the fix and its exceptions. Every rule except unsupported calls can be turned off with a parameter.
BootOnly,TestsOnlyorBootOrTestsOnlyfrom request, job or operation code, applying each marker's conditions. Also markers whose arguments would misstate the restriction.checkRestrictedCallsUnsupported, including its message.checkBorrowedResourcescommit()orrollBack()that ends a transactiontransaction()manages before its closure returns, androllBack(0).checkManagedTransactionsheader(),setcookie(),session_*(),$_GET,$_SESSIONand the other PHP functions and superglobals that act on the process instead of the request.noNativeRequestApischeckDiscardedDatesCarbon.noMutableDatesenv()read with a boolean, integer or float default that isn't cast.checkConfigEnvCastsconfig('key')orget()for a key that always holds one type, which should usestring(),integer()and so on. Only runs when analyzing an application.checkTypedConfigGettersobserve()in a service provider for a model the application owns, which should useObservedBy.checkObserverAttributesjson(),enum()andset()columns. Off by default, since existing migrations shouldn't be edited.checkMigrationColumnTypescheckBindingCyclesnoRawCoroutinescheckSocialiteProvidersnew Swoole\Tablein a service provider'sregister()orboot(), which every process runs.checkSwooleTablesBindWhencondition that reads the current request or user, which would bind that request's choice for the worker's lifetime.checkBindWhenConditionscheckCancellationHandlingThe framework's own reports are fixed or, where the code is intentional, ignored with a reason: for example the coroutine primitives themselves, Horizon's
ignore_user_abort(), and Sentry'sspotlightsetting, which accepts a boolean or a URL.Shared analysis
Several rules need to know where code runs and whether a value is the worker's shared instance.
OnReceiveandOnClose, and Eloquent observers registered withobserve()orObservedBy, are analyzed as code that runs during an operation. The existing worker-state checks use these contexts too.whenSharedrestriction is only reported when the call is known to reach the shared instance: through a facade whose service the container shares, a named application-container resolution such asapp('router'), or a dependency the container injects into a controller action, a job handler or a readonly promoted constructor property. A factory you create withnewis not reported.Configuration types only for the application repository
Configuration value types now apply only to reads from the application's repository: the
config()helper, theConfigfacade, and calls such asapp('config')->get(...). A repository received as a parameter or property, or built withnew Repository($items), may hold other values, so it keeps its declared return types. Larastan applies the application's configuration types toget()on any repository. This is listed in the PHPStan README's differences.Result cache for package analysis
Package analysis boots a Testbench copy of the skeleton that is deleted when the process exits. The result cache recorded the copy's Composer metadata by its path inside that copy, so every warm run saw it as missing and reanalyzed everything that depends on the booted application, about a third of this repository. The cache now records the metadata where Testbench takes it from: the skeleton's
vendordirectory and the working package's, by their own paths, including whether the skeleton'svendordirectory exists. Testbench'sBootstrapper::runtimeVendorPath()returns the working vendor directory for this. A warmcomposer analyseof this repository now reanalyzes nothing and takes a few seconds instead of about forty.Docs
AGENTS.mdsays PHPStan checks the lifetime attributes, rewords binding pattern 3 to say what explicit registration changes, and corrects the description of binding an alias.Validation
Formatting, PHPStan, the analyzer test suite, the Testbench suite and the Support tests pass. Each rule has tests for what it reports, the allowed alternatives and its documented exceptions, and the result cache changes have warm-cache tests. The skeleton was installed the way CI installs it and analyzed with these rules, with no reports.
Compared with
0.4, a coldcomposer analyseof this repository is a few percent slower. Turning the new rules off didn't measurably change that, and warm runs are much faster because of the result cache fix.Summary by cubic
Adds 17 PHPStan rules that catch mistakes unique to Hypervel's long-lived Swoole workers: boot-only methods called while handling a request, pooled connections escaping the callback that borrowed them, and swallowed coroutine cancellations. A few runtime changes make the rules accurate, and a result-cache fix makes warm package analysis reanalyze nothing.
Runtime changes
bind()or transient services keep a per-coroutine root instead of caching the first request's object for the worker.dateis now bound inFoundationServiceProvider; aDatefacade subclass'sDEFAULT_FACADEonly applies whiledateis unbound.ConnectionResolverInterface::connection()and its implementations now return the driver-neutralConnectioninstead ofConnectionInterface, removing severalinstanceofchecks and ignores.'user'instead of the integerLOG_USER.LoadEnvironmentVariablesno longer callshttp_response_code().The rules
env()defaults, untyped config getters,observe()for owned models, binding cycles, raw coroutines, Socialite provider properties, Swoole tables created in providers, and request-dependentBindWhenconditions.Written for commit c329d6f. Summary will update on new commits.
Note
Add PHPStan rules for Hypervel's long-lived workers and per-coroutine facade roots
RestrictedCallRule), swallowed cancellation (SwallowedCancellationRule), borrowed-resource escapes, raw coroutines, mutable/discarded dates, managed transactions, binding cycles, request-dependent bindings, and moreWorkerStateHelper/WorkerStateAnalysismodel with a rewrittenWorkerLifetimesthat tracks event, listener, observer, and injection contextsFacade::resolveFacadeInstanceto keep cached roots per coroutine for non-shared, non-scoped services, and addsContainer::resolvedShared()(contract included) so the facade knows when a root is worker-sharedDatabaseQueue::getDatabase()now returnConnectioninstead ofConnectionInterface(docs updated); also bindsdatetoDateFactoryinFoundationServiceProviderConnectionResolverInterface::connection()return types (custom resolvers and test mocks must returnConnection), facade root caching semantics in Facade.php, andDate::resolveFacadeInstancewhich now caches the default instance directly withoutswap()Macroscope summarized c329d6f.