[Bug]: Building inline task invocation succeeded with an error

[GangWang01]

Issue Description

Building this project that invokes an inline task got strange behavior. The output log displayed the successful task execution as
an error, while build succeeded with 1 error.

<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
  <!-- This simple inline task displays "Hello, world!" -->
  <UsingTask
    TaskName="HelloWorld"
    TaskFactory="CodeTaskFactory"
    AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll" >
    <ParameterGroup />
    <Task>
      <Reference Include="System.Xml"/>
      <Using Namespace="System"/>
      <Using Namespace="System.IO"/>
      <Code Type="Fragment" Language="cs">
<![CDATA[
// Display "Hello, world!"
Log.LogError("Hello, world!");
]]>
      </Code>
    </Task>
  </UsingTask>

   <Target Name="Hello">
    <HelloWorld />
  </Target>
</Project>

The build log is:

Steps to Reproduce

Open Developer Command Prompt and use msbuild.exe to build the project.

Expected Behavior

In the build log task execution is displayed as normal message. Build succeeded without errors.

Actual Behavior

In the build log task execution is displayed as error. Build succeeded with 1 error.

Analysis

No response

Versions & Configurations

No response

[KirillOsenkov]

good bug!

[rainersigwald]

This is a consequence of the ITask interface. Tasks can return success or failure by the bool return value of Execute(), and independently they can log errors. Unfortunately, that means you can say

Log.LogError("Catastrophic failure");
return true; // "Task succeeded"

And unfortunately that's basically what we do for the fragment type of inline code (add return true; after the user-specified code).

We should consider adopting the preferred pattern (return !Log.HasLoggedError;) in our generation around here:

msbuild/src/Tasks/CodeTaskFactory.cs

Lines 390 to 401 in 85d1d66

	private static void CreateExecuteMethodFromFragment(CodeTypeDeclaration codeTypeDeclaration, string executeCode)
	{
	var executeMethod = new CodeMemberMethod
	{
	Name = "Execute",
	Attributes = MemberAttributes.Override | MemberAttributes.Public
	};
	executeMethod.Statements.Add(new CodeSnippetStatement(executeCode));
	executeMethod.ReturnType = new CodeTypeReference(typeof(Boolean));
	executeMethod.Statements.Add(new CodeMethodReturnStatement(new CodeFieldReferenceExpression(null, "_Success")));
	codeTypeDeclaration.Members.Add(executeMethod);
	}

and also in RoslynCodeTaskFactory

msbuild/src/Tasks/RoslynCodeTaskFactory/RoslynCodeTaskFactory.cs

Lines 226 to 239 in 85d1d66

	if (taskInfo.CodeType == RoslynCodeTaskFactoryCodeType.Fragment)
	{
	CodeMemberProperty successProperty = CreateProperty(codeTypeDeclaration, "Success", typeof(bool), true);
	CodeMemberMethod executeMethod = new CodeMemberMethod
	{
	Name = "Execute",
	// ReSharper disable once BitwiseOperatorOnEnumWithoutFlags
	Attributes = MemberAttributes.Override | MemberAttributes.Public,
	ReturnType = new CodeTypeReference(typeof(Boolean))
	};
	executeMethod.Statements.Add(new CodeSnippetStatement(taskInfo.SourceCode));
	executeMethod.Statements.Add(new CodeMethodReturnStatement(new CodePropertyReferenceExpression(null, successProperty.Name)));
	codeTypeDeclaration.Members.Add(executeMethod);