Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/contributing/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ When a **class stub and a trait stub** both declare the same method, Psalm creat

Stub files are loaded in alphabetical order (sorted by full path) to ensure deterministic results across OSes.

When a **stub re-declares a method that already has a conditional `@return` in Laravel's source** (such as `($x is null ? array : $this)`), restating the same condition (even with different branch types) can collide during the merge and collapse to one branch on some Psalm versions. If the stub only needs to add an annotation (e.g. `@psalm-variadic`), drop the conditional from the stub or diverge intentionally to one branch. See `stubs/common/Routing/Route.stubphp` and the issue #888 regression test for a worked example.

## How to add a handler

Handlers implement Psalm event interfaces to override type inference.
Expand Down
15 changes: 14 additions & 1 deletion stubs/common/Routing/Route.stubphp
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,21 @@ class Route
/**
* Get or set the middlewares attached to the route.
*
* Laravel's source declares `@return ($middleware is null ? array : $this)`. The previous
* stub restated the same `$middleware is null` check (with `string[]` on the null branch
* instead of `array`), and that overlap interacts badly with Psalm's stub/source merge on
* some Psalm versions: chained calls (`->middleware([...])->withoutScopedBindings()`)
* collapse to the null branch (issue #888). Until the upstream Psalm bug is fixed, the
* proper layer for this is `vimeo/psalm` itself; this stub keeps only the setter branch
* (`$this`) as a workaround.
*
* Trade-off: the no-arg getter form `$route->middleware()` resolves to `$this` instead
* of `array`. That form is rarely called from user code. `@psalm-variadic` survives
* because issue #888's discriminating tests confirmed it isn't the cause; the new test
* pins the variadic call form.
*
* @param string[]|string|null $middleware
* @return ($middleware is null ? string[] : $this)
* @return $this
*
* @psalm-variadic
*/
Expand Down
61 changes: 61 additions & 0 deletions tests/Type/tests/RouteMiddlewareTest.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
--FILE--
<?php declare(strict_types=1);

namespace App;

use Illuminate\Routing\Route;
use Illuminate\Routing\RouteCollectionInterface;
use Illuminate\Support\Facades\Route as RouteFacade;

// Regression for issue #888: chained calls after Route::middleware(...) must not collapse
// to the array branch of the (now-removed) conditional return type. The setter form must
// always return $this so subsequent ->withoutScopedBindings(), ->name(...), or
// RouteCollectionInterface::add() calls type-check.
function middleware_setter_variants_return_this(Route $route): void
{
$_array = $route->middleware(['web']);
/** @psalm-check-type-exact $_array = Route&static */

$_string = $route->middleware('auth');
/** @psalm-check-type-exact $_string = Route&static */

// Variadic call form — the original reason `@psalm-variadic` lives on the stub.
$_variadic = $route->middleware('auth', 'web');
/** @psalm-check-type-exact $_variadic = Route&static */

// No-arg form: the fix deliberately drops the `array` branch of the original
// conditional return, so even `middleware()` resolves to `$this`. Pin this so a
// future contributor restoring the conditional (`($middleware is null ? ...)`)
// can't silently reintroduce #888.
$_get = $route->middleware();
/** @psalm-check-type-exact $_get = Route&static */
}

// The exact construct from the #888 reproducer title: chaining ->withoutScopedBindings()
// after ->middleware([...]). If middleware() collapses to array, the chained call raises
// InvalidMethodCall.
function middleware_then_without_scoped_bindings(Route $route): void
{
$_chain = $route->middleware(['web'])->withoutScopedBindings();
/** @psalm-check-type-exact $_chain = Route&static */
}

// Facade -> verb -> ->middleware([...]). The Route facade resolution path was flagged in
// the issue as a possible widening source; this pins the chain return type for that path.
function facade_chain_after_middleware(): void
{
$_chain = RouteFacade::get('/test', fn () => 'ok')->middleware(['web']);
/** @psalm-check-type-exact $_chain = Route&static */
}

// Passing the chain result to RouteCollectionInterface::add — the second concrete failure
// mode in #888. If middleware() collapses to array<array-key, string>, this raises
// InvalidArgument.
function pass_to_route_collection_add(RouteCollectionInterface $routes): Route
{
return $routes->add(
RouteFacade::get('/test', fn () => 'ok')->middleware(['web']),
);
}
?>
--EXPECT--