Do not output helplink for custom check diags

src/Build/BuildCheck/API/BuildCheckResult.cs

@@ -18,6 +18,9 @@ public sealed class BuildCheckResult : IBuildCheckResult
 {
     public static BuildCheckResult Create(CheckRule rule, IMSBuildElementLocation location, params string[] messageArgs) => new BuildCheckResult(rule, location, messageArgs);
 
+    internal static BuildCheckResult CreateBuiltIn(CheckRule rule, IMSBuildElementLocation location,
+        params string[] messageArgs) => new BuildCheckResult(rule, location, messageArgs) { _isBuiltIn = true };
+
     public BuildCheckResult(CheckRule checkConfig, IMSBuildElementLocation location, string[] messageArgs)
     {
         CheckRule = checkConfig;
@@ -47,9 +50,14 @@ internal BuildEventArgs ToEventArgs(CheckResultSeverity severity)
 
     public string MessageFormat => CheckRule.MessageFormat;
 
-    // Here we will provide different link for built-in rules and custom rules - once we have the base classes differentiated.
     public string FormatMessage() =>
-        _message ??= $"https://aka.ms/buildcheck/codes#{CheckRule.Id} - {string.Format(CheckRule.MessageFormat, MessageArgs)}";
+        _message ??= _isBuiltIn
+            // Builtin rules get unified helplink.
+            ? $"https://aka.ms/buildcheck/codes#{CheckRule.Id} - {string.Format(CheckRule.MessageFormat, MessageArgs)}"
+            // Custom rules can provide their own helplink.
+            : (!string.IsNullOrEmpty(CheckRule.HelpLinkUri) ? $"{CheckRule.HelpLinkUri} - " : null) +
+              string.Format(CheckRule.MessageFormat, MessageArgs);
 
     private string? _message;
+    private bool _isBuiltIn;
 }

src/Build/BuildCheck/API/CheckRule.cs

@@ -10,6 +10,39 @@ namespace Microsoft.Build.Experimental.BuildCheck;
 /// </summary>
 public class CheckRule
 {
+    /// <summary>
+    /// Creates the descriptor of the BuildCheck rule
+    /// </summary>
+    /// <param name="id">The id of the rule - used to denote the violation in the outputs</param>
+    /// <param name="title">The title of the rule - currently unused</param>
+    /// <param name="description">The detailed description of the rule - currently unused</param>
+    /// <param name="messageFormat">The message format to be used during reporting the violation.</param>
+    /// <param name="defaultConfiguration">The default config of this rule - applicable if user doesn't specify custom values in .editorconfig.</param>
+    /// <param name="helpLinkUri">Optional link to more detailed help for the violation.</param>
+    public CheckRule(
+        string id,
+        string title,
+        string description,
+        string messageFormat,
+        CheckConfiguration defaultConfiguration,
+        string helpLinkUri)
+    {
+        Id = id;
+        Title = title;
+        Description = description;
+        MessageFormat = messageFormat;
+        DefaultConfiguration = defaultConfiguration;
+        HelpLinkUri = helpLinkUri;
+    }
+
+    /// <summary>
+    /// Creates the descriptor of the BuildCheck rule
+    /// </summary>
+    /// <param name="id">The id of the rule - used to denote the violation in the outputs</param>
+    /// <param name="title">The title of the rule - currently unused</param>
+    /// <param name="description">The detailed description of the rule - currently unused</param>
+    /// <param name="messageFormat">The message format to be used during reporting the violation.</param>
+    /// <param name="defaultConfiguration">The default config of this rule - applicable if user doesn't specify custom values in .editorconfig.</param>
     public CheckRule(
         string id,
         string title,
@@ -51,6 +84,8 @@ public CheckRule(
     /// </summary>
     public string MessageFormat { get; }
 
+    public string HelpLinkUri { get; } = string.Empty;
+
     /// <summary>
     /// The default configuration - overridable by the user via .editorconfig.
     /// If no user specified configuration is provided, this default will be used.

src/Build/BuildCheck/API/InternalCheck.cs → src/Build/BuildCheck/API/WorkerNodeCheck.cs

@@ -6,7 +6,7 @@
 
 namespace Microsoft.Build.Experimental.BuildCheck.Checks;
 
-internal abstract class InternalCheck : Check
+internal abstract class WorkerNodeCheck : Check
 {
     /// <summary>
     /// Used by the implementors to subscribe to data and events they are interested in.

src/Build/BuildCheck/Checks/DoubleWritesCheck.cs

@@ -114,7 +114,7 @@ private void CheckWrite(BuildCheckDataContext<TaskInvocationCheckData> context,
 
             if (_filesWritten.TryGetValue(fileBeingWritten, out (string projectFilePath, string taskName) existingEntry))
             {
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     SupportedRule,
                     context.Data.TaskInvocationLocation,
                     context.Data.TaskName,

src/Build/BuildCheck/Checks/NoEnvironmentVariablePropertyCheck.cs

@@ -63,7 +63,7 @@ private void ProcessEnvironmentVariableReadAction(BuildCheckDataContext<Environm
             }
             else if (CheckScopeClassifier.IsActionInObservedScope(_scope, context.Data.EnvironmentVariableLocation.File, context.Data.ProjectFilePath))
             {
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     SupportedRule,
                     context.Data.EnvironmentVariableLocation,
                     GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue)));
@@ -88,7 +88,7 @@ private void HandleScopeReadiness()
                 continue;
             }
 
-            context.ReportResult(BuildCheckResult.Create(
+            context.ReportResult(BuildCheckResult.CreateBuiltIn(
                 SupportedRule,
                 context.Data.EnvironmentVariableLocation,
                 GetFormattedMessage(context.Data.EnvironmentVariableName, context.Data.EnvironmentVariableValue)));

src/Build/BuildCheck/Checks/PropertiesUsageCheck.cs

@@ -14,7 +14,7 @@
 
 namespace Microsoft.Build.Experimental.BuildCheck.Checks;
 
-internal class PropertiesUsageCheck : InternalCheck
+internal class PropertiesUsageCheck : WorkerNodeCheck
 {
     private static readonly CheckRule _usedBeforeInitializedRule = new CheckRule("BC0201", "PropertyUsedBeforeDeclared",
         ResourceUtilities.GetResourceString("BuildCheck_BC0201_Title")!,
@@ -148,7 +148,7 @@ private void ProcessPropertyWrite(BuildCheckDataContext<PropertyWriteData> conte
             {
                 _uninitializedReadsInScope.Remove(writeData.PropertyName);
 
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     _initializedAfterUsedRule,
                     uninitInScopeReadLocation,
                     writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty));
@@ -160,7 +160,7 @@ private void ProcessPropertyWrite(BuildCheckDataContext<PropertyWriteData> conte
             {
                 _uninitializedReadsOutOfScope.Remove(writeData.PropertyName);
 
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     _initializedAfterUsedRule,
                     uninitOutScopeReadLocation,
                     writeData.PropertyName, writeData.ElementLocation?.LocationString ?? string.Empty));
@@ -205,7 +205,7 @@ private void ProcessPropertyRead(BuildCheckDataContext<PropertyReadData> context
                          readData.ElementLocation, readData.ProjectFilePath))
             {
                 // report immediately
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     _usedBeforeInitializedRule,
                     readData.ElementLocation,
                     readData.PropertyName));
@@ -220,7 +220,7 @@ private void DoneWithProject(BuildCheckDataContext<ProjectRequestProcessingDoneD
         {
             if (propWithLocation.Value != null && !_readProperties.Contains(propWithLocation.Key))
             {
-                context.ReportResult(BuildCheckResult.Create(
+                context.ReportResult(BuildCheckResult.CreateBuiltIn(
                     _unusedPropertyRule,
                     propWithLocation.Value,
                     propWithLocation.Key));
@@ -231,7 +231,7 @@ private void DoneWithProject(BuildCheckDataContext<ProjectRequestProcessingDoneD
         //  uninitialized reads immediately (instead we wait if they are attempted to be initialized late).
         foreach (var uninitializedRead in _uninitializedReadsInScope)
         {
-            context.ReportResult(BuildCheckResult.Create(
+            context.ReportResult(BuildCheckResult.CreateBuiltIn(
                 _usedBeforeInitializedRule,
                 uninitializedRead.Value,
                 uninitializedRead.Key));

src/Build/BuildCheck/Checks/SharedOutputPathCheck.cs

@@ -83,7 +83,7 @@ private void EvaluatedPropertiesAction(BuildCheckDataContext<EvaluatedProperties
 
         if (_projectsPerOutputPath.TryGetValue(path!, out string? conflictingProject))
         {
-            context.ReportResult(BuildCheckResult.Create(
+            context.ReportResult(BuildCheckResult.CreateBuiltIn(
                 SupportedRule,
                 // Populating precise location tracked via https://github.com/orgs/dotnet/projects/373/views/1?pane=issue&itemId=58661732
                 ElementLocation.EmptyLocation,

src/BuildCheck.UnitTests/EndToEndTests.cs

@@ -487,9 +487,9 @@ public void CustomCheckTest_NoEditorConfig(string checkCandidate, string[] expec
         }
     }
 
-    [Theory(Skip = "https://github.com/dotnet/msbuild/issues/10702")]
-    [InlineData("CheckCandidate", "X01234", "error", "error X01234")]
-    [InlineData("CheckCandidateWithMultipleChecksInjected", "X01234", "warning", "warning X01234")]
+    [Theory]
+    [InlineData("CheckCandidate", "X01234", "error", "error X01234: http://samplelink.com/X01234")]
+    [InlineData("CheckCandidateWithMultipleChecksInjected", "X01234", "warning", "warning X01234: http://samplelink.com/X01234")]
     public void CustomCheckTest_WithEditorConfig(string checkCandidate, string ruleId, string severity, string expectedMessage)
     {
         using (var env = TestEnvironment.Create())
@@ -514,7 +514,7 @@ public void CustomCheckTest_WithEditorConfig(string checkCandidate, string ruleI
         }
     }
 
-    [Theory(Skip = "https://github.com/dotnet/msbuild/issues/10702")]
+    [Theory]
     [InlineData("X01236", "Something went wrong initializing")]
     // These tests are for failure one different points, will be addressed in a different PR
     // https://github.com/dotnet/msbuild/issues/10522

src/BuildCheck.UnitTests/TestAssets/CustomCheck/Check1.cs

@@ -14,7 +14,8 @@ public sealed class Check1 : Check
             "Title",
             "Description",
             "Message format: {0}",
-            new CheckConfiguration());
+            new CheckConfiguration(),
+            "http://samplelink.com/X01234");
 
         public override string FriendlyName => "CustomRule1";
 

template_feed/content/Microsoft.CheckTemplate/Check1.cs

@@ -11,7 +11,8 @@ public sealed class Check1 : Check
             "Title",
             "Description",
             "Message format: {0}",
-            new CheckConfiguration());
+            new CheckConfiguration(),
+            "http://sampleHelpLink.com/X01234");
 
         public override string FriendlyName => "Company.Check1";