Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Processor with Symfony : Parameter $context of method use Laravel Illuminate type #6890

Closed
fouteox opened this issue Dec 31, 2024 · 8 comments
Labels

Comments

@fouteox
Copy link

fouteox commented Dec 31, 2024

API Platform version(s) affected: 4.0.13

Description
With Symfony project and Phpstan 2.0 :

 ------ ------------------------------------------------------------ 
  Line   src/State/ActionStateProcessor.php                          
 ------ ------------------------------------------------------------ 
  39     Parameter $context of method                                
         App\State\ActionStateProcessor::process() has invalid type  
         Illuminate\Http\Request.                                    
         🪪  class.notFound

How to reproduce
Create a custom processor in symfony project and check phpstan.

Possible Solution
Not use Laravel Illuminate Type on Symfony project.

Additional Context
Le problème semble se situer dans le fichier suivant :
https://github.com/api-platform/core/blob/main/src/State/ProcessorInterface.php

@vinceAmstoutz
Copy link
Contributor

vinceAmstoutz commented Dec 31, 2024

I think that | causes it:
https://github.com/api-platform/core/blob/9389b4f4637b925e55ee68c3f62c88828014c14f/src/State/ProcessorInterface.php#L34C59-L34C84

Any ideas @soyuka for documenting Request|Illuminate\HttpRequest without error when one of them isn't installed (because we're using Symfony or Laravel API Platform variant)?

@mrossard
Copy link
Contributor

mrossard commented Jan 5, 2025

That's hardly an api platform problem IMO...can't you just add a @phpstan-ignore to your ActionStateProcessor.php or something?

@fouteox
Copy link
Author

fouteox commented Jan 5, 2025

Yes I can, but we shouldn't have to cover up a problem I didn't create...

And I didn't specify it in my first message but it is valid for ALL processors.

@mrossard
Copy link
Contributor

mrossard commented Jan 5, 2025

Yes I can, but we shouldn't have to cover up a problem he didn't create...

I don't see an easy way to prevent this on the api platform side though, and it's not an actual problem IMO.
To prevent this you'd need an interface implemented by both the symfony and laravel Request objects, which seems highly unlikely, or you just have to give up specifying the type for the request altogether. That would weaken the framework documentation to accommodate phpstan on the user side, which doesn't seem right...

If that's that much of a bother, you can also composer require -dev api-platform/laravel 😇

@nesl247
Copy link
Contributor

nesl247 commented Jan 7, 2025

I do think this needs to be solved somehow. It doesn't seem right that the user should need to add an ignore in every single processor. We have 60+ processors and growing. I'm sure other people have even more.

What I think would be an option is forProcessorInterface to be extended by a SymfonyProcessorInterface and LaravelProcessorInterface which can explicitly type the context. Otherwise, maybe a phpstan extension could be created that would ignore this particular error depending upon the corresponding package being installed. Though I'm not sure if that's possible or not.

@AirBair
Copy link
Contributor

AirBair commented Jan 8, 2025

Even if i agree with previous comments, you don't need to add it on every processor file, just add it in the phpstan.dist.neon to globally ignore this specific error:

parameters:
    # ....
    ignoreErrors:
        -
            # API Platform ProcessorInterface reference a non-existent Illuminate\Http\Request class for compatibility with Laravel.
            message: '#Parameter \$context of method [a-zA-Z0-9\\_]+::process\(\) has invalid type Illuminate\\Http\\Request#'
            paths:
                - src/Processor/*

@nesl247
Copy link
Contributor

nesl247 commented Jan 8, 2025

Even if i agree with previous comments, you don't need to add it on every processor file, just add it in the phpstan.dist.neon to globally ignore this specific error:

parameters:
    # ....
    ignoreErrors:
        -
            # API Platform ProcessorInterface reference a non-existent Illuminate\Http\Request class for compatibility with Laravel.
            message: '#Parameter \$context of method [a-zA-Z0-9\\_]+::process\(\) has invalid type Illuminate\\Http\\Request#'
            paths:
                - src/Processor/*

I actually didn't realize that phpstan supported wildcards for paths. Thanks for this!

@soyuka soyuka added the bug label Jan 9, 2025
@soyuka
Copy link
Member

soyuka commented Jan 9, 2025

we need a fix for that though, Illuminate\Request extends the Request so we could maybe just remove the laravel specific type?

soyuka added a commit that referenced this issue Jan 9, 2025
@soyuka soyuka closed this as completed Jan 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants