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

Psalm does not understand variable initialization order when initialized within array. #6061

Closed
simPod opened this issue Jul 8, 2021 · 4 comments

Comments

@simPod
Copy link
Contributor

simPod commented Jul 8, 2021

https://psalm.dev/r/d4185292b9

I'd expect not errors about variable $timestamp being not found or unused.

@psalm-github-bot
Copy link

I found these snippets:

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

[
    $timestamp = (new DateTimeImmutable('2017-08-06 00:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ],
    $timestamp = (new DateTimeImmutable('2017-08-06 01:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ],
    $timestamp = (new DateTimeImmutable('2017-08-06 02:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ]
];
Psalm output (using commit 0257752):

ERROR: UndefinedGlobalVariable - 5:24 - Cannot find referenced variable $timestamp in global scope

ERROR: InvalidArrayOffset - 3:1 - Cannot create offset of type false|int, expecting array-key

INFO: UnusedVariable - 4:5 - $timestamp is never referenced or the value is not used

INFO: UnusedVariable - 7:5 - $timestamp is never referenced or the value is not used

INFO: UnusedVariable - 10:5 - $timestamp is never referenced or the value is not used

@muglug
Copy link
Collaborator

muglug commented Jul 9, 2021

Few comments:

  • IMO this is bad code and should be rewritten as https://psalm.dev/r/366293c0e1
  • the bug is caused by evaluation order — currently all keys are evaluated first, which is wrong. It should be possible instead to do it by key and value

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/366293c0e1
<?php

$dates = ['2017-08-06 00:00:00', '2017-08-06 01:00:00', '2017-08-06 02:00:00'];

$arr = [];

foreach ($dates as $date) {
    $timestamp = (new DateTimeImmutable($date))->getTimestamp();
    $arr[$timestamp] = ['timestamp' => $timestamp];
}
Psalm output (using commit f94f3b8):

No issues!

@simPod
Copy link
Contributor Author

simPod commented Jul 12, 2021

Definitely agree the code is not perfect though it's still valid or is it not?

Stumbled upon it in legacy code base where there's complex array structure in tests and the lowest level array is constructed like this. Hard to squeeze foreach inside. I would have to wrap it in a function, call it and unpack ... or sth like that.

[22=>[
    $timestamp = (new DateTimeImmutable('2017-08-06 00:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ],
    $timestamp = (new DateTimeImmutable('2017-08-06 01:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ],
    $timestamp = (new DateTimeImmutable('2017-08-06 02:00:00'))->getTimestamp() => [
        'timestamp' => $timestamp,
    ]
]];

->

<?php

[22=>(function() {
    $dates = ['2017-08-06 00:00:00', '2017-08-06 01:00:00', '2017-08-06 02:00:00'];

    $arr = [];

    foreach ($dates as $date) {
        $timestamp = (new DateTimeImmutable($date))->getTimestamp();
        $arr[$timestamp] = ['timestamp' => $timestamp];
    }

    return $arr;
})()]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants