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

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

Conversation

@nbayramberdiyev
Copy link
Copy Markdown
Contributor

@nbayramberdiyev nbayramberdiyev commented Mar 30, 2025

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
Copy link
Copy Markdown
Contributor

odan commented Apr 29, 2025

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
Copy link
Copy Markdown
Contributor Author

nbayramberdiyev commented Apr 29, 2025

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