Repository navigation
fix(routes): HANDLES for Laravel array and invokable controller actions - #2465
Merged
Merged
Conversation
Closed
2 tasks done
Why: Laravel names a route's controller as [UserController::class, 'show'], or, for an invokable controller, as GetCurrentUserController::class. The route handler scan only took identifiers and strings, so these routes never got a HANDLES edge. The cross-repo HTTP matcher reaches a route's handler through that edge, so apps written this way - the #1146 reporter's app among them - produced 0 CROSS_HTTP_CALLS. Resolving the class by short name is not enough either: the call pass's import map bound `use App\Http\Controllers\UserController` to Admin\UserController by name, even with a composer.json PSR-4 map present, and the generic resolver's multi-candidate fallback bound a vendor controller's show() to another class's show(). Fix: - extract_calls.c: for PHP, [X::class, 'm'] becomes the handler reference "Ns\X::m" and X::class becomes "Ns\X::__invoke". X is qualified the way PHP resolves it: through the file's `use` imports (aliases included), else the file namespace; a leading backslash is already absolute. - registry.c, cbm_registry_resolve_handler (both route passes): a class-qualified handler is placed on a method QN only by what places the class file - the composer.json PSR-4 root in the package map, else the one candidate whose folders mirror the namespace (App\Http\Controllers -> app/Http/Controllers). A class that is not in the repo gets no handler. A handler reference without "::" resolves exactly as before. Proof (release CLI built from this tree vs origin/main 1f9b4db): - laravel.io: HANDLES 0 -> 4, one for each of the 5 route registrations main mints (two share a Route). The use statement tells Articles\ArticlesController apart from Admin\ArticlesController. Route count unchanged (39). With the slashless-path and withRouting fixes also applied: HANDLES 0 -> 58, all from PHP controllers, samples checked against source (aliased AdminArticlesController and AdminUsersController included); the 7 routes left without a handler use methods from a vendor trait (AuthenticatesUsers). - Reporter-shaped cross-repo check (Laravel 11 withRouting api:, invokable GetCurrentUserController under prefix('/users'), TS fetch('/api/users/me')): CROSS_HTTP_CALLS 0 with this fix alone (route /users/me, no /api mount) and 0 with the withRouting fix alone (no HANDLES); 2 with both applied. - krayin/laravel-crm (reduced to one route file): / <- Webkul\Admin\Http\ Controllers\Controller::redirectToLogin, placed through the PSR-4 root Webkul\Admin\ -> packages/Webkul/Admin/src. Tests (edge_types_probe): handles_laravel_class_handlers_issue1146, handles_laravel_class_handlers_parallel_issue1146 and handles_laravel_class_handlers_no_composer_issue1146 assert the exact HANDLES set. They include controls for a same-named Admin\UserController::show, an alias import, an unimported fully-qualified class, a package under a PSR-4 root whose folder does not mirror its namespace, a decoy __invoke, and two vendor controllers that must get no edge. RED on origin/main 3x, green, RED on revert. edge_types_probe, php_lsp, pipeline, parallel, route_canon and cross_repo pass (730); make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…olver Why: since 830f0cc (#2509) the package map stores a composer.json PSR-4 prefix ("Acme\Blog\") as its repo-relative directory ("packages/blog/src"), not as a dotted QN root. The route-handler resolver still formatted that value as a QN root and built "packages/blog/src.Http.PostsController. PostsController.index", which names no node. The PSR-4 step therefore resolved nothing. Handlers whose folders mirror their namespace (App\Http\Controllers -> app/Http/Controllers) still got their edge through the namespace/folder fallback, but a package class under a root whose folder does not mirror its namespace got no HANDLES edge. That is the case of packages/blog/src in the test fixture and of krayin's Webkul\Admin\ -> packages/Webkul/Admin/src. Change: resolve the handler class the way the import pass resolves `use Ns\Class;` for PHP. - pass_pkgmap.c: psr4_file_node and resolve_php_psr4_class take the graph buffer instead of the pipeline ctx, since they only read ctx->gbuf. The new cbm_pipeline_psr4_member_qn runs resolve_php_psr4_class for the class FQN, takes the Class/Interface/Trait/Enum node of that name defined in the resolved file, and returns the QN of <class QN>.<member> when that node exists. - registry.c: psr4_member_qn delegates to it and no longer builds a QN from the package-map value. cbm_registry_resolve_handler takes the graph buffer in place of the package map. The namespace/folder fallback is unchanged. - pass_calls.c and pass_parallel.c pass the graph buffer they already use for the handler lookup: ctx->gbuf, and main_gbuf, which is read-only during the resolve phase. Proof: - handles_laravel_class_handlers_issue1146 and its _parallel twin (edge_types_probe): RED 3/3 before the change ("missing PostsController.index -> /blog", expected=4 actual=3; suite 75 passed, 2 failed). GREEN 3/3 after (77 passed). RED again with the production change reverted. GREEN again once it was restored. - Real input: no Laravel app with a non-mirroring PSR-4 root is available locally. koel/koel v9.11.1 (App\ -> app/) was indexed with the binary before and after, each under a private runtime and cache. The results are identical: 16,564 nodes, 60,177 edges, 15 Routes and 1 HANDLES (/demo/new-session -> NewSessionController.__invoke), with the same handler rows. - Suites: edge_types_probe 77, edge_imports 68, pipeline 317 and route_canon 11 all pass (473). make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/laravel-array-invokable-handlers
branch
from
October 4, 2026 19:12
70c2c07 to
42118ad
Compare
…olved Why: when a composer.json PSR-4 prefix covers a handler class but no class file exists for it, cbm_registry_resolve_handler fell through to the namespace/folder fallback and bound the route to a same-named class in any folder that mirrors the namespace, a class composer never loads. Main's import resolver stops in that case: resolve_php_psr4_class leaves the `use` import unresolved (#1186). The handler now does the same, so no guessed HANDLES edge is minted. Classes that no PSR-4 prefix covers keep the fallback unchanged. Change: - pipeline_internal.h: cbm_pipeline_psr4_member_qn returns a tri-state cbm_psr4_member_t (NOT_COVERED, RESOLVED, UNRESOLVED) and hands out the member QN through an out parameter, set only when RESOLVED. UNRESOLVED means a prefix covers the class but no class file holds the class with that member: the file is absent, or it lacks the class or the member. - pass_pkgmap.c: maps the outcome of resolve_php_psr4_class onto it; the class/member lookup in the resolved file moves into psr4_member_in_file. - registry.c: cbm_registry_resolve_handler returns unresolved on UNRESOLVED and runs the folder fallback only on NOT_COVERED. Proof: - New handles_laravel_psr4_absent_class_issue1146 and its _parallel twin (edge_types_probe) extend the issue-1146 fixture with a route to \Acme\Blog\Http\MissingController::index (covered by Acme\Blog\ -> packages/blog/src/, no class file) and one to PostsController::archive (class file exists, no archive method), plus namespace-mirroring decoys in legacy/Acme/Blog/Http that hold both members. The test asserts exactly the 4 HANDLES of the fixture and that both Route nodes exist. RED before the change on both paths with the 2 decoy edges (legacy...MissingController.index -> /blog/missing, legacy...PostsController.archive -> /blog/archive; expected=4 actual=6; suite 77 passed, 2 failed). GREEN after (79 passed). RED again with the production change reverted (the same 2 edges), GREEN once it was restored. - Real input: koel/koel v9.11.1 (App\ -> app/) indexed with the new binary under a private HOME, cache and runtime: 16,564 nodes, 60,177 edges, 15 Routes and 1 HANDLES (/demo/new-session -> NewSessionController.__invoke), identical to the numbers recorded for 42118ad. - Suites: edge_types_probe 79, edge_imports 68, pipeline 317 and route_canon 11 all pass (475). make lint-ci passes. Refs #1146 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…batch Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…batch Conflict in tests/test_edge_types_probe.c: both sides added a self-contained helper + tests before the Rails test (this branch: et_handles_exact_routes and the class-handler tests; main/#2471: et_route_set_exact and the slashless-route tests) and their RUN_TEST lines; both blocks kept whole, this branch's first. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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
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.
Why: Laravel names a route's controller as [UserController::class, 'show'],
or, for an invokable controller, as GetCurrentUserController::class. The
route handler scan only took identifiers and strings, so these routes never
got a HANDLES edge. The cross-repo HTTP matcher reaches a route's handler
through that edge, so apps written this way - the #1146 reporter's app
among them - produced 0 CROSS_HTTP_CALLS. Resolving the class by short name
is not enough either: the call pass's import map bound
use App\Http\Controllers\UserControllerto Admin\UserController by name, evenwith a composer.json PSR-4 map present, and the generic resolver's
multi-candidate fallback bound a vendor controller's show() to another
class's show().
Fix:
"Ns\X::m" and X::class becomes "Ns\X::__invoke". X is qualified the way PHP
resolves it: through the file's
useimports (aliases included), elsethe file namespace; a leading backslash is already absolute.
class-qualified handler is placed on a method QN only by what places the
class file - the composer.json PSR-4 root in the package map, else the one
candidate whose folders mirror the namespace (App\Http\Controllers ->
app/Http/Controllers). A class that is not in the repo gets no handler.
A handler reference without "::" resolves exactly as before.
Proof (release CLI built from this tree vs origin/main 1f9b4db):
main mints (two share a Route). The use statement tells
Articles\ArticlesController apart from Admin\ArticlesController.
Route count unchanged (39). With the slashless-path and withRouting
fixes also applied: HANDLES 0 -> 58, all from PHP controllers, samples
checked against source (aliased AdminArticlesController and
AdminUsersController included); the 7 routes left without a handler use
methods from a vendor trait (AuthenticatesUsers).
GetCurrentUserController under prefix('/users'), TS fetch('/api/users/me')):
CROSS_HTTP_CALLS 0 with this fix alone (route /users/me, no /api mount) and
0 with the withRouting fix alone (no HANDLES); 2 with both applied.
Controllers\Controller::redirectToLogin, placed through the PSR-4 root
Webkul\Admin\ -> packages/Webkul/Admin/src.
Tests (edge_types_probe): handles_laravel_class_handlers_issue1146,
handles_laravel_class_handlers_parallel_issue1146 and
handles_laravel_class_handlers_no_composer_issue1146 assert the exact HANDLES
set. They include controls for a same-named Admin\UserController::show, an
alias import, an unimported fully-qualified class, a package under a PSR-4
root whose folder does not mirror its namespace, a decoy __invoke, and two
vendor controllers that must get no edge. RED on origin/main 3x, green, RED
on revert. edge_types_probe, php_lsp, pipeline, parallel, route_canon and
cross_repo pass (730); make lint-ci passes.
Refs #1146