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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typing a dependency injection container #4310

Closed
ChristophWurst opened this issue Oct 12, 2020 · 11 comments
Closed

Typing a dependency injection container #4310

ChristophWurst opened this issue Oct 12, 2020 · 11 comments
Labels

Comments

@ChristophWurst
Copy link

Hello 馃憢,

I'm in the process of rolling out Psalm for more components of the Nextcloud ecosystem and I came to the following problem. If you have a dependency injection container, it would be really cool to let Psalm know the container returns an instance of the class string passed as identifier, though containers might also work with regular strings as service identifications.

Sample code: https://psalm.dev/r/f4113ff8b9

If I limit the container interface to only work with class strings, then the typing is easy: https://psalm.dev/r/b86ac518bd. But in practice you have some named services where the string is not actually a class string but just a literal.

Does Psalm automatically use the class string as more specific type? It looks like the union type with string has no effect except eliminating the error Argument 1 of f expects class-string, parent type string(...) provided. Therefore it still complains about every service that is queried by name instead of class name like Class or interface hello does not exist in the example above..

Unfortunately I could not find any other container implementation that uses psalm typing.

Are there any tricks to make this work?

What I'm trying to tell Psalm is

  • If it's a class name then an instance of T is returned
  • Fall back to mixed in any other case
@psalm-github-bot
Copy link

I found these snippets:

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

class A {
    public function foo(): void {}
}

/**
 * @template T
 * @param class-string<T>|string $name
 * @return T|mixed
 */
function f(string $name) {
        /** @var T $obj */
        $obj = "fixme";
        return $obj;
}

$a = f(A::class);
$a->foo();

$b = f('hello');
Psalm output (using commit 903d7f3):

INFO: UnusedParam - 12:19 - Param $name is never referenced in this method

INFO: MixedAssignment - 18:1 - Unable to determine the type that $a is being assigned to

INFO: MixedMethodCall - 19:5 - Cannot determine the type of $a when calling method foo

ERROR: UndefinedClass - 21:8 - Class or interface hello does not exist

INFO: MixedAssignment - 21:1 - Unable to determine the type that $b is being assigned to

INFO: UnusedVariable - 21:1 - Variable $b is never referenced
https://psalm.dev/r/b86ac518bd
<?php

class A {
    public function foo(): void {}
}

/**
 * @template T
 * @param class-string<T> $name
 * @return T
 */
function f(string $name) {
        /** @var T $obj */
        $obj = "fixme";
        return $obj;
}

$a = f(A::class);
$a->foo();
Psalm output (using commit 903d7f3):

INFO: UnusedParam - 12:19 - Param $name is never referenced in this method

@EvgeniiR
Copy link
Contributor

EvgeniiR commented Oct 12, 2020

Looks like conditional types( https://psalm.dev/docs/annotating_code/type_syntax/conditional_types/ ) should help here, but I'm not sure how to correctly tell psalm to allow just string as param https://psalm.dev/r/fb3073f1f0

Upd: but it works well if pass an argument as a variable https://psalm.dev/r/6cee64fd5e
Is this a bug similar to what happened with callables ( #3653 )?

@psalm-github-bot
Copy link

psalm-github-bot bot commented Oct 12, 2020

I found these snippets:

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

/**
 * @template T
 * @param string|class-string<T> $name
 * @return (T is object ? T : mixed)
 */
function get(string $name) {
    return;
}

get('abc');
Psalm output (using commit b85cbd0):

INFO: UnusedParam - 8:21 - Param $name is never referenced in this method

ERROR: UndefinedClass - 12:5 - Class or interface abc does not exist
https://psalm.dev/r/6cee64fd5e
<?php

/**
 * @template T
 * @param string|class-string<T> $name
 * @return (T is object ? T : mixed)
 */
function get(string $name) {
    return;
}

/** @var string $str */
get($str);

/** @var class-string $class */
get($class);
Psalm output (using commit b85cbd0):

INFO: UnusedParam - 8:21 - Param $name is never referenced in this method

@ChristophWurst
Copy link
Author

Thanks a lot for the tip with the conditional types! I learned something new today :)

but it works well if pass an argument as a variable https://psalm.dev/r/6cee64fd5e
Is this a bug similar to what happened with callables

Indeed it works with variables but problems show as soon as literals are used: https://psalm.dev/r/5008a6ddb5.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/6cee64fd5e
<?php

/**
 * @template T
 * @param string|class-string<T> $name
 * @return (T is object ? T : mixed)
 */
function get(string $name) {
    return;
}

/** @var string $str */
get($str);

/** @var class-string $class */
get($class);
Psalm output (using commit eeacec3):

INFO: UnusedParam - 8:21 - Param $name is never referenced in this method
https://psalm.dev/r/5008a6ddb5
<?php

/**
 * @template T
 * @param string|class-string<T> $name
 * @return (T is object ? T : mixed)
 */
function get(string $name) {
    return;
}

/** @var string $str */
get($str);

get('name');

/** @var class-string $class */
get($class);
Psalm output (using commit eeacec3):

INFO: UnusedParam - 8:21 - Param $name is never referenced in this method

ERROR: UndefinedClass - 15:5 - Class or interface name does not exist

@orklah
Copy link
Collaborator

orklah commented Oct 12, 2020

This seem to work better:
https://psalm.dev/r/b4cdb6c683

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/b4cdb6c683
<?php
class name{}
/**
 * @param string|class-string $name
 * @return ($name is class-string ? object : mixed)
 */
function get(string $name) {
    return;
}

$str = 'a';
/** @var class-string $class */
$class = 'b';

/** @psalm-trace $a */
$a = get($str);

/** @psalm-trace $b */
$b = get('name');

/** @psalm-trace $c */
$c = get(name::class);

/** @psalm-trace $d */
$d = get($class);
Psalm output (using commit eeacec3):

INFO: UnusedParam - 7:21 - Param $name is never referenced in this method

INFO: MixedAssignment - 16:1 - Unable to determine the type that $a is being assigned to

INFO: Trace - 16:1 - $a: mixed

INFO: MixedAssignment - 19:1 - Unable to determine the type that $b is being assigned to

INFO: Trace - 19:1 - $b: mixed

INFO: Trace - 22:1 - $c: object

INFO: Trace - 25:1 - $d: object

INFO: UnusedVariable - 16:1 - Variable $a is never referenced

INFO: UnusedVariable - 19:1 - Variable $b is never referenced

INFO: UnusedVariable - 22:1 - Variable $c is never referenced

INFO: UnusedVariable - 25:1 - Variable $d is never referenced

@ChristophWurst
Copy link
Author

This seem to work better:

Thanks for looking into it but then there is no type information on the returned object. Psalm will treat it as object and not an instance of the type that is passed as argument. Which means I need to add a typedoc for every usage of the container method.

But with this new hint of $name is class-string I got it to: https://psalm.dev/r/30724e7f25. So from that snippet I would say that the conditional type does not work as expected. But I'll keep digging and experimenting.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/30724e7f25
<?php
class A {
	public function foo(): void  {}
}

/**
 * @template T
 * @param string|class-string<T> $name
 * @return ($name is class-string ? T : mixed)
 */
function get(string $name) {
    return;
}

$str = 'a';
/** @var class-string $class */
$class = 'b';

/** @psalm-trace $a */
/** @var string $a */
$a = get($str);

/** @psalm-trace $b */
$b = get(A::class);
$b->foo();

/** @psalm-trace $c */
$c = get('Literal');

/** @psalm-trace $d */
$d = get($class);
Psalm output (using commit 7cf6495):

INFO: UnusedParam - 11:21 - Param $name is never referenced in this method

INFO: Trace - 24:1 - $b: A

ERROR: UndefinedClass - 28:10 - Class or interface Literal does not exist

INFO: MixedAssignment - 28:1 - Unable to determine the type that $c is being assigned to

INFO: Trace - 28:1 - $c: mixed

INFO: Trace - 31:1 - $d: object

INFO: UnusedVariable - 21:1 - Variable $a is never referenced

INFO: UnusedVariable - 28:1 - Variable $c is never referenced

INFO: UnusedVariable - 31:1 - Variable $d is never referenced

@muglug muglug added the bug label Oct 12, 2020
@muglug muglug closed this as completed in fcfa746 Oct 12, 2020
@ChristophWurst
Copy link
Author

@muglug you're amazing. Thanks a lot for the quick fix 馃檶.

@muglug
Copy link
Collaborator

muglug commented Oct 12, 2020

You're welcome!

danog pushed a commit to danog/psalm that referenced this issue Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants