WIP - Limit inheritance to a subset of classes #1450

[robchett]

Going a little out of my depth with this one but giving it a shot.

Only covering the first section, not the match cases.

[robchett]

Will cover Interfaces/Traits once I've got Classes sorted.

[robchett]

@orklah can I get some feedback on an preliminary attempt at implementing #1450?

[orklah]

Sorry for the slow response :(

[robchett]

I've been thinking about the second part of #1450

function f1(BaseClass $c) : string {
  switch (get_class($c)) {
    case FooClass:class:
      return $c->thing("me");
    case BarClass:class:
      return $c->thing(5);
  }
}

function f2(BaseClass $c) : string { // this is an error as not all paths are met
  switch (get_class($c)) {
    case FooClass:class:
      return $c->thing("me");
  }
}

For get_class to work for this then @psalm-inheritors would have to be limited to direct decedents?

/**
 * @psalm-inheritors FooClass
 */
class BaseClass {}
class FooClass extends BaseClass {}
class BarClass extends FooClass {} // Is this an error?

Here BarClass fullfils the requirement of being a FooClass so unless FooClass was final there could be any number of outputs of get_class(). also unless BaseClass was abstract or an interface then BaseClass::class would also need to be in the cases.

We'd still be able to do this:

/**
 * @psalm-inheritors FooClass
 */
abstract class BaseClass {}

function f1(BaseClass $c) : string {
    return match (true) {
        $c instanceof FooClass => $c->thing("me"),
        $c instanceof BarClass => $c->thing(5),
    };
}

function f2(BaseClass $c) : string {
    return match (true) {
        $c instanceof FooClass => $c->thing("me"),
        // error, missing branch BarClass 
    };
}

[orklah]

Seems ready to merge apart from the typo! Again, sorry for the delay, reviewing new features always takes me more time so I need to have some free time dedicated to that :) Your PR are very much appreciated :)

About the part with the switch, I agree, it seems dangerous to assume we know all the classes (when they're not all final) and even then it's not that much interesting to check for switch/match completeness

[robchett]

Typo fixed.
No worries on the "delay", a new feature from a 4y old issue is hardly a priority.
Thank you for all your help with this, I understand the internals much better now.

[orklah]

Oh, I didn't notice before but there are failures on CI. One of them is due to this file: https://github.com/vimeo/psalm/blob/master/config.xsd

It should be updated to include InheritorViolation

This file too: https://github.com/vimeo/psalm/blob/master/docs/running_psalm/issues.md

And finally, your doc example in supported_annotations.md should include a <?php tag to prevent the parse error

[orklah]

This brace is the reason behind the last failure it seems

[orklah]

Thanks!

[vudaltsov]

This is cool, thank you!