Preserve CustomObsoleteDiagnosticInfo when changing severity #47777
Conversation
| @@ -50,6 +56,11 @@ public override DiagnosticDescriptor Descriptor | |||
| } | |||
| } | |||
|
|
|||
| internal override DiagnosticInfo GetInstanceWithSeverity(DiagnosticSeverity severity) | |||
There was a problem hiding this comment.
Did you look at the other types that derive from DiagnosticInfo to see if they needed to also override this?
There was a problem hiding this comment.
This is the only one because it's the only one that overrides Descriptor and MessageIdentifier.
|
I continue to get integration failures in release mode after 3 attempts:
I believe my change does not impact the build's ability to produce and copy this file to wherever it belongs, therefore |
|
@AlekseyTs could you please take a look at this PR? |
|
Mail was just sent, integration tests are broken at the moment. I'm going to remove the required bit for the time being. |
| ); | ||
|
|
||
| verify(TestOptions.DebugDll.WithGeneralDiagnosticOption(ReportDiagnostic.Hidden), | ||
| // (6,9): warning TEST1: 'C1.M1()' is obsolete |
There was a problem hiding this comment.
warning [](start = 26, length = 7)
"hidden"?
There was a problem hiding this comment.
Ahh, must have been a copy-paste issue. This severity is not actually verified, so I will just modify the comment.
| verify(TestOptions.DebugDll.WithSpecificDiagnosticOptions(ImmutableDictionary<string, ReportDiagnostic>.Empty.Add("CS0618", ReportDiagnostic.Suppress)), | ||
| // (6,9): warning TEST1: 'C1.M1()' is obsolete | ||
| // M1(); // 1 | ||
| Diagnostic("TEST1", "M1()", isSuppressed: false).WithArguments("C1.M1()").WithLocation(6, 9) |
There was a problem hiding this comment.
Diagnostic("TEST1", "M1()", isSuppressed: false).WithArguments("C1.M1()").WithLocation(6, 9) [](start = 16, length = 92)
Why isn't this diagnostic suppressed?
There was a problem hiding this comment.
Suppressions to CS0618 don't apply to this diagnostic. See #42119
| @@ -137,7 +137,7 @@ internal DiagnosticInfo(CommonMessageProvider messageProvider, bool isWarningAsE | |||
| } | |||
|
|
|||
| // Create a copy of this instance with a explicit overridden severity | |||
| internal DiagnosticInfo GetInstanceWithSeverity(DiagnosticSeverity severity) | |||
| internal virtual DiagnosticInfo GetInstanceWithSeverity(DiagnosticSeverity severity) | |||
There was a problem hiding this comment.
Half of me wants to make this method abstract to force all derived types to acknowledge this potential issue. The other half of me thinks that's a bit overkill here.
There was a problem hiding this comment.
Yeah, it makes me wish for the ability to say "all of these members in the group must be overridden, or none of them".
ghost
added this to the 
Closes #47736
Need to make a pass to add tests, but I think this fixes the problem.