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

Feature: Psalm pseudo-type for instantiable-object #6075

Open
boesing opened this issue Jul 12, 2021 · 7 comments
Open

Feature: Psalm pseudo-type for instantiable-object #6075

boesing opened this issue Jul 12, 2021 · 7 comments
Assignees
Labels

Comments

@boesing
Copy link
Contributor

boesing commented Jul 12, 2021

Hey there,

not sure if thats already possible but is there a way to provide something like this?

https://psalm.dev/r/16e0076766

Foo::class is the Interface and thus matches the requirements of the template. But it wont be instantiable because its the interface and thus leads to runtime errors.

What I want to have is something like:

/**
 * @template T of instantiable<Foo>
 */

So instantiable would match for class names which provide either final __construct or anything else which is already verified by UnsafeInstantiation.

Is this something which could be achieved?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/16e0076766
<?php

interface Foo
{
}

/**
 * @template T of Foo
 */
final class Bar
{
    /**
     * @psalm-var class-string<T>
     */
    private string $bar;
    
    /**
     * @psalm-param class-string<T> $bar
     */
    public function __construct(string $bar)
    {
        $this->bar = $bar;
    }
    
    /**
     * @psalm-return T
     */
    public function createInstance(): object
    {
        $className = $this->bar;
    	return new $className;
    }
}

$factory = new Bar(Foo::class);
$factory->createInstance();
Psalm output (using commit e93b37a):

No issues!

@weirdan
Copy link
Collaborator

weirdan commented Jul 12, 2021

This actually looks like a bug to me. Not only because we don't know whether the class-string is instantiable, but also because constructor signature is unknown. Even if it's instantiable, it may have required arguments and would still fail at runtime.

@weirdan weirdan added the bug label Jul 12, 2021
@muglug
Copy link
Collaborator

muglug commented Jul 13, 2021

Yeah, in retrospect making interface names pass a class-string check was a mistake. I think that could be a major-version change.

@muglug
Copy link
Collaborator

muglug commented Jul 14, 2021

actually it probably doesn't need to be a major-version change

@muglug
Copy link
Collaborator

muglug commented Jul 14, 2021

This is quite a big refactor, I'll have a look at it this week

@muglug muglug self-assigned this Jul 14, 2021
@muglug
Copy link
Collaborator

muglug commented Jul 14, 2021

Actually this is more complex. I’m afraid introducing the change before a major version would induce gnashing of teeth.

I foresee this change happening in two steps: the first very minor step makes interface-string<X> a synonym for class-string<X>. We should ask library authors to add interface-string<Foo> wherever interface strings are acceptable (e.g in templates assertions) for forward-compatibility.

This will help prepare for the next step, which would happen in a major version change, changing the meaning of class-string to mean only classes.

@boesing
Copy link
Contributor Author

boesing commented Jul 15, 2021

Sounds reasonable and I'd be fine with that.

Shall we move this to a dedicated issue instead? This issue was initially meant to be a feature request which tells psalm that only classes are accepted which are safely instantiable without any __construct arguments.

So one can simply do new $className without being afraid that it crashes.

But I see the pitfalls, so maybe keeping it to track (even for me) so this can be probably achieved later on. But I'd be fine with closing this as wont do. But to have a clear issue ticket regarding the class-string, interface-string problematic would probably be more clear? What do you think?

muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
muglug added a commit that referenced this issue Jul 15, 2021
Fixes #6075 by preventing that class of potential errors from interface strings
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

3 participants