Skip to content

Improve types on routes - #3379

Closed
nbayramberdiyev wants to merge 3 commits into
slimphp:4.xfrom
nbayramberdiyev:improve-types-on-routes
Closed

nbayramberdiyev wants to merge 3 commits into
slimphp:4.xfrom
nbayramberdiyev:improve-types-on-routes

Conversation

@nbayramberdiyev

@nbayramberdiyev nbayramberdiyev commented Mar 30, 2025 •

Copy link
Copy Markdown
Contributor

My routes are defined as follows: ->get('/', [HomeController::class, 'index']);, which is a valid route definition. However, the latest version of Psalm emits the following error:

Argument 2 of Slim\App::get expects a public static callable, but a non-static callable provided (see https://psalm.dev/004)

This PR addresses this issue by adding the array{class-string, string} type to the route handler definition.

@coveralls

Copy link
Copy Markdown

Coverage Status

coverage: 99.523% (+0.001%) from 99.522%
when pulling c23d3c2 on nbayramberdiyev:improve-types-on-routes
into ad56812 on slimphp:4.x.

@odan

odan commented Apr 29, 2025

Copy link
Copy Markdown
Contributor

Hi! For Slim, we only use phpstan and not psalm. We also do not intend to support the psalm-specific "syntax" for the docblocks because it would be too much effort to maintain both. Thank you for your understanding.

@odan odan closed this Apr 29, 2025
@nbayramberdiyev
nbayramberdiyev deleted the improve-types-on-routes branch April 29, 2025 17:52
@nbayramberdiyev

nbayramberdiyev commented Apr 29, 2025 •

Copy link
Copy Markdown
Contributor Author

Hi! For Slim, we only use phpstan and not psalm. We also do not intend to support the psalm-specific "syntax" for the docblocks because it would be too much effort to maintain both. Thank you for your understanding.

I'm having some trouble understanding this. The syntax isn't specific to Psalm, it's also used by PHPStan. That said, the current definition of string|callable for the route handler is incomplete for either static analysis tool. Just to clarify, I'm not trying to introduce any Psalm-specific syntax here.

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.

3 participants