BuildCheck has better message when Check fails on Initialize

src/Build/BackEnd/Components/Logging/EventSourceSink.cs

@@ -8,6 +8,7 @@
 using Microsoft.Build.Shared;
 
 using InternalLoggerException = Microsoft.Build.Exceptions.InternalLoggerException;
+using System.Diagnostics;
 
 #nullable disable
 

src/Build/BackEnd/Shared/EventsCreatorHelper.cs

@@ -60,4 +60,35 @@ public static BuildErrorEventArgs CreateErrorEventFromText(BuildEventContext bui
 
         return buildEvent;
     }
+
+    public static BuildWarningEventArgs CreateWarningEventFromText(BuildEventContext buildEventContext, string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message)
+    {
+        ErrorUtilities.VerifyThrowInternalNull(buildEventContext, nameof(buildEventContext));
+        ErrorUtilities.VerifyThrowInternalNull(file, nameof(file));
+        ErrorUtilities.VerifyThrowInternalNull(message, nameof(message));
+
+        string? subcategory = null;
+
+        if (subcategoryResourceName != null)
+        {
+            subcategory = AssemblyResources.GetString(subcategoryResourceName);
+        }
+
+        BuildWarningEventArgs buildEvent =
+        new BuildWarningEventArgs(
+            subcategory,
+            errorCode,
+            file!.File,
+            file.Line,
+            file.Column,
+            file.EndLine,
+            file.EndColumn,
+            message,
+            helpKeyword,
+            "MSBuild");
+
+        buildEvent.BuildEventContext = buildEventContext;
+
+        return buildEvent;
+    }
 }

src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs

@@ -188,6 +188,7 @@ internal void RegisterCustomCheck(
         {
             if (_enabledDataSources[(int)buildCheckDataSource])
             {
+                List<CheckFactoryContext> checksToRemove = new();
                 foreach (var factory in factories)
                 {
                     var instance = factory();
@@ -201,10 +202,24 @@ internal void RegisterCustomCheck(
                         if (checkFactoryContext != null)
                         {
                             _checkRegistry.Add(checkFactoryContext);
-                            SetupSingleCheck(checkFactoryContext, projectPath);
-                            checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckSuccessfulAcquisition", instance.FriendlyName);
+                            try
+                            {
+                                SetupSingleCheck(checkFactoryContext, projectPath);
+                                checkContext.DispatchAsComment(MessageImportance.Normal, "CustomCheckSuccessfulAcquisition", instance.FriendlyName);
+                            }
+                            catch (BuildCheckConfigurationException e)
+                            {
+                                checkContext.DispatchAsWarningFromText(
+                                    null,
+                                    null,
+                                    null,
+                                    new BuildEventFileInfo(projectPath),
+                                    e.Message);
+                                checksToRemove.Add(checkFactoryContext);
+                            }
                         }
                     }
+                    RemoveChecks(checksToRemove, checkContext);
                 }
             }
         }
@@ -295,7 +310,7 @@ private void SetupChecksForNewProject(string projectFullPath, ICheckContext chec
                 }
                 catch (BuildCheckConfigurationException e)
                 {
-                    checkContext.DispatchAsErrorFromText(
+                    checkContext.DispatchAsWarningFromText(
                         null,
                         null,
                         null,
@@ -305,6 +320,14 @@ private void SetupChecksForNewProject(string projectFullPath, ICheckContext chec
                 }
             }
 
+            RemoveChecks(checksToRemove, checkContext);
+
+            stopwatch.Stop();
+            _tracingReporter.AddNewProjectStats(stopwatch.Elapsed);
+        }
+
+        private void RemoveChecks(List<CheckFactoryContext> checksToRemove, ICheckContext checkContext)
+        {
             checksToRemove.ForEach(c =>
             {
                 _checkRegistry.Remove(c);
@@ -316,9 +339,6 @@ private void SetupChecksForNewProject(string projectFullPath, ICheckContext chec
                 _tracingReporter.AddCheckStats(checkToRemove!.Check.FriendlyName, checkToRemove.Elapsed);
                 checkToRemove.Check.Dispose();
             }
-
-            stopwatch.Stop();
-            _tracingReporter.AddNewProjectStats(stopwatch.Elapsed);
         }
 
         public void ProcessEvaluationFinishedEventArgs(

src/Build/BuildCheck/Infrastructure/CheckContext/CheckDispatchingContext.cs

@@ -62,4 +62,11 @@ public void DispatchAsErrorFromText(string? subcategoryResourceName, string? err
 
         _eventDispatcher.Dispatch(buildEvent);
     }
+
+    public void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message)
+    {
+        BuildWarningEventArgs buildEvent = EventsCreatorHelper.CreateWarningEventFromText(_eventContext, subcategoryResourceName, errorCode, helpKeyword, file, message);
+
+        _eventDispatcher.Dispatch(buildEvent);
+    }
 }

src/Build/BuildCheck/Infrastructure/CheckContext/CheckLoggingContext.cs

@@ -39,4 +39,8 @@ public void DispatchAsCommentFromText(MessageImportance importance, string messa
     public void DispatchAsErrorFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message)
         => loggingService
             .LogErrorFromText(eventContext, subcategoryResourceName, errorCode, helpKeyword, file, message);
+
+    public void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message)
+        => loggingService
+            .LogWarningFromText(eventContext, subcategoryResourceName, errorCode, helpKeyword, file, message);
 }

src/Build/BuildCheck/Infrastructure/CheckContext/ICheckContext.cs

@@ -40,4 +40,9 @@ internal interface ICheckContext
     /// Dispatch the instance of <see cref="BuildEventContext"/> as a comment with provided text for the message.
     /// </summary>
     void DispatchAsCommentFromText(MessageImportance importance, string message);
+
+    /// <summary>
+    /// Dispatch the instance of <see cref="BuildEventContext"/> as a warning message.
+    /// </summary>
+    void DispatchAsWarningFromText(string? subcategoryResourceName, string? errorCode, string? helpKeyword, BuildEventFileInfo file, string message);
 }

src/BuildCheck.UnitTests/EndToEndTests.cs

@@ -343,6 +343,37 @@ public void CustomCheckTest_WithEditorConfig(string checkCandidate, string ruleI
         }
     }
 
+    [Theory]
+    [InlineData("X01236", "Something went wrong initializing")]
+    // [InlineData("X01237", "message")]
+    // [InlineData("X01238", "message")]
+    public void CustomChecksFailGracefully(string ruleId, string expectedMessage)
+    {
+        using (var env = TestEnvironment.Create())
+        {
+            string checkCandidate = "CheckCandidateWithMultipleChecksInjected";
+            string checkCandidatePath = Path.Combine(TestAssetsRootPath, checkCandidate);
+
+            // Can't use Transitive environment due to the need to dogfood local nuget packages.
+            AddCustomDataSourceToNugetConfig(checkCandidatePath);
+            string editorConfigName = Path.Combine(checkCandidatePath, EditorConfigFileName);
+            File.WriteAllText(editorConfigName, ReadEditorConfig(
+                new List<(string, string)>() { (ruleId, "warning") },
+                ruleToCustomConfig: null,
+                checkCandidatePath));
+
+            string projectCheckBuildLog = RunnerUtilities.ExecBootstrapedMSBuild(
+                $"{Path.Combine(checkCandidatePath, $"{checkCandidate}.csproj")} /m:1 -nr:False -restore -check -verbosity:n", out bool success);
+
+            success.ShouldBeTrue();
+            projectCheckBuildLog.ShouldContain(expectedMessage);
+            projectCheckBuildLog.ShouldNotContain("This check should have been disabled");
+
+            // Cleanup
+            File.Delete(editorConfigName);
+        }
+    }
+
     [Theory]
     [InlineData(true)]
     [InlineData(false)]

src/BuildCheck.UnitTests/Microsoft.Build.BuildCheck.UnitTests.csproj

@@ -20,6 +20,7 @@
     <ProjectReference Include=".\TestAssets\CustomCheck\CustomCheck.csproj" />
     <ProjectReference Include=".\TestAssets\CustomCheck2\CustomCheck2.csproj" />
     <ProjectReference Include=".\TestAssets\InvalidCustomCheck\InvalidCustomCheck.csproj" />
+    <ProjectReference Include=".\TestAssets\ErrorCustomCheck\ErrorCustomCheck.csproj" />
   </ItemGroup>
 
   <ItemGroup>
@@ -45,4 +46,8 @@
     </None>
   </ItemGroup>
 
+  <ItemGroup>
+    <Compile Remove="TestAssets\ErrorCustomCheck\ErrorWhenRegisteringActions.cs" />
+  </ItemGroup>
+
 </Project>

src/BuildCheck.UnitTests/TestAssets/CheckCandidateWithMultipleChecksInjected/.editorconfigtest

@@ -2,3 +2,7 @@ root = true
 
 [*.csproj]
 build_check.X01234.Severity=X01234Severity
+
+build_check.X01236.Severity=X01236Severity
+build_check.X01237.Severity=X01237Severity
+build_check.X01238.Severity=X01238Severity

src/BuildCheck.UnitTests/TestAssets/CheckCandidateWithMultipleChecksInjected/CheckCandidateWithMultipleChecksInjected.csproj

@@ -9,6 +9,7 @@
     <PackageReference Include="CustomCheck" Version="1.0.0"/>
     <PackageReference Include="CustomCheck2" Version="1.0.0"/>
     <PackageReference Include="InvalidCustomCheck" Version="1.0.0"/>
+    <PackageReference Include="ErrorCustomCheck" Version="1.0.0"/>
   </ItemGroup>
 
   <ItemGroup>

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/ErrorCustomCheck.csproj

@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <Import Project="..\Common\CommonTest.props" />
+
+  <PropertyGroup>
+    <TargetFramework>netstandard2.0</TargetFramework>
+    <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <None Include="ErrorCustomCheck.props" Pack="true" PackagePath="build\ErrorCustomCheck.props" />
+    <Content Include="README.md" />
+  </ItemGroup>
+
+  <Import Project="..\Common\CommonTest.targets" />
+
+</Project>

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/ErrorCustomCheck.props

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project>
+  <PropertyGroup>
+	  <MSBuildCheck>$([MSBuild]::RegisterBuildCheck($(MSBuildThisFileDirectory)ErrorCustomCheck.dll))</MSBuildCheck>
+  </PropertyGroup>
+</Project>

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/ErrorOnInitializeCheck.cs

@@ -0,0 +1,40 @@
+using System;
+using System.Collections.Generic;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Experimental.BuildCheck;
+
+namespace ErrorCustomCheck
+{
+    public sealed class ErrorOnInitializeCheck : Check
+    {
+        public static CheckRule SupportedRule = new CheckRule(
+            "X01236",
+            "Title",
+            "Description",
+            "Message format: {0}",
+            new CheckConfiguration());
+
+        public override string FriendlyName => "ErrorOnInitializeCheck";
+
+        public override IReadOnlyList<CheckRule> SupportedRules { get; } = new List<CheckRule>() { SupportedRule };
+
+        public override void Initialize(ConfigurationContext configurationContext)
+        {
+            // configurationContext to be used only if check needs external configuration data.
+            throw new Exception("Something went wrong initializing");
+        }
+
+        public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
+        {
+            registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
+        }
+
+        private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedPropertiesCheckData> context)
+        {
+            context.ReportResult(BuildCheckResult.Create(
+                SupportedRule,
+                ElementLocation.EmptyLocation,
+                "This check should have been disabled"));
+        }
+    }
+}

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/ErrorOnRegisteredAction.cs

@@ -0,0 +1,36 @@
+using System;
+using System.Collections.Generic;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Experimental.BuildCheck;
+
+namespace ErrorCustomCheck
+{
+    public sealed class ErrorOnRegisteredAction : Check
+    {
+        public static CheckRule SupportedRule = new CheckRule(
+            "X01237",
+            "Title",
+            "Description",
+            "Message format: {0}",
+            new CheckConfiguration());
+
+        public override string FriendlyName => "ErrorOnEvaluatedPropertiesCheck";
+
+        public override IReadOnlyList<CheckRule> SupportedRules { get; } = new List<CheckRule>() { SupportedRule };
+
+        public override void Initialize(ConfigurationContext configurationContext)
+        {
+            // configurationContext to be used only if check needs external configuration data.
+        }
+
+        public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
+        {
+            registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
+        }
+
+        private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedPropertiesCheckData> context)
+        {
+            throw new Exception("something went wrong");
+        }
+    }
+}

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/ErrorWhenRegisteringActions.cs

@@ -0,0 +1,40 @@
+using System;
+using System.Collections.Generic;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Experimental.BuildCheck;
+
+namespace ErrorCustomCheck
+{
+    public sealed class ErrorWhenRegisteringActions : Check
+    {
+        public static CheckRule SupportedRule = new CheckRule(
+            "X01238",
+            "Title",
+            "Description",
+            "Message format: {0}",
+            new CheckConfiguration());
+
+        public override string FriendlyName => "ErrorOnEvaluatedPropertiesCheck";
+
+        public override IReadOnlyList<CheckRule> SupportedRules { get; } = new List<CheckRule>() { SupportedRule };
+
+        public override void Initialize(ConfigurationContext configurationContext)
+        {
+            // configurationContext to be used only if check needs external configuration data.
+        }
+
+        public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
+        {
+            registrationContext.RegisterEvaluatedPropertiesAction(EvaluatedPropertiesAction);
+            throw new Exception("something went wrong");
+        }
+
+        private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedPropertiesCheckData> context)
+        {
+            context.ReportResult(BuildCheckResult.Create(
+                SupportedRule,
+                ElementLocation.EmptyLocation,
+                "This check should have been disabled"));
+        }
+    }
+}

src/BuildCheck.UnitTests/TestAssets/ErrorCustomCheck/README.md

@@ -0,0 +1,21 @@
+# MSBuild Custom Check Template
+
+## Overview
+MSBuild Custom Check Template is a .NET template designed to streamline the creation of MSBuild check libraries. This template facilitates the development of custom checks targeting .NET Standard, enabling developers to inspect and enforce conventions, standards, or patterns within their MSBuild builds.
+
+## Features
+- Simplified template for creating MSBuild check libraries.
+- Targeting .NET Standard for cross-platform compatibility.
+- Provides a starting point for implementing custom check rules.
+
+## Getting Started
+To use the MSBuild Custom Check Template, follow these steps:
+1. Install the template using the following command:
+   ```bash
+   dotnet new install msbuildcheck
+2. Instantiate a custom template:
+   ```bash
+   dotnet new msbuildcheck -n <ProjectName>
+
+### Prerequisites
+- .NET SDK installed on your machine.