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

InvalidPropertyFetch error emmited on accessing static properties #3609

Closed
emodric opened this issue Jun 18, 2020 · 18 comments
Closed

InvalidPropertyFetch error emmited on accessing static properties #3609

emodric opened this issue Jun 18, 2020 · 18 comments
Labels

Comments

@emodric
Copy link

emodric commented Jun 18, 2020

After upgrading from 3.11.5 to 3.11.6 I started getting InvalidPropertyFetch error on accessing static properties where class name is stored in a variable.

Issue reproducer at https://psalm.dev/r/7bad6b85e8

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/7bad6b85e8
<?php

declare(strict_types=1);

namespace Foo;

class Bar
{
    public static string $foo = 'bar';
}

$class = Bar::class;

echo isset($class::$foo) ? (string) $class::$foo : '';
Psalm output (using commit 137647a):

ERROR: InvalidPropertyFetch - 14:12 - Cannot fetch property on non-object $class of type class-string

@mr-feek
Copy link
Contributor

mr-feek commented Jun 18, 2020

@muglug happy to make a PR if you'd like to provide some guidance :)

@muglug muglug added the bug label Jun 18, 2020
@muglug
Copy link
Collaborator

muglug commented Jun 18, 2020

@mr-feek I think the solution is to narrow this check to only match when $stmt->class has an object (where some $stmt_class_type->hasObjectType()

@muglug
Copy link
Collaborator

muglug commented Jun 18, 2020

so you'll need to run that expression through ExpressionAnalyzer::analyze so the $statements_analyzer->node_data has the correct type

@muglug
Copy link
Collaborator

muglug commented Jun 18, 2020

Actually let me have a go at this, it's a little tricky

@mr-feek
Copy link
Contributor

mr-feek commented Jun 18, 2020

@muglug muglug closed this as completed in 7d9a99a Jun 18, 2020
@muglug
Copy link
Collaborator

muglug commented Jun 18, 2020

Thanks, I realised my suggestion would still break in this scenario:

class Beep {
    public static string $boop = "boop";
}
$beep = rand(0, 1) ? new Beep() : Beep::class;
echo $beep::$boop;

so ended up with something a little more complex

@mr-feek
Copy link
Contributor

mr-feek commented Jun 18, 2020

Thanks @muglug ! Do you consider "hotfix releases" in order to fix regressions so we don't have to wait ~ 2 weeks for the next release?

@emodric
Copy link
Author

emodric commented Jun 19, 2020

A hotfix release would be great, to fix our CI!

Thanks @mr-feek and @muglug for speedy fix 👍

@muglug
Copy link
Collaborator

muglug commented Jun 19, 2020

Yeah I should have a release this weekend

@TysonAndre
Copy link
Contributor

I still see this when the class name is returned by get_class() - https://psalm.dev/r/6fa0f75968

Maybe a different issue type should be emitted for unanalyzable static property fetches when the class is a string?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/6fa0f75968
<?php
class MyClass {
    /** @var string */
    public static $prop = 'value';
}
function test(MyClass $c): void {
    $x = get_class($c);
    var_export($x::$prop);
}
test(new MyClass());
Psalm output (using commit 29eb830):

ERROR: InvalidPropertyFetch - 8:16 - Cannot fetch property on non-object $__fake_var_155 of type string

@emodric
Copy link
Author

emodric commented Jun 23, 2020

This one too still has issues:

https://psalm.dev/r/2835c940a7

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2835c940a7
<?php

class Foo
{
    public static string $bar = 'baz';
}

function getClass(): string
{
    return 'Foo';
}

$class = getClass();

echo (string) $class::$foo;
Psalm output (using commit 9b86021):

ERROR: InvalidPropertyFetch - 15:15 - Cannot fetch property on non-object $__fake_var_148 of type string

@sapphirecat
Copy link

Found this issue because I am seeing this happen on 3.13.1. (In my actual code base, Psalm knows it's of type class-string, but the error on the site is displaying as "of type string". I'm not sure why that happens, or if that's important.)

https://psalm.dev/r/ecc58fd19f

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ecc58fd19f
<?php

class Controller {
    /** @var Array<string,string> $scope */
    public static array $scope = ['getCustomer' => 'read', 'putCustomer' => 'write'];
    public function getCustomer(): void {}
    public function putCustomer(): void {}
}

class Middleware {
    /**
     * @var class-string $controller
     * @var list<string> $grants
     */
    public function validateAccess(string $controller, string $method, array $grants): void {
        if (! isset($controller::$scope)) {
            throw new Exception("Access Denied");
        }
        // ... then check $controller::$scope[$method] vs $grants ...
    }
}

$m = new Middleware();
$m->validateAccess(Controller::class, 'getCustomer', ['read', 'write']);
Psalm output (using commit afce2dc):

ERROR: InvalidPropertyFetch - 16:21 - Cannot fetch property on non-object $__fake_var_464 of type string

@weirdan
Copy link
Collaborator

weirdan commented Aug 6, 2020

but the error on the site is displaying as "of type string". I'm not sure why that happens, or if that's important.)

That's because you need to use @param to annotate parameters, but you used @var instead.

@sapphirecat
Copy link

That's because you need to use @param to annotate parameters, but you used @var instead.

Alas, I should have checked it in PhpStorm first. Thanks. I can confirm my actual code uses @param properly.

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

6 participants