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

class_alias not considered for inheritance checks in type hints #8461

Closed
Istador opened this issue Dec 3, 2022 · 9 comments
Closed

class_alias not considered for inheritance checks in type hints #8461

Istador opened this issue Dec 3, 2022 · 9 comments
Labels

Comments

@Istador
Copy link

Istador commented Dec 3, 2022

Bug report

I'm using class_alias to replace an old implementation in the global namespace (in a bigger older library) with a new and improved implementation from a custom namespace (as its own optional dependency to replace the one in the library). The old global class name is still in use at a lot of places, e.g. in type hints (in other classes in the library and in the application).

Extending the new class and passing an object of it into methods that uses the old class as parameter type hints works at least since PHP 7.3 (I haven't tested earlier versions), but PHPStan wrongly reports this as an error (tested with 1.8.0 and 1.9.2).

Code snippet that reproduces the problem

https://phpstan.org/r/0e405105-3752-4e55-be7c-77258e2d15a4

<?php
class Latest {
	public int $version = 2;
}
if (!class_exists(\Latest::class)) {
	class Deprecated {
		public int $version = 1;
	}
}
\class_alias(\Latest::class, \Deprecated::class);

class Usage {
    public static function use(\Deprecated $ext) : void {
        var_dump($ext->version);
    }
}
class LatestExtension extends \Latest {}
Usage::use(new \LatestExtension);

Expected output

The expected output is int(2) and not to produce an error, but PHPStan with level 5 or higher fails at the last line:

Parameter #1 $ext of static method Usage::use() expects Deprecated, LatestExtension given.

Deprecated is an alias for Latest and LatestExtension is a sub-class of Latest, therefore providing an argument of type LatestExtension should satisfy Deprecated.

Did PHPStan help you today? Did it make you happy in any way?

It helps to find bugs and programming misconceptions before reaching production. It's a great tool to use in CI/CD pipelines as well as locally.

@ondrejmirtes
Copy link
Member

Hi, class aliases need to be registered the way how it's described in the documentation, then it's gonna work: https://phpstan.org/user-guide/discovering-symbols#class-aliases

@Istador
Copy link
Author

Istador commented Dec 4, 2022

Hi, class aliases need to be registered the way how it's described in the documentation, then it's gonna work: https://phpstan.org/user-guide/discovering-symbols#class-aliases

I already tried that out without success, but I now tested that out again more formally in an extra example repository with separate files for each class and composer autoloader.

It now outputs a slightly different output than with the code snippet before. Instead of:

Parameter #1 $ext of static method Usage::use() expects Deprecated, LatestExtension given.

It now fails with:

Parameter #1 $ext of static method Usage::use() expects Latest, LatestExtension given.

Maybe the code snippet tool on phpstan.org isn't good to test this issue out, because there doesn't seem to be a way to provide additional files for bootstrapping code?

Either way. Regardeless of providing the class aliasses with or without bootstrapFiles I get the same error message (though I load the file also via the composer.json autoload, so I think adding it to bootstrapFiles is redundant?):

@Istador
Copy link
Author

Istador commented Dec 13, 2022

@ondrejmirtes Can you reopen this issue or should I create a new issue?

@ondrejmirtes ondrejmirtes reopened this Dec 13, 2022
@ondrejmirtes
Copy link
Member

Yeah, I checked your example and it's really a bug, thanks.

@pretzlaw
Copy link

Friendly reminder that this issue is really nagging when creating compatibility.

Imagine offering some nice tool to the world. This nice tool is using package "foo/bar:6.0" if you run PHP 8.2 . But it requires "foo/bar:5.0" when you use PHP 8.1 .

Then you want to create compatibility for "foo/bar:5" and "foo/bar:6" to make everyone happy but ...
https://phpstan.org/r/f90a397a-fcb9-4713-aeab-ad78e9c682b1

@homersimpsons
Copy link

homersimpsons commented Jan 26, 2024

I think I have this issue, I'm writing some "mocking" classes to allow phpstan to pass safely while I migrate a code base.

I have the following snippet:

// compatibility.php registered in bootstrapFiles
class A {}
class_alias('A', 'MyLib\\A');

// MyClass.php
class B extends MyLib\A {}

function returnsA(): MyLib\A
{
	return new B(); // PHPStan error: Method returnsA() should return A but returns B
}

The above error is invalid as B extends MyLib\A and MyLib\A is A. This is rather strange because in the error message PHPStan resolves the return type to the non-aliased name, but it fails to see that the extends points to the same class.

I fail to reproduce this on the playground as I cannot specify the bootstrapFiles, but here is the snippet: https://phpstan.org/r/db5c125d-8aa3-4950-b142-2f8fb965d740

If this is not the same issue as this one, I can open a new issue.

Thank you for this awesome tool !

@Istador
Copy link
Author

Istador commented Jan 28, 2024

I've looked at it again and this issue seems to be fixed now for me.

Not the example in the playground that I provided, but when providing the class alias in bootstrapFiles (or alternatively via the composer autoloader).

According to my testing (commit) it is working since version 1.10.39 that released on 2023-10-17. That release provided a fix for issue #10023 which appears to me to be a duplicate of this issue.

I also locally tried out the example from @homersimpsons with different phpstan versions and it also seems to be fixed since version 1.10.39 for me.

versions

@Istador Istador closed this as completed Jan 28, 2024
@homersimpsons
Copy link

I also locally tried out the example from @homersimpsons with different phpstan versions and it also seems to be fixed since version 1.10.39 for me

Thanks for that digging @Istador, I'm on 1.10.34 so that may explain why it is still failing on my side.

Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants