WIP: Introduce @psalm-use to import types

[malukenho]

The implementation is very hacky, just to test what I want to achieve here.

That is a proposal of how we could import types defined in other
classes, so we don't need to repeat the types over and over again.

That could be interesting when composing types as well.

/** @psalm-type TPhone = array{phone: string} */
class Phone {
    /** @psalm-return TPhone */
    toArray(): array {}
}

/** @psalm-type TName = array{name: string} */
class Name {
    /** @psalm-return TName */
    toArray(): array {}
}

/**
 * @psalm-use TPhone from Phone
 * @psalm-use TName from Name
 *
 * @psalm-type TUserArray = TPhone&TName
 */
class User {
    /** @psalm-return TUserArray */
    toArray(): array {}
}

[weirdan]

The specific syntax doesn't make sense though, as typedefs are scoped to files (see #2332 (comment)).

[ragboyjr]

I think it has to be attached per class however otherwise autoloading won't work seamlessly. If we reach a type that hasn't been defined, we know what class it's defined on and can autoload the file to get the definitions.

[mholubowski]

Please make this work, would be very useful. My team does not understand how Psalm is supposed to be used at any scale if you can't define types once and use them throughout your project. Can someone point me in the right direction on this?

[muglug]

My team does not understand how Psalm is supposed to be used at any scale if you can't define types once and use them throughout your project.

Not sure if that's just a lack of imagination on your part. Psalm is used at scale in a bunch of places.

[muglug]

Psalm has two stages - scanning (parsing all the files, collecting information) and analysis (finding bugs).

You can think of the scanning step as performing static reflection – i.e. "what does this method return". That static reflection depends on every docblock having a type that can be easily derived without having to reference any other file.

Type aliases (in the form of already-supported @psalm-type annotations) are currently compatible with that model, as @psalm-type aliases are scoped to the file, and any references to type aliases can be quickly resolved at scanning time on a per-file basis.

If we add this layer of indirection for types, there are two options.

1. Psalm would have to introduce a separate preliminary stage where it just gathered up type definitions. I had to implement a version of this step when I created an experimental Hack to PHP transpiler at the end of 2018. I believe Hack itself does something similar. That extra layer of indirection would take quite a lot of time – it would require a lot of extra scaffolding, and longer Psalm runs, for minimal gain.

2. Add late-resolved type definition – something like TClassTypeAlias – an atomic type similar to TScalarClassConstant that would have to be fleshed out in the TypeExpander. That option would make far more sense, performance-wise.

[muglug]

As for syntax, I think you'd want something like

/**
 * @psalm-import-type CartArray from Cart
 */

as use has a slightly different meaning (and also might be confused with trait uses).

For the late-resolved type definition option:

You'd also need to collect these type_imports in ReflectorVisitor and, everywhere we currently pass type_aliases, we want to also pass these type_imports (preferably as the next argument in the argument list).

---

This is not a trivial task – it's a bit of a big undertaking to get it working reasonably well.

It seems like the main benefit would be for code that has a lot of array shape annotations. It's always been my position that DTOs much less messy than large array shapes, but I also appreciate that some just want to be able to describe their current codebase without having to copy-paste these large docblocks.

Given the 👍 support for this, hopefully someone can start on an implementation. I'm happy to give pointers (like I've given here) but don't want to have to write a lot of code for this. Good luck!

[malukenho]

Thank you for the feedback @muglug! I'll continue with a real implementation for this proposal.

[muglug]

Awesome! I look forward to it!

[muglug]

@malukenho I've added a Psalm\Internal\Type\TypeAlias object which you could use to store the references in the scanning stage – it should prevent you having to deal with too much parsing logic.

I've also added a Psalm\Type\Atomic\TTypeAlias atomic type which is the type equivalent, storing the same data (declaring class an type alias name on that declaring class).

[muglug]

Ok, I've also added docblock parsing (in theory at least) for @psalm-import-type CartArray from Cart.

The only remaining part for you now, I think, is to add support in the TypeExpander for TTypeAlias

[muglug]

Ok so I went ahead and added most of the necessary functionality, such that 0086eb2 works.

There are bunch of possible edge cases though – currently only the optimistic case is supported (where the class and type alias both exist).

If someone could add code to throw InvalidDocblock when the @psalm-import-type statement is malformed, and add tests – any tests – that'd be great.

[mholubowski]

@muglug Thank you for responding to this!

Not sure if that's just a lack of imagination on your part. Psalm is used at scale in a bunch of places.

You're totally correct, and I apologies for my comment coming off the wrong way.

It seems like the main benefit would be for code that has a lot of array shape annotations.

You nailed it! In our case we have a Typescript client with many deeply nested data types, and now we're adding a PHP service that will need to deal with (both take and return) these same types. Here's an example:

 ['status' => array(
                'type' => 'varchar',
                'length' => '50',
                'required' => true,
                'is_enum' => true,
                'enum_options' => [
                    ['paid', 'Paid'],
                    ['unpaid', 'Unpaid'],
                    ['rejected', 'Rejected'],
                ],
                'display_name' => 'Status',
                'form_options' => [
                    'new' => [
                        'show' => true,
                        'validation_type' => ['string', 'required', 'email']
                    ]
                 ],
                'show_list_table_column' => true
            )
]

I'm simply not clear on the best way to currently "feed this shape" to the type system, and reuse it project wide as the input and output of many functions. Would you instead use classes create a class for every 'layer' of nesting?

To reiterate, this is an issue with my lack of understanding and an attempt at learning. 1000 apologies for the ignorant questions, and I do hope that my journey into Psalm is representative of some non-neglible group of developers and this will help solve their speedbumps in the future.

[muglug]

@mholubowski thanks, I appreciate it.

@malukenho I'm going to close this now, as the functionality you've suggested in this PR now exists in Psalm master: https://psalm.dev/r/157a5e9fbb. It's still not entirely fleshed out – there are likely bugs I've missed – but it should be enough for you and others to play with in advance of the release.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/157a5e9fbb

<?php

/** @psalm-type TPhone = array{phone: string} */
class Phone {
    /** @psalm-return TPhone */
    public function toArray(): array {
        return ["phone" => "Nokia"];
    }
}

/** @psalm-type TName = array{name: string} */
class Name {
    /** @psalm-return TName */
    function toArray(): array {
        return ["name" => "Matt"];
    }
}

/**
 * @psalm-import-type TPhone from Phone
 * @psalm-import-type TName from Name
 *
 * @psalm-type TUserArray = TPhone&TName
 */
class User {
    /** @psalm-return TUserArray */
    function toArray(): array {
        return array_merge(
            (new Name)->toArray(),
            (new Phone)->toArray()
        );
    }
}

Psalm output (using commit 111303d):

No issues!

[malukenho]

@muglug I was planning to work on it this weekend. But I can at least try it out. You're awesome! Thank you for your work.

[muglug]

Yeah if you can add tests and failure-handling that’d be awesome