Write APICompat suppressions when baseline suppressions aren't empty

[ViktorHofer]

When there aren't any suppressions but baseline suppressions exist, write to the suppression file to make it empty.

Pass information back to the caller so that even if an empty collection of suppression are written to the file, still log a message to indicate that the suppression file got updated.

[ViktorHofer]

@ericstj when the /p:GenerateAPICompatSuppressionFile=true flag is passed in, the baseline suppression file not being empty but there aren't actual suppressions, should we write an empty suppressions file or not write at all and try to delete if it exists?

With this change, we update the file and make it empty:

<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" />

[ericstj]

@ericstj when the /p:GenerateAPICompatSuppressionFile=true flag is passed in, the baseline suppression file not being empty but there aren't actual suppressions, should we write an empty suppressions file or not write at all and try to delete if it exists?

With this change, we update the file and make it empty:

<?xml version="1.0" encoding="utf-8"?>
<!-- https://learn.microsoft.com/dotnet/fundamentals/package-validation/diagnostic-ids -->
<Suppressions xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema" />

I think in our case we'd prefer to delete the file since we conditionally set it based on existence -- would that work in general? Suppose someone specified the file name - if we deleted that file would we handle a specified path to a non-existent file on run? Maybe not. I guess emitting an empty file is OK. If folks want to delete it, then can notice the empty file and delete it.

[ViktorHofer]

Suppose someone specified the file name - if we deleted that file would we handle a specified path to a non-existent file on run? Maybe not. I guess emitting an empty file is OK. If folks want to delete it, then can notice the empty file and delete it.

The suppression engine file loader would throw a FileNotFoundException when GenerateAPICompatSuppressionFile is false:

sdk/src/Compatibility/ApiCompat/Microsoft.DotNet.ApiCompatibility/Logging/SuppressionEngine.cs

Line 45 in 5ef57c8

catch (FileNotFoundException) when (BaselineAllErrors)

I kind of lean towards deleting the empty suppression file. The switch name might confusing here though as it implies something getting generated, not deleted (GenerateAPICompatSuppressionFile).

EDIT: Given this a second thought, not deleting is probably less breaking here and won't change a potential exisiting workflow. I.e. for runtime's ApiCompat.proj we want the suppression files emptied but not deleted. If they would get deleted we would see the FileNotFoundException.

[ericstj]

I'm fine either way, just test the behavior works in and doesn't introduce new errors for folks.

[ericstj]

Should we add a test case for the scenario where we had baselines and they are no longer needed -- ensure that the file is emptied and true is returned?

[ViktorHofer]

I just noticed that we already have one that validates that: SuppressionEngine_WriteSuppressionsToFile_ReturnsEmptyWithAllUnnecessarySuppressions

[ericstj]

change looks ok, please consider adding a test for this scenario.

[ViktorHofer]

Yes, I will add a test for that scenario before merging.