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

Switching to PHPStan parser #27

Merged

Conversation

moufmouf
Copy link
Member

@moufmouf moufmouf commented Jul 3, 2018

As suggested in #24 , PHPStan parser is now stable and complete enough so we can simply drop better-reflection.

This PR removes better-reflection and replaces it with PHPStan parser.
As a side effect, the rules should be really faster, which is awesome.

Ping @juliangut

@juliangut
Copy link

This looks awesome, I'll give it a try ASAP

In the meantime I've noticed there are still some references to BetterReflection here
And shouldn't this messages be transformed to ShouldNotHappenExcetion?

@moufmouf
Copy link
Member Author

moufmouf commented Jul 4, 2018

In the meantime I've noticed there are still some references to BetterReflection here

👍 I'll fix this this afternoon.

And shouldn't this messages be transformed to ShouldNotHappenExcetion?

Actually, you are not allowed to throw an exception in a __toString() method.

I'm giving the changes a test run on some projects before merging this. Do not hesitate if you see any bug.

@juliangut
Copy link

I'm testing it and it really got fast!

For now I think I've found something regarding Traits

------ ------------------------------------------------------------------------------------------------------------- 
  Line   src/Shared/Domain/DTO/PayloadTrait.php (in context of class XXX\Shared\Domain\CQRS\AbstractAsyncCommand)  
 ------ ------------------------------------------------------------------------------------------------------------- 
  71     Method XXX\Shared\Domain\CQRS\AbstractAsyncCommand::get() has no                                          
         return typehint specified.                                                                                   
  111    In method "XXX\Shared\Domain\CQRS\AbstractAsyncCommand::__call",                                          
         parameter $parameters type is "array". Please provide a @param                                               
         annotation to further specify the type of the array. For instance:                                           
         @param int[] $parameters                                                                                     
  111    Method XXX\Shared\Domain\CQRS\AbstractAsyncCommand::__call() has no                                       
         return typehint specified.                                                                                   
 ------ -------------------------------------------------------------------------------------------------------------
abstract class AbstractAsyncCommand implements AsyncCommand
{
    use SerializablePayloadTrait;
    [...]
}

trait SerializablePayloadTrait
{
    use PayloadTrait {
        setPayloadParameter as protected setSerializablePayloadParameter;
    }
   [...]
}

trait PayloadTrait
{
    /**
     * @param string $parameter
     * @return mixed
     */
    public function get(string $parameter)
    [...]

    /**
     * @param string  $method
     * @param mixed[] $parameters
     * @return DTO|bool|mixed
     */
    public function __call(string $method, array $parameters)
    [...]
}

I see here that when a Trait uses another Trait PHPStan Rule looses not only the method docblock but also doesn't consider the list of ignored methods for return types

@moufmouf
Copy link
Member Author

moufmouf commented Jul 4, 2018

Thanks for the feedback! I'll look into it.

It is indeed fast.

My test suite jumped from 15s to 1.7s :)

@moufmouf
Copy link
Member Author

moufmouf commented Jul 4, 2018

The error related to dockblock losts in traits has already been reported to PHPStan: phpstan/phpstan#1142
It is not related to this rule so we will have to be patient and wait for a fix.

@moufmouf
Copy link
Member Author

moufmouf commented Jul 4, 2018

Closes #24

@moufmouf moufmouf merged commit e0892ef into thecodingmachine:master Jul 4, 2018
@moufmouf moufmouf deleted the switching_to_phpstan_parser branch July 4, 2018 13:39
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

2 participants