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

Fix Dynamic call to static method Illuminate\Foundation\Application::configurationIsCached() #917

Merged
merged 6 commits into from
Sep 22, 2021
Merged

Fix Dynamic call to static method Illuminate\Foundation\Application::configurationIsCached() #917

merged 6 commits into from
Sep 22, 2021

Conversation

LastDragon-ru
Copy link
Contributor

  • Added or updated tests
  • Documented user facing changes
  • Updated CHANGELOG.md

Changes

larastan + phpstan-strict generates false positives (related to #483):

Dynamic call to static method  Illuminate\Foundation\Application::configurationIsCached().

this PR seems to fix it.

Breaking changes

Not sure.

PS: If needed I can provide link to reproduce.

@szepeviktor
Copy link
Collaborator

@LastDragon-ru Thank you for the PR!

Big Tragedy! Unit tests fail.

@LastDragon-ru
Copy link
Contributor Author

Big Tragedy! Unit tests fail.

Not sure that it is related to the PR 🤔

-- --------------------------------------------------------------------------- 
     Error                                                                      
 -- --------------------------------------------------------------------------- 
     Child process error (exit code 255): PHP Fatal error:  Declaration of      
     Illuminate\Http\Response::setContent($content) must be compatible with     
     Symfony\Component\HttpFoundation\Response::setContent(?string $content):   
     static in                                                                  
     /home/runner/work/larastan/larastan/vendor/laravel/framework/src/Illumina  
     te/Http/Response.php on line 48                                            
     Fatal error: Declaration of                                                
     Illuminate\Http\Response::setContent($content) must be compatible with     
     Symfony\Component\HttpFoundation\Response::setContent(?string $content):   
     static in                                                                  
     /home/runner/work/larastan/larastan/vendor/laravel/framework/src/Illumina  
     te/Http/Response.php on line 48                                            
                                                                                
     Child process error (exit code 255): PHP Fatal error:  Declaration of      
     Illuminate\Http\Request::duplicate(?array $query = null, ?array $request   
     = null, ?array $attributes = null, ?array $cookies = null, ?array $files   
     = null, ?array $server = null) must be compatible with                     
     Symfony\Component\HttpFoundation\Request::duplicate(?array $query = null,  
     ?array $request = null, ?array $attributes = null, ?array $cookies =       
     null, ?array $files = null, ?array $server = null): static in              
     /home/runner/work/larastan/larastan/vendor/laravel/framework/src/Illumina  
     te/Http/Request.php on line 445                                            
     Fatal error: Declaration of Illuminate\Http\Request::duplicate(?array      
     $query = null, ?array $request = null, ?array $attributes = null, ?array   
     $cookies = null, ?array $files = null, ?array $server = null) must be      
     compatible with                                                            
     Symfony\Component\HttpFoundation\Request::duplicate(?array $query = null,  
     ?array $request = null, ?array $attributes = null, ?array $cookies =       
     null, ?array $files = null, ?array $server = null): static in              
     /home/runner/work/larastan/larastan/vendor/laravel/framework/src/Illumina  
     te/Http/Request.php on line 445                                            
                                                                                
 -- ---------------------------------------------------------------------------

@szepeviktor
Copy link
Collaborator

Not sure that it is related to the PR 🤔

You are right. The last scheduled CI run was broken before this PR https://github.com/nunomaduro/larastan/actions

@szepeviktor
Copy link
Collaborator

Temporarily disabling tests on Laravel 9.
@LastDragon-ru We need 1 test that fails without this PR and single line of text in the changelog.

CHANGELOG.md Outdated Show resolved Hide resolved
@LastDragon-ru
Copy link
Contributor Author

We need 1 test that fails without this PR and

We have got a problem: it will fail only if https://github.com/phpstan/phpstan-strict-rules/ is installed, but it is not installed and I have no idea how to enable it for tests. (?)

Also, there is a similar problem with Macroable: larastan marks all methods static but they should be static and non-static at the same time (it is possible?), or we will have a lot of "Dynamic call to static method".

single line of text in the changelog.

Fixed

@szepeviktor
Copy link
Collaborator

All right.
Let's wait for @canvural

Copy link
Collaborator

@canvural canvural left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Thanks for the PR!

I think the change is small enough and looks correct. In this case we can get away with without a test. I just left 2 small comments, after those it should be ready to merge.

.github/workflows/tests.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@canvural
Copy link
Collaborator

Ah, and there is a conflict to fix.

@LastDragon-ru
Copy link
Contributor Author

Ah, and there is a conflict to fix.

@canvural fixed.

@canvural canvural merged commit b374347 into larastan:master Sep 22, 2021
@canvural
Copy link
Collaborator

Thank you!

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.

None yet

3 participants