RS1029 should not have default severity error

[sharwell]

See #3150 (comment)

[mavasani]

Based on the discussions in dotnet/roslyn#40351, I believe we want this analyzer diagnostic to have the highest visibility and hence be an error. It was even considered to be a diagnostic in the compiler itself, which will have even a higher severity as it will break everyone by default.

Tagging @terrajobst

[sharwell]

This is about providing a consistent experience across all the analyzers, and not about reducing visibility of a particular diagnostic. Treating warnings as errors in CI is already a well-established best practice, so this change will not impact general visibility of this diagnostic.

[mavasani]

I don't agree there is any problem around experience consistency here - we strongly want to deter use of reserved diagnostic IDs in 3rd party analyzers, and having them as errors forces a build break for analyzer authors violating it, as opposed to it being a warning which does not break a build, and likely might be ignored by many analyzer authors. Given that analyzer error diagnostics can be suppressed with pragmas/suppress message attributes, I feel having this diagnostic as an error is most appropriate.

I will let @terrajobst make a call on this, given he was driving adding a diagnostic when a reserved diagnostic ID is used by 3rd party analyzers.

[sharwell]

My primary argument against treating this as errors is compilation errors are used to prevent code generation for semantically undefined (broken) code (exceptions exist, but are exceedingly rare). The semantics in this case (and any other analyzer case) are unambiguous, so it could be a Level-0 warning but it doesn't meet the same bar that leads to the compiler producing errors.

While I've found plenty of reasons why various warnings should be promoted to errors in builds, I haven't found a reason why one should deviate from this long-standing approach to the distinction between warnings and errors.

[mavasani]

My primary argument against treating this as errors is compilation errors are used to prevent code generation for semantically undefined (broken) code (exceptions exist, but are exceedingly rare)

That is a valid argument. I am not going to push back more on switching RS1029 to be a warning, unless @terrajobst feels strongly that this needs to be flagged as an error.

I can submit a PR to change the default severity later today.