Please sync with changes I made in #40311

Note that on my side, I don't see the benefit of committing this file since it can be generated.
We'd better document how to generate it somewhere if we really want this.

@Nyholm

Don't add @psalm- annotations. "Standard" annotations goes a long way. And when the next super cool static analyser comes around, all the @psalm- annotations are obsolete.

Just FYI the current crop of tool makers (Psalm, PHPStan and Phan) generally support each others' annotations as much as possible, with some support for @psalm- annotations recent versions of PhpStorm too. They're useful to describe generic behaviour, which I imagine Symfony has quite a bit of.

@nicolas-grekas

Note that on my side, I don't see the benefit of committing this file since it can be generated.

It's designed to be committed – the idea is that it represents all the Psalm issues you don't that much care about, because those issues are found in existing code which presumably works already. This file can get big – Vimeo's baseline file is 1.5MB, covering 1M lines of PHP – but using the baseline has been instrumental in helping us improve the quality of new code.

Alternatively (or maybe in tandem) you can craft your psalm.xml file to target individual sorts of issues in particular files/folders, but that generally takes more work.

We'd better document how to generate it somewhere if we really want this.

There's currently some documentation here: https://psalm.dev/docs/running_psalm/dealing_with_code_issues/#using-a-baseline-file, but also happy to accept PRs if anything's confusing.

Sorry if I was unclear. I was just too focused on the rules when writing the PR description.

Having a baseline in the code base would make it easier for contributors to verify their patch before making a PR. Ie, to get a list of errors before pushing.

The baseline is not needed for the people that makes a PR to just fix psalm errors. They can generate the baseline themselves and then start working.

The drawbacks of this PR are that the git history get noisy and the cron will create weekly PRs to update the baseline.

An alternative approach to help contributors to is to add documentation or maybe a script to generate the baseline and run the analysis on the patch.

Rule 1) Don't add @psalm- annotations. "Standard" annotations goes a long way. And when the next super cool static analyser comes around, all the @psalm- annotations are obsolete.

Question: Do we accept more advanced type declarations (like generics and array shapes for instance) in standard annotations then? Something like @return array{foo:int, bar: string}? I think we have decided against those in the past, but they are extremely helpful if we want to achieve more accurate results with Psalm or other static analyzers.

Rule 5) If possible, document array keys on arguments and returns.

This party answers my question, I think.

Something like @return array{foo:int, bar: string}?

Yes, I think so.

like generics?

Generics like template?

Im more hesitant. But PHPStorm will add support for these in an upcoming version. So I would lean towards "Yes".

Something like @return array{foo:int, bar: string}?

Yes, I think so.

Unless something has changed the past months, this will confuse IDEs, wouldn't it? (which is why there are vendor prefixed annotations for these special type syntaxis)

I'm a bit unsure how we expect this to work if we expect regular updates of the baseline file is needed (for introducing "bad code" for reasons mentioned in the description). Wouldn't this result in the Psalm check to just almost always fail? I think picking the Psalm rules with care and marking specific blocks of code as ignored might be a much more useful way of using Psalm. Taking the stance of "We need good code, but we explicitly wrote bad code in section X, Y and Z for performance reasons".

like generics?

Generics like template?

First of all, a passive usage of generics as in array<string, SomeClass> or Collection<string, MyEntity>.

And yes, declaring our own generics using @template would also be helpful, like in this example.

/**
 * @template TEvent of object
 * @param TEvent $event
 * @return TEvent
 */
public function dispatch(object $event): object;

Unless something has changed the past months, this will confuse IDEs, wouldn't it? (which is why there are vendor prefixed annotations for these special type syntaxis)

Yes. PhpStorm for instance is still really bad at understanding generic traversable types. This is usually where I use vendor-prefixed annotations next to standard ones.

My 2cts: no bug, no fix.

I will make a PR to the contribution guidelines where I discuss the "rules".

Fixing psalm errors is welcomed. We just need to figure out what kinds of fixes that are okey.

Bug: I cannot check static analysis locally
Fix: commit the baseline file

I'm more and more using Vim with asynchrounous linting, which continuously runs Psalm. A committed baseline file will be a great help for me to not get a completely red editor.

Moreover, PHPstorm also integrates with Psalm in the recent releases. Quicker static analysis feedback = less annoyance/hindering when contributing.

(and yes, I use Psalm and Git on a daily basis, so I know that I can create the baseline file myself and add .github/baseline.xml to .git/info/exclude. But I guess a large part of Symfony's contributors isn't that used to doing things like this)