Hi, it's not gonna be that easy because I still want it to work on Rule classes (see the failed build). I know it isn't exactly right but it's a common pattern. Basically I want to specify the type of the parameter in phpDoc because >I know< what's going to be passed there.

@ondrejmirtes Don't worry, we can ignore all errors from the Rule class with one simple regex. We don't have to change that code. Although I do think it would be cleaner to actually add an assert or throw a ShouldNotHappenException.

I have some issues with traits at the moment (for some reason the phpDoc types are ignored in traits). I'll see if I can fix that right now.

Great, now it suddenly works and I've changed absolutely nothing 😬

You cannot solve #532 without introducing generics first. The Rule interface is an example of that, because it should actually be Rule<in T : Node>. See also #729 (comment) for example.

@JanTvrdik #532 is not about generics but about covariance and contravariance. We don't need generics for the support that. Sure, because of the introduction of this rule I had to ignore this error:

- '#^Declaration of PHPStan\\Rules\\.*::processNode\(.*\): .* should be compatible with PHPStan\\Rules\\Rule::processNode\(.*\): .*$#'

@JanTvrdik

Also, I don't think people will really use generics until it's a language feature or something their IDE supports. For example, at our company we're not using the array<KType, VType> syntax because PHPStorm doesn't understand it. I'm not arguing about the usefulness of generics themselves. I just feel like that would make for a horrible IDE experience.

@JanTvrdik #532 is not about generics but about covariance and contravariance.

Yes, but without generics you're missing the key information that is required to decide whether the parameter contravariance is valid. You simply do not have enough data.

@JanTvrdik

Yes, but without generics you're missing the key information that is required to decide whether the parameter contravariance is valid.

Well, this is gonna break some code. Even if we introduce generics people will have to go and annotate their code with generic parameters. Generics aren't gonna change that.

There's other ways to achieve similar results. You could:

class SomeRule implements Rule
{
    public function processNode(Node $node, Scope $scope): array
    {
        if (!$node instanceof Function_) {
            throw new \InvalidArgumentException();
        }

        // ...
    }
}

Obviously this does not protect you from the calling side as the signature is still the same. But that's really not the point here. The method signature rule is all about keeping the promise of the method signature.

Note that PHP itself has simple forms of covariance and contravariance while lacking generic support.

@ondrejmirtes

Can you give me feedback on this PR?

One issue I've had is that the Squiz.Commenting.FunctionComment.InvalidNoReturn rule doesn't let me annotate a return type if all the method does is throw an exception. I created an issue over at squizlabs but it doesn't look like that will be fixed. What do we do in cases like this? I can understand if phpcs:ignore is not an option.

Also, how do you feel about the whole generics issue?

@JanTvrdik

I added support for the %reportMaybes% parameter. This allows you to use parameter covariance up until level 6 when reportMaybes is enabled. After that it's prohibited. Unrelated types will always throw an error.

Does this defuse the situation in your opinion?

I'm still a little bit torn about this - people sometimes use phpDocs to override badly documented types of parent classes, therefore breaking LSP on purpose.

people sometimes use phpDocs to override badly documented types of parent classes, therefore breaking LSP on purpose.

That's definitely a valid argument. However, I don't think we should treat the exception as the rule. You can always ignore such cases or ignore the rule completely if that's happens all over your codebase. Also, only invariant types are reported all the way up to level 6.

After fixing all phpDoc issues this finally works with no dirty workarounds 🎉

Sorry, I had some personal issues these days. Give me a few days or feel free to take over.

No problem, I'll wait 👌 Take care.

@ondrejmirtes This is ready. 🙂

Perfect! :) Thank you very much.

@ondrejmirtes Awesome, thank you! Do we wanna turn on %reportMaybesInMethodSignatures% in strict-rules now? If so I'll create a pull request over there too.

Yeah, please do :) I also added a commit here so that the build doesn't break (because of Rule interface implementations): 978f829