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

@var MyTrait $mockForTrait causes UndefinedDocblockClass #3695

Open
caugner opened this issue Jun 26, 2020 · 21 comments
Open

@var MyTrait $mockForTrait causes UndefinedDocblockClass #3695

caugner opened this issue Jun 26, 2020 · 21 comments

Comments

@caugner
Copy link
Contributor

caugner commented Jun 26, 2020

When testing traits using phpUnits' TestCase::getMockForTrait() I would like to have IDE autocompletions.

However, type hinting the mock with the Trait "class" causes Psalm to complain:

https://psalm.dev/r/8d489615b1

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/8d489615b1
<?php

trait MyTrait {
    public function foo(): void
    {}
}

class FakeTrait {
    use MyTrait;
}

/**
  * @return mixed
  */
function getMockForTrait(string $traitName)
{
    if (!$traitName) {
        throw new RuntimeException();
    }
    
    return new FakeTrait();
}

/**
  * This type hint allows my IDE to auto-complete:
  * @var MyTrait $trait
  */
$trait = getMockForTrait(MyTrait::class);
$trait->foo();
Psalm output (using commit c95ebfe):

ERROR: UndefinedDocblockClass - 28:1 - Docblock-defined class or interface MyTrait does not exist

ERROR: UndefinedDocblockClass - 29:1 - Docblock-defined class or interface MyTrait does not exist

@caugner
Copy link
Contributor Author

caugner commented Jun 26, 2020

Also, @psalm-suppressing only suppresses the error for the initial type hint, not for the method call (but maybe this is intended?):

https://psalm.dev/r/ba5df0544b

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ba5df0544b
<?php

trait MyTrait {
    public function foo(): void
    {}
}

class FakeTrait {
    use MyTrait;
}

/**
  * @return mixed
  */
function getMockForTrait(string $traitName)
{
    if (!$traitName) {
        throw new RuntimeException();
    }
    
    return new FakeTrait();
}

/**
  * This type hint allows my IDE to auto-complete:
  * @psalm-suppress UndefinedDocblockClass
  * @var MyTrait $trait
  */
$trait = getMockForTrait(MyTrait::class);
$trait->foo();
Psalm output (using commit c95ebfe):

ERROR: UndefinedDocblockClass - 30:1 - Docblock-defined class or interface MyTrait does not exist

@sanmai
Copy link
Contributor

sanmai commented Sep 29, 2021

Another example: https://psalm.dev/r/b033c99c11

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/b033c99c11
<?php

interface Foo {
	function bar(): void;
    
    /** @return Foo[] */
	function getFoo(): array;
}

trait FooExtra {
 	function bar(): void {
    	// Doin' something
    }
}

class ConcreteFoo implements Foo {
    use FooExtra;
    
    function getFoo(): array {
     	return [new ConcreteFoo(), new ConcreteFoo2()];
    }    
}

class ConcreteFoo2 implements Foo {
    use FooExtra;
        
    function getFoo(): array {
     	return [];
    }
}


$foo1 = new ConcreteFoo();

foreach ($foo1->getFoo() as $foo2) {
    /** @var FooExtra $foo2 */
 	$foo2->bar();
}
Psalm output (using commit 5ac21dd):

ERROR: UndefinedDocblockClass - 37:3 - Docblock-defined class, interface or enum named FooExtra does not exist

@Radiergummi
Copy link

I have another use case for this; in Laravel, Notification classes will generally be invoked with an instance of a model that uses the Notifiable trait, like so:

class MyNotification extends Notification
{
    public function toMail(Notifiable $recipient): MailMessage
    {
        // ...
    }
}

Alas, Notifiable is not an interface, and there is none for it. I'm not a huge Laravel fan, but that's what we have to deal with anyway.
Due to this issue, the parameter cannot be type-hinted accurately.

@orklah orklah added the traits label Nov 4, 2021
@orklah
Copy link
Collaborator

orklah commented Nov 4, 2021

This issue is emitted here:

new UndefinedDocblockClass(

If you look a few lines above, there's this:

if (!$options->allow_trait || !$codebase->classlikes->traitExists($fq_class_name, $code_location)) {

who looks like it could allow traits if a specific flag is true. If someone wants to try to make progress on this, I guess a good start would be to check try to toggle this flag to true and check if this could work.

I won't work on this because I don't use traits much and I'm very not motivated to work with them, but I'll offer help if I can to anyone who wants to try

@ricardoboss
Copy link
Contributor

I experimented a bit in https://github.com/ricardoboss/psalm/tree/issue/3695-trait-in-docblock-error and it seems to be more complex than I initially thought. The only changes include allowing traits when checking for a class like type in a docblock and for a method call target (i.e. what @orklah proposed).

A workaround is to just give Psalm what it wants: an interface to check the method calls against: https://psalm.dev/r/8067be27b9

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/8067be27b9
<?php

trait MyTrait {
    public function foo(): void
    {}
}

interface MyTraitInterface {
    public function foo(): void;
}

class FakeTrait {
    use MyTrait;
}

/**
  * @return mixed
  */
function getMockForTrait(string $traitName)
{
    if (!$traitName) {
        throw new RuntimeException();
    }
    
    return new FakeTrait();
}

/**
  * This type hint allows my IDE to auto-complete:
  * @var MyTraitInterface $trait
  */
$trait = getMockForTrait(MyTrait::class);
$trait->foo();
Psalm output (using commit eca56c0):

No issues!

@orklah
Copy link
Collaborator

orklah commented Nov 22, 2021

I tried myself and it comes to: https://github.com/orklah/psalm/tree/traits2

There's one failure in tests:
https://psalm.dev/r/72d6bd8836

After the changes, this doesn't emit an error anymore. Does anyone know why this code is bad?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/72d6bd8836
<?php
                    trait T {}

                    class X {
                      /** @var T|null */
                      public $hm;
                    }
Psalm output (using commit eca56c0):

ERROR: UndefinedDocblockClass - 5:32 - Docblock-defined class, interface or enum named T does not exist

@ricardoboss
Copy link
Contributor

@orklah it is bad because then you would be able to use traits as property types, as the testsuites indicate: https://github.com/orklah/psalm/runs/4292943773?check_suite_focus=true & https://github.com/orklah/psalm/runs/4292945923?check_suite_focus=true#step:7:241

@weirdan
Copy link
Collaborator

weirdan commented Nov 22, 2021

Traits are not types, they do not provide any guaranties as their methods may be aliased, shadowed by another trait and their visibility can be changed.

@orklah
Copy link
Collaborator

orklah commented Nov 22, 2021

@weirdan so, by that logic, the original example should stay an error and Psalm is just doing its job?

@weirdan
Copy link
Collaborator

weirdan commented Nov 22, 2021

That would be my take, yes.

@orklah
Copy link
Collaborator

orklah commented Nov 22, 2021

Okay, I'll let this open for now, it seem to be a popular issue. We should at least try to formalize this in the documentation before closing. I'll change the labels though.

@weirdan
Copy link
Collaborator

weirdan commented Nov 22, 2021

To provide a concrete example, the C class in the following snippet does not implement neither T1 nor T2 'interfaces': https://3v4l.org/soWld

@ricardoboss
Copy link
Contributor

After checking if an object uses a specific trait (class_uses) we should be able to give Psalm some kind of type hint.

I imagine something like an intersection type could work: https://psalm.dev/r/77767bde76

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/77767bde76
<?php

trait T1 {
    public function foo(): int { return 1; }
    public function bar(): string { return 'z'; }
}

trait T2 {
    public function foo(): float { return M_PI; }
    public function bar(): array { return [42]; }
}


class C {
    use T1, T2 { 
        T1::foo insteadof T2;
        T2::bar insteadof T1;
    }
}

$c = new C();

/** @var C&T1 $c */
echo $c->foo() === 1;
Psalm output (using commit eca56c0):

ERROR: UndefinedDocblockClass - 24:1 - Docblock-defined class, interface or enum named T1 does not exist

ERROR: UndefinedDocblockClass - 24:6 - Docblock-defined class, interface or enum named T1 does not exist

@weirdan
Copy link
Collaborator

weirdan commented Nov 22, 2021

&T1 (if T1 is treated as a type) implies foo() will return int and bar() will return string.

@ricardoboss
Copy link
Contributor

Correct, but in a perfect world, the whole docblock would be unnecessary. Maybe the @var annotation isn't sufficient for declaring what method signature of a specific trait should be used for type checking...

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

6 participants