Allow adding new errors to baseline.

[bdsl]

Currently psalm supports removing errors from the baseline with --update-baseline but I don't believe adding new errors is supported. Adding new errors is of course possible running psalm with --set-baseline=psalm-baseline.xml but I thin that only works properly if the baseline file is deleted first, and it's a little awkward to use.

Of course in general we want errors to go down not up and we don't want to introduce new buggy code into a codebase, but I think there's a fairly common scenario where adding new errors make sense.

Suppose we have a legacy codebase that looks like this - simplified for brefity:

<?php

class PizzaTopping{
	public function __construct(
        public string $name
    ) {}
}

/**
 * @return PizzaTopping
 */
function getPizzaTopping(string $toppingId)
{
    switch ($toppingId) {
        case '1':
            return new PizzaTopping('Black Olive');
        default:
	    return null;
    }
}

function showTopping(string $toppingId): void
{
    echo getPizzaTopping($toppingId)->name;
}

// many more functions that do other things with piza toppings not shown. Some do null checks some don't.

Psalm will detect issues on the incorrectly documented getPizzaTopping function, but as this is a large legacy code base we add them to a baseline when we first start using Psalm and move on with our lives.

Some time later we decide we want to fix getPizzaTopping and correctly document it as returning ?PizzaTopping . At that point Psalm will detect that we should have done a null check in showTopping and report an error there. But showTopping has been in production for years and it's working fine, and there are dozens of functions like it. It isn't cost effective to fix them all now. So we probably revert the change to the getPizzaTopping declaration.

Instead I'm proposing a feature where we would add the issues detected in showTopping to the baseline. We don't need to fix it since we we know it's working, but the next time we call getPizzaTopping Psalm will insist we do the null check where required.

[orklah]

--set-baseline do work well even if there is already a baseline. The only thing is that the end report won't display any issues and everything is added to the baseline silently

Furthermore, it's unfortunate that we have to retype the filename everytime.

A --add-to-baseline that displays the list of new issues added would be wonderful

[bdsl]

Right, the list of new issues would be nice to paste into the PR that fixes the getPizzaTopping declaration, since the baseline.xml isn't really designed to be easy to read.

[bdsl]

I think another good use case for adding issues to the baseline is when you want to deprecate a function that's widely used in your codebase, or if you upgrade a dependency library and the deprecates stuff that you can't immediately stop using.

[bdsl]

An alternative feature might be a psalter command to add in place @psalm-suppress on all errors - maybe with an optional line of explanation about why the issue exists to be copied into every place.

[bdsl]

@muglug any thoughts on this? Would you be interested in a PR to add this feature?

[orklah]

Can you summarize what you'd want to add please? I solved my main issue with --set-baseline in the PR above but I'm not sure you explained why it this flag doesn't suit you

[bdsl]

Hi Orklah. I've been testing this out. --set-baseline=psalm-baseline.xml, but I feel like it's not very user friendly. I suppose I'd just want a purpose made and documented command that updates the existing baseline as configured in psalm.xml, where you don't have to type in the baseline file name, and maybe it outputs a summary of what it's changed in the baseline. I feel like it would be easier to encourage people to use that.

Also for some reason if I do set-baseline but I forget the equals and do --set-baseline psalm-baseline.xml instead it replaces the baseline with a tiny three line file, with no elements inside the files xml element.

[orklah]

Mmh, possibly everything could be resolved by just allowing --set-baseline not to take a value. --set-baseline=file would do the same as today and --set-baseline would retrieve the file already in config and just refresh the file

[bdsl]

yeah that could work, although I think a dedicated command would be almost as easy to build. There's also the question of whether Psalm would want to encourage the practice of adding to baseline. Clearly open to people adding new problems to the baseline, rather than my intended use which is for old problems newly visible to Psalm.

[weirdan]

Implemented in #10696

[bdsl]

Looks great, thanks!