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

Having MethodSignatureMismatch when a trait method has the return type 'static', but is overriden in a trait #8673

Open
ollieread opened this issue Nov 5, 2022 · 6 comments

Comments

@ollieread
Copy link

Any methods that I have in traits that have a return type of static, which are overridden in a later trait, are giving me MethodSignatureMismatch errors, saying that it should return the trait, which isn't possible.

Here's an example error.

ERROR: MethodSignatureMismatch - src/Concerns/SortsCollection.php:59:5 - Method Smpl\Collections\Concerns\SortsCollection::setComparator with return type 'Smpl\Collections\SortedSet' is different to return type 'Smpl\Collections\Concerns\HasComparator' of inherited method Smpl\Collections\Concerns\HasComparator::setComparator (see https://psalm.dev/042)
    /**
     * @param \Smpl\Utils\Contracts\Comparator<E>|null $comparator
     *
     * @return static
     */
    public function setComparator(?Comparator $comparator = null): static

Here's the recreation: https://psalm.dev/r/c655b72643

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c655b72643
<?php
/**
 * @template E of mixed
 */
trait SomeTrait {
    /** @return static */
    public function get(): static {
        return $this;
    }
}

trait SomeOtherTrait {
    /** @return static */
    public function get(): static {
        return $this;
    }
}

/**
 * @template E o of mixed
 */
class SomeClass {
    /** @use SomeTrait<E> */
    use SomeTrait;
}

/**
 * @template E o of mixed
 * @extends SomeClass<E>
 */
class SomeOtherClass extends SomeClass {
    use SomeOtherTrait;
}
Psalm output (using commit 1986c8b):

ERROR: MethodSignatureMismatch - 14:5 - Method SomeOtherTrait::get with return type 'SomeOtherClass' is different to return type 'SomeTrait' of inherited method SomeTrait::get

@orklah
Copy link
Collaborator

orklah commented Nov 6, 2022

It may be a check to avoid this: https://3v4l.org/bfbC0 in older php versions and we missed the change somehow?

@ollieread
Copy link
Author

It may be a check to avoid this: https://3v4l.org/bfbC0 in older php versions and we missed the change somehow?

Yeah, most likely. I remember that used to be an annoying issue, but im using 8.1, so it should be fine.

@orklah
Copy link
Collaborator

orklah commented Nov 6, 2022

Unfortunately, I have like 0 practice with traits so I'm easily confused, especially with advanced usage.

I dug a little, the issue is emitted from here:

new MethodSignatureMismatch(

I noticed a php > 8 check, so I went looking for the change that introduced it, and it is here:
127e66d
in which I noticed a new param called $trait_mismatches_are_fatal with a value $codebase->php_major_version >= 8

So, my previous explanation seems wrong. We had a TraitMethodSignatureMismatch issue emitted before PHP 8 and now we have a MethodSignatureMismatch because mismatch are supposed to be more problematic in PHP 8. However, your code clearly doesn't emit a fatal, so this case should be allowed.

I'd say it's a wrongly inferred mismatch, possibly due to having static inferrence inside traits (that confuses me even more ><)

@ollieread
Copy link
Author

ollieread commented Nov 6, 2022

To make matters a bit more complicated @orklah, there are multiple traits involved in my specific use case.

There's a BaseCollection class which uses the HasComparator trait. This trait provides the getComparator() and setComparator() methods for all Collections. However, there are 5 classes that are "sorted" variants (SortedCollection, SortedSet, etc, etc), which use a trait called SortsCollection, which overrides both of the previously mentioned methods.

The full hierarchy is:

  • ComparesValues interface defines getComparator and setComparator
  • HasComparator trait implements getComparator and setComparator
  • Collection interface extends ComparesValue.
  • BaseCollection abstract class implements Collection and uses HasComparator
  • SortsCollection trait implements getComparator and setComparator
  • Sorted(Collection|Deque|Queue|Set|Stack) final classes extend BaseCollection and use SortsCollection

@ollieread
Copy link
Author

So, my previous explanation seems wrong. We had a TraitMethodSignatureMismatch issue emitted before PHP 8 and now we have a MethodSignatureMismatch because mismatch are supposed to be more problematic in PHP 8. However, your code clearly doesn't emit a fatal, so this case should be allowed.

That being said, there is no mismatch. Since the return type is static, the returned value will always be the last class in the hierarchy.

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

No branches or pull requests

2 participants