Merge pull request #49449 from mavasani/FixWarnAsErrorMinus

/warnaserror-:DiagnosticId should not nullify editorconfig diagnostic…
dotnet · Nov 24, 2020 · c536d5b · c536d5b
2 parents e192dc2 + 2bc1677
commit c536d5b
Show file tree

Hide file tree

Showing 4 changed files with 155 additions and 10 deletions.
diff --git a/src/Compilers/CSharp/Portable/Compilation/CSharpDiagnosticFilter.cs b/src/Compilers/CSharp/Portable/Compilation/CSharpDiagnosticFilter.cs
@@ -178,23 +178,36 @@ internal static ReportDiagnostic GetDiagnosticReport(
 
             ReportDiagnostic report;
             bool isSpecified = false;
+            bool specifiedWarnAsErrorMinus = false;
 
             if (specificDiagnosticOptions.TryGetValue(id, out report))
             {
                 // 2. Command line options (/nowarn, /warnaserror)
                 isSpecified = true;
+
+                // 'ReportDiagnostic.Default' is added to SpecificDiagnosticOptions for "/warnaserror-:DiagnosticId",
+                if (report == ReportDiagnostic.Default)
+                {
+                    specifiedWarnAsErrorMinus = true;
+                }
             }
-            else if (syntaxTreeOptions != null)
+
+            // Apply syntax tree options, if applicable.
+            if (syntaxTreeOptions != null &&
+                (!isSpecified || specifiedWarnAsErrorMinus))
             {
                 // 3. Editor config options (syntax tree level)
                 // 4. Global analyzer config options (compilation level)
-                if (tree != null && syntaxTreeOptions.TryGetDiagnosticValue(tree, id, cancellationToken, out report) ||
-                    syntaxTreeOptions.TryGetGlobalDiagnosticValue(id, cancellationToken, out report))
+                // Do not apply config options if it is bumping a warning to an error and "/warnaserror-:DiagnosticId" was specified on the command line.
+                if ((tree != null && syntaxTreeOptions.TryGetDiagnosticValue(tree, id, cancellationToken, out var reportFromSyntaxTreeOptions) ||
+                    syntaxTreeOptions.TryGetGlobalDiagnosticValue(id, cancellationToken, out reportFromSyntaxTreeOptions)) &&
+                    !(specifiedWarnAsErrorMinus && severity == DiagnosticSeverity.Warning && reportFromSyntaxTreeOptions == ReportDiagnostic.Error))
                 {
                     isSpecified = true;
+                    report = reportFromSyntaxTreeOptions;
 
                     // '/warnaserror' should promote warnings configured in analyzer config to error.
-                    if (report == ReportDiagnostic.Warn && generalDiagnosticOption == ReportDiagnostic.Error)
+                    if (!specifiedWarnAsErrorMinus && report == ReportDiagnostic.Warn && generalDiagnosticOption == ReportDiagnostic.Error)
                     {
                         report = ReportDiagnostic.Error;
                     }

diff --git a/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs b/src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
@@ -12350,6 +12350,70 @@ public void TestWarnAsErrorMinusDoesNotEnableDisabledByDefaultAnalyzers(Diagnost
             }
         }
 
+        [WorkItem(49446, "https://github.com/dotnet/roslyn/issues/49446")]
+        [Theory]
+        // Verify '/warnaserror-:ID' prevents escalation to 'Error' when config file bumps severity to 'Warning'
+        [InlineData(false, DiagnosticSeverity.Info, DiagnosticSeverity.Warning, DiagnosticSeverity.Error)]
+        [InlineData(true, DiagnosticSeverity.Info, DiagnosticSeverity.Warning, DiagnosticSeverity.Warning)]
+        // Verify '/warnaserror-:ID' prevents escalation to 'Error' when default severity is 'Warning' and no config file setting is specified.
+        [InlineData(false, DiagnosticSeverity.Warning, null, DiagnosticSeverity.Error)]
+        [InlineData(true, DiagnosticSeverity.Warning, null, DiagnosticSeverity.Warning)]
+        // Verify '/warnaserror-:ID' prevents escalation to 'Error' when default severity is 'Warning' and config file bumps severity to 'Error'
+        [InlineData(false, DiagnosticSeverity.Warning, DiagnosticSeverity.Error, DiagnosticSeverity.Error)]
+        [InlineData(true, DiagnosticSeverity.Warning, DiagnosticSeverity.Error, DiagnosticSeverity.Warning)]
+        // Verify '/warnaserror-:ID' has no effect when default severity is 'Info' and config file bumps severity to 'Error'
+        [InlineData(false, DiagnosticSeverity.Info, DiagnosticSeverity.Error, DiagnosticSeverity.Error)]
+        [InlineData(true, DiagnosticSeverity.Info, DiagnosticSeverity.Error, DiagnosticSeverity.Error)]
+        public void TestWarnAsErrorMinusDoesNotNullifyEditorConfig(
+            bool warnAsErrorMinus,
+            DiagnosticSeverity defaultSeverity,
+            DiagnosticSeverity? severityInConfigFile,
+            DiagnosticSeverity expectedEffectiveSeverity)
+        {
+            var analyzer = new NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault: true, defaultSeverity, throwOnAllNamedTypes: false);
+            var diagnosticId = analyzer.Descriptor.Id;
+
+            var dir = Temp.CreateDirectory();
+            var src = dir.CreateFile("test.cs").WriteAllText(@"class C { }");
+            var additionalFlags = new[] { "/warnaserror+" };
+
+            if (severityInConfigFile.HasValue)
+            {
+                var severityString = DiagnosticDescriptor.MapSeverityToReport(severityInConfigFile.Value).ToAnalyzerConfigString();
+                var analyzerConfig = dir.CreateFile(".editorconfig").WriteAllText($@"
+[*.cs]
+dotnet_diagnostic.{diagnosticId}.severity = {severityString}");
+
+                additionalFlags = additionalFlags.Append($"/analyzerconfig:{analyzerConfig.Path}").ToArray();
+            }
+
+            if (warnAsErrorMinus)
+            {
+                additionalFlags = additionalFlags.Append($"/warnaserror-:{diagnosticId}").ToArray();
+            }
+
+            int expectedWarningCount = 0, expectedErrorCount = 0;
+            switch (expectedEffectiveSeverity)
+            {
+                case DiagnosticSeverity.Warning:
+                    expectedWarningCount = 1;
+                    break;
+
+                case DiagnosticSeverity.Error:
+                    expectedErrorCount = 1;
+                    break;
+
+                default:
+                    throw ExceptionUtilities.UnexpectedValue(expectedEffectiveSeverity);
+            }
+
+            VerifyOutput(dir, src, includeCurrentAssemblyAsAnalyzerReference: false,
+                expectedWarningCount: expectedWarningCount,
+                expectedErrorCount: expectedErrorCount,
+                additionalFlags: additionalFlags,
+                analyzers: new[] { analyzer });
+        }
+
         [Fact]
         public void SourceGenerators_EmbeddedSources()
         {

diff --git a/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicDiagnosticFilter.vb b/src/Compilers/VisualBasic/Portable/Compilation/VisualBasicDiagnosticFilter.vb
@@ -135,20 +135,35 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
             Dim report As ReportDiagnostic
             Dim tree = location?.SourceTree
             Dim isSpecified As Boolean = False
+            Dim specifiedWarnAsErrorMinus As Boolean = False
 
             ' Global options depend on other options, so calculate those first
             If caseInsensitiveSpecificDiagnosticOptions.TryGetValue(id, report) Then
                 ' 2. Command line options (/nowarn, /warnaserror)
                 isSpecified = True
-            ElseIf syntaxTreeOptions IsNot Nothing Then
-                If (tree IsNot Nothing AndAlso syntaxTreeOptions.TryGetDiagnosticValue(tree, id, cancellationToken, report)) OrElse
-                   syntaxTreeOptions.TryGetGlobalDiagnosticValue(id, cancellationToken, report) Then
-                    ' 3. Editor config options (syntax tree level)
-                    ' 4. Global analyzer config options (compilation level)
+
+                ' 'ReportDiagnostic.Default' is added to SpecificDiagnosticOptions for "/warnaserror-:DiagnosticId",
+                If report = ReportDiagnostic.Default Then
+                    specifiedWarnAsErrorMinus = True
+                End If
+            End If
+
+            ' Apply syntax tree options, if applicable.
+            If syntaxTreeOptions IsNot Nothing AndAlso
+               (Not isSpecified OrElse specifiedWarnAsErrorMinus) Then
+
+                ' 3. Editor config options (syntax tree level)
+                ' 4. Global analyzer config options (compilation level)
+                ' Do not apply config options if it is bumping a warning to an error and "/warnaserror-:DiagnosticId" was specified on the command line.
+                Dim reportFromSyntaxTreeOptions As ReportDiagnostic
+                If ((tree IsNot Nothing AndAlso syntaxTreeOptions.TryGetDiagnosticValue(tree, id, cancellationToken, reportFromSyntaxTreeOptions)) OrElse
+                     syntaxTreeOptions.TryGetGlobalDiagnosticValue(id, cancellationToken, reportFromSyntaxTreeOptions)) AndAlso
+                    Not (specifiedWarnAsErrorMinus AndAlso severity = DiagnosticSeverity.Warning AndAlso reportFromSyntaxTreeOptions = ReportDiagnostic.Error) Then
                     isSpecified = True
+                    report = reportFromSyntaxTreeOptions
 
                     ' '/warnaserror' should promote warnings configured in analyzer config to error.
-                    If report = ReportDiagnostic.Warn AndAlso generalDiagnosticOption = ReportDiagnostic.Error Then
+                    If Not specifiedWarnAsErrorMinus AndAlso report = ReportDiagnostic.Warn AndAlso generalDiagnosticOption = ReportDiagnostic.Error Then
                         report = ReportDiagnostic.Error
                     End If
                 End If

diff --git a/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb b/src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb
@@ -10058,6 +10058,59 @@ End Class")
             End If
         End Sub
 
+        <WorkItem(49446, "https://github.com/dotnet/roslyn/issues/49446")>
+        <Theory>
+        <InlineData(False, DiagnosticSeverity.Info, DiagnosticSeverity.Warning, DiagnosticSeverity.Error)>
+        <InlineData(True, DiagnosticSeverity.Info, DiagnosticSeverity.Warning, DiagnosticSeverity.Warning)>
+        <InlineData(False, DiagnosticSeverity.Warning, Nothing, DiagnosticSeverity.Error)>
+        <InlineData(True, DiagnosticSeverity.Warning, Nothing, DiagnosticSeverity.Warning)>
+        <InlineData(False, DiagnosticSeverity.Warning, DiagnosticSeverity.Error, DiagnosticSeverity.Error)>
+        <InlineData(True, DiagnosticSeverity.Warning, DiagnosticSeverity.Error, DiagnosticSeverity.Warning)>
+        <InlineData(False, DiagnosticSeverity.Info, DiagnosticSeverity.Error, DiagnosticSeverity.Error)>
+        <InlineData(True, DiagnosticSeverity.Info, DiagnosticSeverity.Error, DiagnosticSeverity.Error)>
+        Public Sub TestWarnAsErrorMinusDoesNotNullifyEditorConfig(warnAsErrorMinus As Boolean,
+                                                                  defaultSeverity As DiagnosticSeverity,
+                                                                  severityInConfigFile As DiagnosticSeverity?,
+                                                                  expectedEffectiveSeverity As DiagnosticSeverity)
+            Dim analyzer = New NamedTypeAnalyzerWithConfigurableEnabledByDefault(isEnabledByDefault:=True, defaultSeverity, throwOnAllNamedTypes:=False)
+            Dim diagnosticId = analyzer.Descriptor.Id
+
+            Dim dir = Temp.CreateDirectory()
+            Dim src = dir.CreateFile("test.vb").WriteAllText("
+Class C
+End Class")
+            Dim additionalFlags = {"/warnaserror+"}
+
+            If severityInConfigFile.HasValue Then
+                Dim severityString = DiagnosticDescriptor.MapSeverityToReport(severityInConfigFile.Value).ToAnalyzerConfigString()
+                Dim analyzerConfig = dir.CreateFile(".editorconfig").WriteAllText($"
+[*.vb]
+dotnet_diagnostic.{diagnosticId}.severity = {severityString}")
+
+                additionalFlags = additionalFlags.Append($"/analyzerconfig:{analyzerConfig.Path}").ToArray()
+            End If
+
+            If warnAsErrorMinus Then
+                additionalFlags = additionalFlags.Append($"/warnaserror-:{diagnosticId}").ToArray()
+            End If
+
+            Dim expectedWarningCount As Integer = 0, expectedErrorCount As Integer = 0
+            Select Case expectedEffectiveSeverity
+                Case DiagnosticSeverity.Warning
+                    expectedWarningCount = 1
+                Case DiagnosticSeverity.[Error]
+                    expectedErrorCount = 1
+                Case Else
+                    Throw ExceptionUtilities.UnexpectedValue(expectedEffectiveSeverity)
+            End Select
+
+            VerifyOutput(dir, src, includeCurrentAssemblyAsAnalyzerReference:=False,
+                         expectedWarningCount:=expectedWarningCount,
+                         expectedErrorCount:=expectedErrorCount,
+                         additionalFlags:=additionalFlags,
+                         analyzers:=ImmutableArray.Create(Of DiagnosticAnalyzer)(analyzer))
+        End Sub
+
         <Fact>
         <WorkItem(44087, "https://github.com/dotnet/roslyn/issues/44804")>
         Public Sub GlobalAnalyzerConfigDiagnosticOptionsCanBeOverridenByCommandLine()