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

Using setters with class-string-map #4460

Closed
weirdan opened this issue Oct 30, 2020 · 4 comments
Closed

Using setters with class-string-map #4460

weirdan opened this issue Oct 30, 2020 · 4 comments

Comments

@weirdan
Copy link
Collaborator

weirdan commented Oct 30, 2020

I've been experimenting with undocumented class-string-map type, and I think the following (slight modification of the passing test case) should be type-safe: https://psalm.dev/r/53e6c832e1

Related: #4458 (using a class-string-map there could be a valid solution).

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/53e6c832e1
<?php
                        namespace Bar;
     
                        /**
                         * @psalm-consistent-constructor
                         */
                        class Foo {}
     
                        class A {
                            /** @var class-string-map<T as Foo, T> */
                            public static array $map = [];
     
                            /**
                             * @template T as Foo
                             * @param class-string<T> $class
                             * @return T
                             */
                            public function get(string $class) : Foo {
                                if (isset(self::$map[$class])) {
                                    return self::$map[$class];
                                }
                                throw new \RuntimeException();
     

                            }
                            
                            /** @param class-string<Foo> $class */
                            public function set(string $class): void {
                                self::$map[$class] = new $class();
                            }
                        }
Psalm output (using commit 579327a):

ERROR: InvalidPropertyAssignmentValue - 29:33 - Bar\A::$map with declared type 'class-string-map<T as Bar\Foo, T:class-string-map as Bar\Foo>' cannot be assigned type 'non-empty-array<class-string<Bar\Foo>|(class-string<T:class-string-map as Bar\Foo>), Bar\Foo|(T:class-string-map as Bar\Foo)>'

@muglug
Copy link
Collaborator

muglug commented Oct 30, 2020

Yeah, when I added class-string-map I said at the time that I wouldn't fix any new issues with it. If you want to have a go here, feel free, but this looks pretty complex.

@ptomulik
Copy link
Contributor

This is interesting:

https://psalm.dev/r/f8a3440139

using if(isset($this->instances[$key])){} somehow makes psalm to not emit the error

@psalm-github-bot
Copy link

I found these snippets:

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

class Container
{
    /**
     * @psalm-var class-string-map<T, T>
     */
    private array $instances = [];

    /**
     * @psalm-template U of object
     * @psalm-param class-string<U> $key
     * @psalm-param U $instance
     */
    public function set(string $key, object $instance): void
    {
        if(isset($this->instances[$key])) {} // <- !!!
        $this->instances[$key] = $instance;
    }
}
Psalm output (using commit c0e757c):

No issues!

ptomulik added a commit to ptomulik/psalm that referenced this issue Feb 26, 2023
ptomulik added a commit to ptomulik/psalm that referenced this issue Feb 26, 2023
ptomulik added a commit to ptomulik/psalm that referenced this issue Feb 27, 2023
ptomulik added a commit to ptomulik/psalm that referenced this issue Feb 27, 2023
ptomulik added a commit to ptomulik/psalm that referenced this issue Feb 28, 2023
@orklah orklah closed this as completed in 78e6545 Feb 28, 2023
orklah added a commit that referenced this issue Feb 28, 2023
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

4 participants