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

Do not invalidate cache during suppression #9936

Closed
wants to merge 1 commit into from
Closed

Do not invalidate cache during suppression #9936

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
4 changes: 1 addition & 3 deletions src/Psalm/Internal/Cli/Psalm.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,9 +421,7 @@ private static function initShowInfo(array $options): bool

private static function initIsDiff(array $options): bool
{
return !isset($options['no-diff'])
&& !isset($options['set-baseline'])
&& !isset($options['update-baseline']);
return !isset($options['no-diff']);
Copy link
Contributor Author

@ro0NL ro0NL Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

            && !isset($options['set-baseline'])
            && !isset($options['update-baseline']);

i cant really rationalize this. i feel like it hacks the caching for whatever purpose. IMHO then --no-diff should be passed explicitly (ref #7681)

Copy link
Collaborator

Choose a reason for hiding this comment

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

I fear Psalm may be unable to refresh or recreate the baseline unless it has analyzed your whole codebase. It would make sense at least. So whenever your ask it to do that, it can't just analyze files that changed or were removed and launch the whole analysis.

You may have to choose between having a squeaky clean baseline and a fast analysis

Copy link
Contributor Author

@ro0NL ro0NL Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

im not sure i follow, it seems psalm is able to recreate the baseline from cache just fine 🤔

i can confirm after rm psalm-baseline.xml psalm regenerates the same baseline file using vendor/bin/psalm --set-baseline=psalm-baseline.xml

Copy link
Contributor Author

@ro0NL ro0NL Jun 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

i can also confirm after rm psalm-baseline.xml AND intentionally forcing an error (referenced an unknown class in src/ somewhere), the baseline is updated as expected

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 can also confirm after fixing that error i get Baseline for issue "UndefinedClass" has 1 extra entry. (see https://psalm.dev/316) AND it's removed from the baseline file

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's two things that can reduce the time of Psalm's analysis:

  • the cache (we store parsed files and also store method/class definitions)
  • the diff option (Psalm will only check changed files instead of analyzing everything again)

Both can be controlled through --no-cache and --no-diff (though I think --no-cache implies --no-diff).

I think you may have misinterpreted Psalm using the --no-diff mode when you ask it to regenerate baseline vs Psalm using no cache at all.

Could you make a few tests with --no-diff, --no-cache and --set-baseline to see if it matches your expectations?

}

/**
Expand Down