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

Enable more checks for native classes on PHP 8 #7476

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

danog
Copy link
Collaborator

@danog danog commented Jan 24, 2022

Coupled with #7475, aims to reach feature parity with phpstan vs psalm.

This PR enables these checks only on 8.1, and like phpstan it doesn't support suppression via the ReturnTypeWillChange attribute, which could possibly cause BC problems when seeking to support PHP 7 (unlike my earlier merged PR which does support ReturnTypeWillChange, but doesn't check for things like mismatched types).

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/66d598df84
<?php

class Foo implements Iterator
{
    public function current(string $foo) {
        return 'foo';
    }
    
    public function next($a) {
    }
    
    public function key() {}
    
    public function valid(): string
    {
        return 'foo';
    }
    
    public function rewind(): void
    {
        
    }
}
Psalm output (using commit ab2b77d):

ERROR: MethodSignatureMustProvideReturnType - 5:21 - Method Foo::current must have a return type signature!

ERROR: MethodSignatureMustProvideReturnType - 9:21 - Method Foo::next must have a return type signature!

ERROR: MethodSignatureMustProvideReturnType - 12:21 - Method Foo::key must have a return type signature!

INFO: MissingParamType - 9:26 - Parameter $a has no provided type

@danog danog changed the title Enable more checks for native classes Enable more checks for native classes on 8.1 Jan 24, 2022
@danog danog marked this pull request as draft January 24, 2022 11:12
@danog danog changed the base branch from 4.x to master March 14, 2022 10:19
@danog danog changed the title Enable more checks for native classes on 8.1 Enable more checks for native classes on PHP 8 Mar 14, 2022
@danog danog marked this pull request as ready for review March 14, 2022 10:55
@danog
Copy link
Collaborator Author

danog commented Mar 14, 2022

All done, the checks are enabled when targeting 8.0+.

@orklah
Copy link
Collaborator

orklah commented Mar 14, 2022

Thanks for the work. I'm afraid we have some issues about user_defined when using stubs (which a lot of users do) that won't allow this to be merged for now...

We had to disable your other related feature about type inheritance in the core because of that stub issue so I feat this will be compromised as well. Please keep this open, if the fix is made in the stubs, we'll be able to merge that then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants