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 cache bug with constructor property promotion #10080

Closed
ging-dev opened this issue Aug 3, 2023 · 5 comments · Fixed by #10228
Closed

psalm cache bug with constructor property promotion #10080

ging-dev opened this issue Aug 3, 2023 · 5 comments · Fixed by #10228
Assignees

Comments

@ging-dev
Copy link
Contributor

ging-dev commented Aug 3, 2023

<?php

class Example
{
    public function __construct(private string $error)
    {
    }

    public function other(): string
    {
        return $this->error;
    }
}

run psalm with cache. After, change content to:

<?php

class Example
{
    public function __construct()
    {
    }

    public function other(): string
    {
        return $this->error;
    }
}

re-run psalm, psalm doesn't show an error message, but an error actually exists

@psalm-github-bot
Copy link

Hey @ging-dev, can you reproduce the issue on https://psalm.dev ?

@weirdan
Copy link
Collaborator

weirdan commented Aug 3, 2023

Can you provide a reproducer repo? I tried this in Psalm's own repo, and errors were reported.

@ging-dev
Copy link
Contributor Author

ging-dev commented Aug 3, 2023

Can you provide a reproducer repo? I tried this in Psalm's own repo, and errors were reported.

https://github.com/ging-dev/psalm-cache-reproduce

Here is video i reproduced it:

2023-08-03.16-26-34.mp4

@weirdan weirdan self-assigned this Aug 3, 2023
@weirdan
Copy link
Collaborator

weirdan commented Aug 3, 2023

Relevant parts from the debug log:

$ vendor/bin/psalm --debug 
Process plugin adjustments...
Target PHP version: 8.2 (inferred from current PHP version).
Scanning files...
1 changed files:
    /home/weirdan/src/psalm/issues/psalm-cache-reproduce/src/Bug/Reproduce.php
[.....]
Parsing /home/weirdan/src/psalm/issues/psalm-cache-reproduce/src/Bug/Reproduce.php
Deep scanning /home/weirdan/src/psalm/issues/psalm-cache-reproduce/src/Bug/Reproduce.php
[.....]
Analyzing files...
Getting /home/weirdan/src/psalm/issues/psalm-cache-reproduce/src/Bug/Reproduce.php
Analyzing /home/weirdan/src/psalm/issues/psalm-cache-reproduce/src/Bug/Reproduce.php
Skipping analysis of pre-analyzed method App\Bug\Reproduce::error
[.....]

@weirdan
Copy link
Collaborator

weirdan commented Aug 3, 2023

Changes to __construct() should cause error() to be reanalyzed, and it should be triggered somewhere around here:

if (isset($all_referencing_methods[$member_id])) {
$newly_invalidated_methods = array_merge(
$all_referencing_methods[$member_id],
$newly_invalidated_methods,
);
}

ging-dev added a commit to ging-dev/psalm that referenced this issue Sep 25, 2023
ging-dev added a commit to ging-dev/psalm that referenced this issue Sep 25, 2023
@ging-dev ging-dev mentioned this issue Sep 25, 2023
ging-dev added a commit to ging-dev/psalm that referenced this issue Sep 27, 2023
ging-dev added a commit to ging-dev/psalm that referenced this issue Oct 16, 2023
ging-dev added a commit to ging-dev/psalm that referenced this issue Oct 16, 2023
orklah added a commit that referenced this issue Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants