Skip to content

Commit

Permalink
Do not skip emit if errors are suppressed (dotnet#34174)
Browse files Browse the repository at this point in the history
In the command line compilation we try to discover when to stop the
compilation stages based on if an error is produced. If the error was
produced by /warnaserror, then suppressed, this should not be considered
a compilation-halting error.

Fixes dotnet#34101
  • Loading branch information
agocke authored Mar 19, 2019
1 parent 9d6284e commit fb51650
Show file tree
Hide file tree
Showing 5 changed files with 99 additions and 8 deletions.
26 changes: 26 additions & 0 deletions src/Compilers/CSharp/Test/CommandLine/CommandLineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,32 @@ private CSharpCommandLineArguments FullParse(string commandLine, string baseDire
return CSharpCommandLineParser.Default.Parse(args, baseDirectory, sdkDirectory, additionalReferenceDirectories);
}

[ConditionalFact(typeof(WindowsDesktopOnly))]
[WorkItem(34101, "https://github.com/dotnet/roslyn/issues/34101")]
public void SuppressedWarnAsErrorsStillEmit()
{
var dir = Temp.CreateDirectory();
var src = dir.CreateFile("temp.cs").WriteAllText(@"
#pragma warning disable 1591

public class P {
public static void Main() {}
}");
const string docName = "doc.xml";

var cmd = CreateCSharpCompiler(null, dir.Path, new[] { "/nologo", "/errorlog:errorlog", $"/doc:{docName}", "/warnaserror", src.Path });

var outWriter = new StringWriter(CultureInfo.InvariantCulture);
var exitCode = cmd.Run(outWriter);
Assert.Equal(0, exitCode);
Assert.Equal("", outWriter.ToString());

string exePath = Path.Combine(dir.Path, "temp.exe");
Assert.True(File.Exists(exePath));
var result = ProcessUtilities.Run(exePath, arguments: "");
Assert.Equal(0, result.ExitCode);
}

[ConditionalFact(typeof(WindowsDesktopOnly), Reason = ConditionalSkipReason.TestExecutionNeedsWindowsTypes)]
public void XmlMemoryMapped()
{
Expand Down
19 changes: 19 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,25 @@ namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen
{
public class CodeGenTests : CSharpTestBase
{
[Fact]
public void EmitWithSuppressedWarnAsError()
{
var src = @"
#pragma warning disable 1591
public class P {
public static void Main() {}
}";
var parseOptions = TestOptions.RegularWithDocumentationComments;
var options = TestOptions.ReleaseDll
.WithXmlReferenceResolver(XmlFileResolver.Default)
.WithGeneralDiagnosticOption(ReportDiagnostic.Error);

var comp = CreateCompilation(src, parseOptions: parseOptions, options: options);
comp.VerifyEmitDiagnostics();
CompileAndVerify(comp);
}

[Fact()]
[WorkItem(776642, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/776642")]
public void Bug776642a()
Expand Down
30 changes: 25 additions & 5 deletions src/Compilers/Core/Portable/CommandLine/CommonCompiler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,26 @@ internal bool ReportErrors(IEnumerable<Diagnostic> diagnostics, TextWriter conso
private bool ReportErrors(DiagnosticBag diagnostics, TextWriter consoleOutput, ErrorLogger errorLoggerOpt)
=> ReportErrors(diagnostics.ToReadOnly(), consoleOutput, errorLoggerOpt);

/// <summary>
/// Returns true if there are any diagnostics in the bag which have error severity 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
/// since diagnostics with default "error" severity can never be suppressed or reduced
/// below error severity.
/// </summary>
internal static bool HasUnsuppressedErrors(DiagnosticBag diagnostics)
{
foreach (var diag in diagnostics.AsEnumerable())
{
if (IsReportedError(diag))
{
return true;
}
}
return false;
}

/// <summary>
/// Returns true if the diagnostic is an error that should be reported.
/// </summary>
Expand Down Expand Up @@ -648,7 +668,7 @@ private void CompileAndEmit(

// Print the diagnostics produced during the parsing stage and exit if there were any errors.
compilation.GetDiagnostics(CompilationStage.Parse, includeEarlierStages: false, diagnostics, cancellationToken);
if (diagnostics.HasAnyErrors())
if (HasUnsuppressedErrors(diagnostics))
{
return;
}
Expand All @@ -674,7 +694,7 @@ private void CompileAndEmit(
}

compilation.GetDiagnostics(CompilationStage.Declare, includeEarlierStages: false, diagnostics, cancellationToken);
if (diagnostics.HasAnyErrors())
if (HasUnsuppressedErrors(diagnostics))
{
return;
}
Expand Down Expand Up @@ -786,7 +806,7 @@ private void CompileAndEmit(
{
using (var win32ResourceStreamOpt = GetWin32Resources(MessageProvider, Arguments, compilation, diagnostics))
{
if (diagnostics.HasAnyErrors())
if (HasUnsuppressedErrors(diagnostics))
{
return;
}
Expand Down Expand Up @@ -882,7 +902,7 @@ private void CompileAndEmit(
}
}

if (diagnostics.HasAnyErrors())
if (HasUnsuppressedErrors(diagnostics))
{
return;
}
Expand All @@ -902,7 +922,7 @@ private void CompileAndEmit(
if (analyzerExceptionDiagnostics != null)
{
diagnostics.AddRange(analyzerExceptionDiagnostics);
if (analyzerExceptionDiagnostics.HasAnyErrors())
if (HasUnsuppressedErrors(analyzerExceptionDiagnostics))
{
return;
}
Expand Down
8 changes: 5 additions & 3 deletions src/Compilers/Core/Portable/Compilation/Compilation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Collections.Immutable;
using System.ComponentModel;
using System.Diagnostics;
using System.Diagnostics.Contracts;
using System.IO;
using System.Linq;
using System.Reflection;
Expand Down Expand Up @@ -1357,7 +1358,7 @@ internal bool FilterAndAppendAndFreeDiagnostics(DiagnosticBag accumulator, ref D
/// <summary>
/// Filter out warnings based on the compiler options (/nowarn, /warn and /warnaserror) and the pragma warning directives.
/// </summary>
/// <returns>True when there is no error.</returns>
/// <returns>True if there are no unsuppressed errors (i.e., no errors which fail compilation).</returns>
internal bool FilterAndAppendDiagnostics(DiagnosticBag accumulator, IEnumerable<Diagnostic> incoming, HashSet<int> exclude)
{
bool hasError = false;
Expand All @@ -1376,7 +1377,8 @@ internal bool FilterAndAppendDiagnostics(DiagnosticBag accumulator, IEnumerable<
{
continue;
}
else if (filtered.Severity == DiagnosticSeverity.Error)
else if (filtered.Severity == DiagnosticSeverity.Error &&
!filtered.IsSuppressed)
{
hasError = true;
}
Expand Down Expand Up @@ -2578,7 +2580,7 @@ internal CommonPEModuleBuilder CheckOptionsAndCreateModuleBuilder(
}
}

if (diagnostics.HasAnyErrors())
if (CommonCompiler.HasUnsuppressedErrors(diagnostics))
{
return null;
}
Expand Down
24 changes: 24 additions & 0 deletions src/Compilers/VisualBasic/Test/CommandLine/CommandLineTests.vb
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,30 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.CommandLine.UnitTests
Return VisualBasicCommandLineParser.Script.Parse(args, baseDirectory, sdkDirectory, additionalReferenceDirectories)
End Function

<Fact>
<WorkItem(34101, "https://github.com/dotnet/roslyn/issues/34101")>
Public Sub SuppressedWarnAsErrorsStillEmit()
Dim dir = Temp.CreateDirectory()
Dim src = dir.CreateFile("temp.vb").WriteAllText("
#Disable Warning BC42302
Module Module1
Sub Main()
Dim x = 42 ''' <test />
End Sub
End Module")
Const docName As String = "doc.xml"

Dim cmd = New MockVisualBasicCompiler(Nothing, dir.Path, {"/nologo", "/errorlog:errorlog", $"/doc:{docName}", "/warnaserror", src.Path})

Dim outWriter = New StringWriter(CultureInfo.InvariantCulture)
Dim exitCode = cmd.Run(outWriter)
Assert.Equal("", outWriter.ToString())
Assert.Equal(0, exitCode)

Dim exePath = Path.Combine(dir.Path, "temp.exe")
Assert.True(File.Exists(exePath))
End Sub

<Fact>
Public Sub XmlMemoryMapped()
Dim dir = Temp.CreateDirectory()
Expand Down

0 comments on commit fb51650

Please sign in to comment.