Merge pull request #5207 from Forgind/MSB4132-opt-out

Emit MSB4181 when a task returns false (fails) without logging any errors. Fixes #2036

Permit a task to disable this Fixes #5203

Author: Forgind

ref/Microsoft.Build.Framework/net/Microsoft.Build.Framework.cs

@@ -203,6 +203,10 @@ public partial interface IBuildEngine6 : Microsoft.Build.Framework.IBuildEngine,
     {
         System.Collections.Generic.IReadOnlyDictionary<string, string> GetGlobalProperties();
     }
+    public partial interface IBuildEngine7 : Microsoft.Build.Framework.IBuildEngine, Microsoft.Build.Framework.IBuildEngine2, Microsoft.Build.Framework.IBuildEngine3, Microsoft.Build.Framework.IBuildEngine4, Microsoft.Build.Framework.IBuildEngine5, Microsoft.Build.Framework.IBuildEngine6
+    {
+        bool AllowFailureWithoutError { get; set; }
+    }
     public partial interface ICancelableTask : Microsoft.Build.Framework.ITask
     {
         void Cancel();

ref/Microsoft.Build.Framework/netstandard/Microsoft.Build.Framework.cs

@@ -203,6 +203,10 @@ public partial interface IBuildEngine6 : Microsoft.Build.Framework.IBuildEngine,
     {
         System.Collections.Generic.IReadOnlyDictionary<string, string> GetGlobalProperties();
     }
+    public partial interface IBuildEngine7 : Microsoft.Build.Framework.IBuildEngine, Microsoft.Build.Framework.IBuildEngine2, Microsoft.Build.Framework.IBuildEngine3, Microsoft.Build.Framework.IBuildEngine4, Microsoft.Build.Framework.IBuildEngine5, Microsoft.Build.Framework.IBuildEngine6
+    {
+        bool AllowFailureWithoutError { get; set; }
+    }
     public partial interface ICancelableTask : Microsoft.Build.Framework.ITask
     {
         void Cancel();

ref/Microsoft.Build.Utilities.Core/net/Microsoft.Build.Utilities.Core.cs

@@ -352,6 +352,7 @@ protected Task(System.Resources.ResourceManager taskResources, string helpKeywor
         public Microsoft.Build.Framework.IBuildEngine4 BuildEngine4 { get { throw null; } }
         public Microsoft.Build.Framework.IBuildEngine5 BuildEngine5 { get { throw null; } }
         public Microsoft.Build.Framework.IBuildEngine6 BuildEngine6 { get { throw null; } }
+        public Microsoft.Build.Framework.IBuildEngine7 BuildEngine7 { get { throw null; } }
         protected string HelpKeywordPrefix { get { throw null; } set { } }
         public Microsoft.Build.Framework.ITaskHost HostObject { get { throw null; } set { } }
         public Microsoft.Build.Utilities.TaskLoggingHelper Log { get { throw null; } }

ref/Microsoft.Build.Utilities.Core/netstandard/Microsoft.Build.Utilities.Core.cs

@@ -197,6 +197,7 @@ protected Task(System.Resources.ResourceManager taskResources, string helpKeywor
         public Microsoft.Build.Framework.IBuildEngine4 BuildEngine4 { get { throw null; } }
         public Microsoft.Build.Framework.IBuildEngine5 BuildEngine5 { get { throw null; } }
         public Microsoft.Build.Framework.IBuildEngine6 BuildEngine6 { get { throw null; } }
+        public Microsoft.Build.Framework.IBuildEngine7 BuildEngine7 { get { throw null; } }
         protected string HelpKeywordPrefix { get { throw null; } set { } }
         public Microsoft.Build.Framework.ITaskHost HostObject { get { throw null; } set { } }
         public Microsoft.Build.Utilities.TaskLoggingHelper Log { get { throw null; } }

ref/Microsoft.Build/net/Microsoft.Build.cs

@@ -967,6 +967,7 @@ public partial class BuildParameters
     {
         public BuildParameters() { }
         public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectCollection) { }
+        public bool AllowFailureWithoutError { get { throw null; } set { } }
         public System.Collections.Generic.IDictionary<string, string> BuildProcessEnvironment { get { throw null; } }
         public System.Threading.ThreadPriority BuildThreadPriority { get { throw null; } set { } }
         public System.Globalization.CultureInfo Culture { get { throw null; } set { } }

ref/Microsoft.Build/netstandard/Microsoft.Build.cs

@@ -963,6 +963,7 @@ public partial class BuildParameters
     {
         public BuildParameters() { }
         public BuildParameters(Microsoft.Build.Evaluation.ProjectCollection projectCollection) { }
+        public bool AllowFailureWithoutError { get { throw null; } set { } }
         public System.Collections.Generic.IDictionary<string, string> BuildProcessEnvironment { get { throw null; } }
         public System.Globalization.CultureInfo Culture { get { throw null; } set { } }
         public string DefaultToolsVersion { get { throw null; } set { } }

src/Build.UnitTests/BackEnd/BuildResult_Tests.cs

@@ -10,8 +10,8 @@
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 using Microsoft.Build.Unittest;
+using Shouldly;
 using Xunit;
-
 using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;
 
 namespace Microsoft.Build.UnitTests.BackEnd

src/Build.UnitTests/BackEnd/FailingTask.cs

@@ -0,0 +1,20 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+namespace Microsoft.Build.UnitTests
+{
+    public class FailingTask : Task
+    {
+        public override bool Execute()
+        {
+            BuildEngine.GetType().GetProperty("AllowFailureWithoutError").SetValue(BuildEngine, EnableDefaultFailure);
+            return false;
+        }
+
+        [Required]
+        public bool EnableDefaultFailure { get; set; }
+    }
+}

src/Build.UnitTests/BackEnd/LoggingContext_Tests.cs

@@ -4,8 +4,10 @@
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.Shared;
+using Shouldly;
 using System;
 using Xunit;
+using Xunit.Abstractions;
 
 namespace Microsoft.Build.UnitTests.BackEnd
 {
@@ -14,29 +16,36 @@ namespace Microsoft.Build.UnitTests.BackEnd
     /// </summary>
     public class LoggingContext_Tests
     {
+        private readonly ITestOutputHelper _output;
+
+        public LoggingContext_Tests(ITestOutputHelper outputHelper)
+        {
+            _output = outputHelper;
+        }
+
         /// <summary>
         /// A few simple tests for NodeLoggingContexts. 
         /// </summary>
         [Fact]
         public void CreateValidNodeLoggingContexts()
         {
-            NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(), 1, true);
-            Assert.True(context.IsInProcNode);
-            Assert.True(context.IsValid);
+            NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 1, true);
+            context.IsInProcNode.ShouldBeTrue();
+            context.IsValid.ShouldBeTrue();
 
             context.LogBuildFinished(true);
-            Assert.False(context.IsValid);
+            context.IsValid.ShouldBeFalse();
 
-            Assert.Equal(1, context.BuildEventContext.NodeId);
+            context.BuildEventContext.NodeId.ShouldBe(1);
 
-            NodeLoggingContext context2 = new NodeLoggingContext(new MockLoggingService(), 2, false);
-            Assert.False(context2.IsInProcNode);
-            Assert.True(context2.IsValid);
+            NodeLoggingContext context2 = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 2, false);
+            context2.IsInProcNode.ShouldBeFalse();
+            context2.IsValid.ShouldBeTrue();
 
             context2.LogBuildFinished(true);
-            Assert.False(context2.IsValid);
+            context2.IsValid.ShouldBeFalse();
 
-            Assert.Equal(2, context2.BuildEventContext.NodeId);
+            context2.BuildEventContext.NodeId.ShouldBe(2);
         }
 
         /// <summary>
@@ -49,9 +58,25 @@ public void InvalidNodeIdOnNodeLoggingContext()
         {
             Assert.Throws<InternalErrorException>(() =>
             {
-                NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(), -2, true);
+                _ = new NodeLoggingContext(new MockLoggingService(), -2, true);
             }
            );
         }
+
+        [Fact]
+        public void HasLoggedErrors()
+        {
+            NodeLoggingContext context = new NodeLoggingContext(new MockLoggingService(_output.WriteLine), 1, true);
+            context.HasLoggedErrors.ShouldBeFalse();
+
+            context.LogCommentFromText(Framework.MessageImportance.High, "Test message");
+            context.HasLoggedErrors.ShouldBeFalse();
+
+            context.LogWarningFromText(null, null, null, null, "Test warning");
+            context.HasLoggedErrors.ShouldBeFalse();
+
+            context.LogErrorFromText(null, null, null, null, "Test error");
+            context.HasLoggedErrors.ShouldBeTrue();
+        }
     }
-}
+}

src/Build.UnitTests/BackEnd/OnError_Tests.cs

@@ -11,6 +11,9 @@
 using System.Xml;
 using InvalidProjectFileException = Microsoft.Build.Exceptions.InvalidProjectFileException;
 using Xunit;
+using System.Reflection;
+using Shouldly;
+using System.Linq;
 
 namespace Microsoft.Build.UnitTests.BackEnd
 {
@@ -559,6 +562,29 @@ public void OutOfOrderOnError()
            );
         }
 
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public void ErrorWhenTaskFailsWithoutLoggingErrorEscapeHatch(bool emitError)
+        {
+            MockLogger logger = ObjectModelHelpers.BuildProjectExpectFailure($@"
+<Project>
+    <UsingTask TaskName=""FailingTask"" AssemblyFile=""{Assembly.GetExecutingAssembly().Location}"" />
+    <Target Name=""MyTarget"">
+        <FailingTask EnableDefaultFailure=""{emitError}"" />
+    </Target>
+</Project>");
+            if (emitError)
+            {
+                logger.ErrorCount.ShouldBe(1);
+                logger.Errors.First().Code.ShouldBe("MSB4181");
+            }
+            else
+            {
+                logger.ErrorCount.ShouldBe(0);
+            }
+        }
+
         #region Postbuild
         /*
          * Method:  PostBuildBasic