Support NoWarn as MSBuildWarningsAsMessages

[nguerrera]

Currently, it is not very discoverable how to disable a warning that comes from an sdk target or wherever else that is not one of the fixed components that read $(NoWarn) and police themselves, which includes NuGet and csc, but not sdk targets.

The nice thing about NoWarn is that it has project property page UI ready to go.

And even once you find MSBuildWarningsAsMessages, it's quite a mouthful and requires explaining a technicality when teaching: that it will still be logged as a low importance message. NoWarn really conveys the user intent much better.

cc @dsplaisted @rainersigwald

[rainersigwald]

My only concern with this is it's a potentially confusing change in behavior for users: if they were using NoWarn on things that didn't opt into it, those warnings would now disappear. That's not unreasonable, but is it worth the change?

[nguerrera]

I'm not sure I understand. Why would they be doing that?

[rainersigwald]

It would have been a harmless "error" of misunderstanding: at some point, someone tried to silence a warning. It didn't work, but they left it in. Now it would be silenced, which I think is a breaking (but admittedly not that breaking) change from current behavior.

[nguerrera]

I see. Maybe we save this for 17? It is technically breaking.

[jeffkl]

One issue is that NoWarn supports just the integers but MSBuildWarningsAsMessages treats everything as a string. Not that this couldn't be changed but that's how its currently implemented. So for instance /p:NoWarn=123 would not suppress a warning like MSB123. Another reason we didn't use NoWarn was because we wanted suppression and elevation of warnings. Is there a property like NoWarn to indicate which warnings should be errors?

[dsplaisted]

We talked about this and decided we should try to do this for the next major version. Mainly because NoWarn currently works for NuGet warnings as well as compiler warnings, so it is weird that MSBuild warnings are the odd one out.

[zivkan]

From our meeting today, I'd like to ask for this issue to be extended to all related warning/error properties such as WarnAsError and WarnNotAsError.

From the original email:

My understanding is that currently each component (NuGet and Roslyn/compilers at least) are responsible for their own WarnAsError implementation. NuGet has a bug in our pack task, so while WarnAsErrors are reported as errors, the task does not return a failed status, so people’s CI builds are not failing despite the logs showing an error. Plus, no NuGet task support WarnNotAsError. I was wondering if it would be worthwhile for msbuild to provide functionality around elevating warnings to errors to minimise code duplication and to minimise risk that bugs happen.

[sbomer]

@rainersigwald @dsplaisted is this planned for the next release? We are hoping to rely on it for disabling linker warnings.

[sbomer]

Just FYI, we have added a --nowarn flag to the linker, so we aren't relying on this for NoWarn. It would still be great to have WarnAsError flow to MSBuildWarningsAsErrors - though we might also need to add our own option to work around #5511.

[sbomer]

@Forgind awesome! Are there plans to do the same for the other properties? (See #4421 (comment))

[Forgind]

@sbomer, I think we wanted to go one at a time, since this could be a breaking change.

In any case, adding support for letting MSBuildWarningsAsErrors default to WarnAsError is straightforward, but I'm not sure how I'd implement WarnNotAsError. Could be a new intrinsic function or converting it to an item and converting it back, but both feel like overkill. Do you have any suggestions for that? I also couldn't find any documentation on it, just another issue in this repo explaining roughly how it should work, so I'm not sure how used it would be.

[sbomer]

I had assumed there was a MSBuildWarningsNotAsErrors, but it doesn't look like that's the case - I guess because msbuild doesn't have an option like --warnaserror-:CODE to turn off warnings as errors.

The way it works in Roslyn and ILLink is that TreatWarningsAsErrors will pass --warnaserror to turn all warnings into errors, then WarningsNotAsErrors will pass --warnaserror-:CODE to turn it back off for individual ones.

[rainersigwald]

#3062 tracks adding WarnNotAsError.