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.

Sign up for GitHub

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

Jump to bottom

Issue with template inheritance and overriding method signatures #10010

Closed
wants to merge 1 commit into from
Closed

Issue with template inheritance and overriding method signatures #10010

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions tests/Template/ClassTemplateTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4207,6 +4207,68 @@ public function aggregate(...$values): null|int|float
'ignored_issues' => [],
'php_version' => '8.0',
],
'inheritedConditionalsWithMatchingTypes' => [
'code' => '<?php
/** @template InstanceType */
interface PluginManagerInterface
{
/**
* @template TRequestedInstance extends InstanceType
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we support extends here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it works flawless if I do not overwrite the method in the abstract class. the issue is adding the object return type which makes it less specific.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I confirm we don't do anything with the extends keyword so it's just comments as far as Psalm is concerned

Copy link
Contributor Author

@boesing boesing Jul 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but it actually seems to work until I add the overridden method with the object return type 🤔 or its just a coincdedence as I do not verify some where if that is actively validated.

So to implement this, I have to focus the extends?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you show an example where psalm behaves differently based on the extends?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I cant. I just know that the way psalm behaves differs once I extend the get method and narrow the type from mixed to object

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you post two snippets so we see the difference exactly? I'm still a bit confused

Copy link
Contributor Author

@boesing boesing Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://psalm.dev/r/e80a63390b
https://psalm.dev/r/442b189bf4

But I am pretty sure that it does not respect the extends within the template annotation in method scope but not sure if that is the underlying problem tho.

Copy link
Contributor Author

@boesing boesing Aug 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I've digged a bit deeper and found out that there is a general issue with @inheritdoc as due to this, templates are completely dropped and therefore the whole type detection seems to be problematic when it comes to assertions.

https://psalm.dev/r/538a0d30d4

I am totally lost here, but thats maybe due to the fact that I spent too much time on other psalm related stuff in the past days.
Would really love to get some help on this but will also try to solve this on my own in the next weeks.

* @param class-string<TRequestedInstance>|string $id
* @return ($id is class-string ? TRequestedInstance : InstanceType)
* @throws InvalidArgumentException
*/
public function get(string $id): mixed;
}

interface PluginInterface
{}

class ConcretePlugin implements PluginInterface
{}

/**
* @template InstanceType
* @template-implements PluginManagerInterface<InstanceType>
*/
abstract class AbstractPluginManager implements PluginManagerInterface
{
public function get(string $id): mixed
{
throw new InvalidArgumentException();
}
}

/**
* @template InstanceType of object
* @template-extends AbstractPluginManager<InstanceType>
*/
abstract class AbstractSingleInstancePluginManager extends AbstractPluginManager
{
/**
* {@inheritDoc}
*/
public function get(string $id): object
{
return parent::get($id);
}
}

/** @template-extends AbstractSingleInstancePluginManager<PluginInterface> */
class ConcretePluginManager extends AbstractSingleInstancePluginManager
{}

$plugins = new ConcretePluginManager();
$classString = $plugins->get(ConcretePlugin::class);
$string = $plugins->get("foo");
',
'assertions' => [
'$classString' => 'ConcretePlugin',
'$string' => 'PluginInterface',
],
'ignored_issues' => [],
'php_version' => '8.0',
],
];
}

Expand Down
Loading