Skip to content

Commit

Permalink
Fix diagnostic suppressor feature for compiler warnings in presence o…
Browse files Browse the repository at this point in the history
…f /warnaserror

Follow up to dotnet#36067
Fixes dotnet#36215
  • Loading branch information
mavasani committed Jun 19, 2019
1 parent f7ce215 commit d3a2da9
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10790,7 +10790,7 @@ public void InvalidPathCharacterInPdbPath()
}

[WorkItem(20242, "https://github.com/dotnet/roslyn/issues/20242")]
[Fact(Skip = "https://github.com/dotnet/roslyn/issues/36215")]
[Fact]
public void TestSuppression_CompilerParserWarningAsError()
{
string source = @"
Expand Down
12 changes: 2 additions & 10 deletions src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -560,7 +560,7 @@ internal bool ReportDiagnostics(IEnumerable<DiagnosticInfo> diagnostics, TextWri
=> ReportDiagnostics(diagnostics.Select(info => Diagnostic.Create(info)), consoleOutput, errorLoggerOpt);

/// <summary>
/// Returns true if there are any diagnostics in the bag which have error severity and are
/// Returns true if there are any diagnostics in the bag which have default severity error and are
/// not marked "suppressed". Note: does NOT do filtering, so it may return false if a
/// non-error diagnostic were later elevated to an error through filtering (e.g., through
/// warn-as-error). This is meant to be a check if there are any "real" errors, in the bag
Expand All @@ -571,22 +571,14 @@ internal static bool HasUnsuppressedErrors(DiagnosticBag diagnostics)
{
foreach (var diag in diagnostics.AsEnumerable())
{
if (IsReportedError(diag))
if (diag.DefaultSeverity == DiagnosticSeverity.Error && !diag.IsSuppressed)
{
return true;
}
}
return false;
}

/// <summary>
/// Returns true if the diagnostic is an error that should be reported.
/// </summary>
private static bool IsReportedError(Diagnostic diagnostic)
{
return (diagnostic.Severity == DiagnosticSeverity.Error) && !diagnostic.IsSuppressed;
}

protected virtual void PrintError(Diagnostic diagnostic, TextWriter consoleOutput)
{
consoleOutput.WriteLine(DiagnosticFormatter.Format(diagnostic, Culture));
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/Core/Portable/Compilation/Compilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1347,7 +1347,7 @@ internal void CompleteCompilationEventQueue_NoLock()
/// </summary>
/// <param name="accumulator">Bag to which filtered diagnostics will be added.</param>
/// <param name="incoming">Diagnostics to be filtered.</param>
/// <returns>True if there were no errors or warnings-as-errors.</returns>
/// <returns>True if there were no errors.</returns>
internal bool FilterAndAppendAndFreeDiagnostics(DiagnosticBag accumulator, ref DiagnosticBag incoming)
{
bool result = FilterAndAppendDiagnostics(accumulator, incoming.AsEnumerableWithoutResolution(), exclude: null);
Expand Down Expand Up @@ -1378,7 +1378,7 @@ internal bool FilterAndAppendDiagnostics(DiagnosticBag accumulator, IEnumerable<
{
continue;
}
else if (filtered.Severity == DiagnosticSeverity.Error &&
else if (filtered.DefaultSeverity == DiagnosticSeverity.Error &&
!filtered.IsSuppressed)
{
hasError = true;
Expand Down
10 changes: 6 additions & 4 deletions src/Compilers/Core/Portable/Diagnostic/DiagnosticBag.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ public bool IsEmptyWithoutResolution
}

/// <summary>
/// Returns true if the bag has any diagnostics with Severity=Error. Does not consider warnings or informationals.
/// Returns true if the bag has any diagnostics with DefaultSeverity=Error. Does not consider warnings or informationals
/// or warnings promoted to error via /warnaserror.
/// </summary>
/// <remarks>
/// Resolves any lazy diagnostics in the bag.
Expand All @@ -68,7 +69,7 @@ public bool HasAnyErrors()

foreach (Diagnostic diagnostic in Bag)
{
if (diagnostic.Severity == DiagnosticSeverity.Error)
if (diagnostic.DefaultSeverity == DiagnosticSeverity.Error)
{
return true;
}
Expand All @@ -78,7 +79,8 @@ public bool HasAnyErrors()
}

/// <summary>
/// Returns true if the bag has any non-lazy diagnostics with Severity=Error.
/// Returns true if the bag has any non-lazy diagnostics with DefaultSeverity=Error. Does not consider warnings or informationals
/// or warnings promoted to error via /warnaserror.
/// </summary>
/// <remarks>
/// Does not resolve any lazy diagnostics in the bag.
Expand All @@ -95,7 +97,7 @@ internal bool HasAnyResolvedErrors()

foreach (Diagnostic diagnostic in Bag)
{
if ((diagnostic as DiagnosticWithInfo)?.HasLazyInfo != true && diagnostic.Severity == DiagnosticSeverity.Error)
if ((diagnostic as DiagnosticWithInfo)?.HasLazyInfo != true && diagnostic.DefaultSeverity == DiagnosticSeverity.Error)
{
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9386,7 +9386,7 @@ End Class"
End Sub

<WorkItem(20242, "https://github.com/dotnet/roslyn/issues/20242")>
<Fact(Skip:="https://github.com/dotnet/roslyn/issues/36215")>
<Fact>
Public Sub TestSuppression_CompilerWarningAsError()
' warning BC40008 : 'C' is obsolete
Dim source = "
Expand Down

0 comments on commit d3a2da9

Please sign in to comment.