[BuildCheck] Implement double writes analyzer (#10184)

Fixes #9881

### Context

We believe there is value in flagging cases where the same file is written by more than one task during a build. dotnet/source-build#4390 is an example of a recent hard-to-diagnose build issue that could have been prevented by this analyzer.

### Changes Made

This PR adds a new built-in analyzer and makes a few supporting changes.

### Testing

- Existing and new unit tests.
- Built a few larger repos with the analyzer enabled.

### Notes

The changes contain a fix/workaround for #10176.

src/Build/BuildCheck/Analyzers/DoubleWritesAnalyzer.cs

@@ -0,0 +1,127 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Diagnostics;
+using System.IO;
+using System.Linq;
+using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Experimental.BuildCheck;
+using static Microsoft.Build.Experimental.BuildCheck.TaskInvocationAnalysisData;
+
+#if FEATURE_MSIOREDIST
+using Path = Microsoft.IO.Path;
+#endif
+
+namespace Microsoft.Build.Experimental.BuildCheck.Analyzers;
+
+internal sealed class DoubleWritesAnalyzer : BuildAnalyzer
+{
+    public static BuildAnalyzerRule SupportedRule = new BuildAnalyzerRule("BC0102", "DoubleWrites",
+        "Two tasks should not write the same file",
+        "Tasks {0} and {1} from projects {2} and {3} write the same file: {4}.",
+        new BuildAnalyzerConfiguration() { Severity = BuildAnalyzerResultSeverity.Warning, IsEnabled = true });
+
+    public override string FriendlyName => "MSBuild.DoubleWritesAnalyzer";
+
+    public override IReadOnlyList<BuildAnalyzerRule> SupportedRules { get; } = [SupportedRule];
+
+    public override void Initialize(ConfigurationContext configurationContext)
+    {
+        /* This is it - no custom configuration */
+    }
+
+    public override void RegisterActions(IBuildCheckRegistrationContext registrationContext)
+    {
+        registrationContext.RegisterTaskInvocationAction(TaskInvocationAction);
+    }
+
+    /// <summary>
+    /// Contains the first project file + task that wrote the given file during the build.
+    /// </summary>
+    private readonly Dictionary<string, (string projectFilePath, string taskName)> _filesWritten = new(StringComparer.CurrentCultureIgnoreCase);
+
+    private void TaskInvocationAction(BuildCheckDataContext<TaskInvocationAnalysisData> context)
+    {
+        // This analyzer uses a hard-coded list of tasks known to write files.
+        switch (context.Data.TaskName)
+        {
+            case "Csc":
+            case "Vbc":
+            case "Fsc": AnalyzeCompilerTask(context); break;
+            case "Copy": AnalyzeCopyTask(context); break;
+        }
+    }
+
+    private void AnalyzeCompilerTask(BuildCheckDataContext<TaskInvocationAnalysisData> context)
+    {
+        var taskParameters = context.Data.Parameters;
+
+        // Compiler tasks have several parameters representing files being written.
+        AnalyzeParameter("OutputAssembly");
+        AnalyzeParameter("OutputRefAssembly");
+        AnalyzeParameter("DocumentationFile");
+        AnalyzeParameter("PdbFile");
+
+        void AnalyzeParameter(string parameterName)
+        {
+            if (taskParameters.TryGetValue(parameterName, out TaskParameter? taskParameter))
+            {
+                string outputPath = taskParameter.EnumerateStringValues().FirstOrDefault() ?? "";
+                AnalyzeWrite(context, outputPath);
+            }
+        }
+    }
+
+    private void AnalyzeCopyTask(BuildCheckDataContext<TaskInvocationAnalysisData> context)
+    {
+        var taskParameters = context.Data.Parameters;
+
+        // The destination is specified as either DestinationFolder or DestinationFiles.
+        if (taskParameters.TryGetValue("SourceFiles", out TaskParameter? sourceFiles) &&
+            taskParameters.TryGetValue("DestinationFolder", out TaskParameter? destinationFolder))
+        {
+            string destinationFolderPath = destinationFolder.EnumerateStringValues().FirstOrDefault() ?? "";
+            foreach (string sourceFilePath in sourceFiles.EnumerateStringValues())
+            {
+                AnalyzeWrite(context, Path.Combine(destinationFolderPath, Path.GetFileName(sourceFilePath)));
+            }
+        }
+        else if (taskParameters.TryGetValue("DestinationFiles", out TaskParameter? destinationFiles))
+        {
+            foreach (string destinationFilePath in destinationFiles.EnumerateStringValues())
+            {
+                AnalyzeWrite(context, destinationFilePath);
+            }
+        }
+    }
+
+    private void AnalyzeWrite(BuildCheckDataContext<TaskInvocationAnalysisData> context, string fileBeingWritten)
+    {
+        if (!string.IsNullOrEmpty(fileBeingWritten))
+        {
+            // Absolutize the path. Note that if a path used during a build is relative, it is relative to the directory
+            // of the project being built, regardless of the project/import in which it occurs.
+            fileBeingWritten = Path.GetFullPath(fileBeingWritten, context.Data.ProjectFileDirectory);
+
+            if (_filesWritten.TryGetValue(fileBeingWritten, out (string projectFilePath, string taskName) existingEntry))
+            {
+                context.ReportResult(BuildCheckResult.Create(
+                    SupportedRule,
+                    context.Data.TaskInvocationLocation,
+                    context.Data.TaskName,
+                    existingEntry.taskName,
+                    Path.GetFileName(context.Data.ProjectFilePath),
+                    Path.GetFileName(existingEntry.projectFilePath),
+                    fileBeingWritten));
+            }
+            else
+            {
+                _filesWritten.Add(fileBeingWritten, (context.Data.ProjectFilePath, context.Data.TaskName));
+            }
+        }
+   }
+}

src/Build/BuildCheck/Infrastructure/BuildCheckManagerProvider.cs

@@ -133,7 +133,8 @@ private static readonly (string[] ruleIds, bool defaultEnablement, BuildAnalyzer
         [
             // BuildCheckDataSource.EventArgs
             [
-                ([SharedOutputPathAnalyzer.SupportedRule.Id], SharedOutputPathAnalyzer.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<SharedOutputPathAnalyzer>)
+                ([SharedOutputPathAnalyzer.SupportedRule.Id], SharedOutputPathAnalyzer.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<SharedOutputPathAnalyzer>),
+                ([DoubleWritesAnalyzer.SupportedRule.Id], DoubleWritesAnalyzer.SupportedRule.DefaultConfiguration.IsEnabled ?? false, Construct<DoubleWritesAnalyzer>),
             ],
             // BuildCheckDataSource.Execution
             []

src/Build/BuildCheck/Infrastructure/BuildEventsProcessor.cs

@@ -38,13 +38,23 @@ private struct ExecutingTaskData
         public Dictionary<string, TaskInvocationAnalysisData.TaskParameter> TaskParameters;
     }
 
+    /// <summary>
+    /// Uniquely identifies a task.
+    /// </summary>
+    private record struct TaskKey(int ProjectContextId, int TargetId, int TaskId)
+    {
+        public TaskKey(BuildEventContext context)
+            : this(context.ProjectContextId, context.TargetId, context.TaskId)
+        { }
+    }
+
     private readonly SimpleProjectRootElementCache _cache = new SimpleProjectRootElementCache();
     private readonly BuildCheckCentralContext _buildCheckCentralContext = buildCheckCentralContext;
 
     /// <summary>
     /// Keeps track of in-flight tasks. Keyed by task ID as passed in <see cref="BuildEventContext.TaskId"/>.
     /// </summary>
-    private readonly Dictionary<int, ExecutingTaskData> _tasksBeingExecuted = [];
+    private readonly Dictionary<TaskKey, ExecutingTaskData> _tasksBeingExecuted = [];
 
     // This requires MSBUILDLOGPROPERTIESANDITEMSAFTEREVALUATION set to 1
     internal void ProcessEvaluationFinishedEventArgs(
@@ -105,7 +115,7 @@ internal void ProcessTaskStartedEventArgs(
                     parameters: taskParameters),
             };
 
-            _tasksBeingExecuted.Add(taskStartedEventArgs.BuildEventContext.TaskId, taskData);
+            _tasksBeingExecuted.Add(new TaskKey(taskStartedEventArgs.BuildEventContext), taskData);
         }
     }
 
@@ -119,12 +129,15 @@ internal void ProcessTaskFinishedEventArgs(
             return;
         }
 
-        if (taskFinishedEventArgs.BuildEventContext is not null &&
-            _tasksBeingExecuted.TryGetValue(taskFinishedEventArgs.BuildEventContext.TaskId, out ExecutingTaskData taskData))
+        if (taskFinishedEventArgs?.BuildEventContext is not null)
         {
-            // All task parameters have been recorded by now so remove the task from the dictionary and fire the registered build check actions.
-            _tasksBeingExecuted.Remove(taskFinishedEventArgs.BuildEventContext.TaskId);
-            _buildCheckCentralContext.RunTaskInvocationActions(taskData.AnalysisData, buildAnalysisContext, ReportResult);
+            TaskKey taskKey = new TaskKey(taskFinishedEventArgs.BuildEventContext);
+            if (_tasksBeingExecuted.TryGetValue(taskKey, out ExecutingTaskData taskData))
+            {
+                // All task parameters have been recorded by now so remove the task from the dictionary and fire the registered build check actions.
+                _tasksBeingExecuted.Remove(taskKey);
+                _buildCheckCentralContext.RunTaskInvocationActions(taskData.AnalysisData, buildAnalysisContext, ReportResult);
+            }
         }
     }
 
@@ -147,7 +160,7 @@ internal void ProcessTaskParameterEventArgs(
         }
 
         if (taskParameterEventArgs.BuildEventContext is not null &&
-            _tasksBeingExecuted.TryGetValue(taskParameterEventArgs.BuildEventContext.TaskId, out ExecutingTaskData taskData))
+            _tasksBeingExecuted.TryGetValue(new TaskKey(taskParameterEventArgs.BuildEventContext), out ExecutingTaskData taskData))
         {
             // Add the parameter name and value to the matching entry in _tasksBeingExecuted. Parameters come typed as IList
             // but it's more natural to pass them as scalar values so we unwrap one-element lists.
@@ -187,7 +200,11 @@ private static void ReportResult(
         }
 
         BuildEventArgs eventArgs = result.ToEventArgs(config.Severity);
-        eventArgs.BuildEventContext = loggingContext.BuildEventContext;
+
+        // TODO: This is a workaround for https://github.com/dotnet/msbuild/issues/10176
+        // eventArgs.BuildEventContext = loggingContext.BuildEventContext;
+        eventArgs.BuildEventContext = BuildEventContext.Invalid;
+
         loggingContext.LogBuildEvent(eventArgs);
     }
 }

src/Build/BuildCheck/OM/BuildCheckDataContext.cs

@@ -10,6 +10,7 @@
 using Microsoft.Build.Experimental.BuildCheck.Infrastructure;
 using Microsoft.Build.Experimental;
 using Microsoft.Build.Framework;
+using System.IO;
 
 namespace Microsoft.Build.Experimental.BuildCheck;
 
@@ -19,10 +20,18 @@ namespace Microsoft.Build.Experimental.BuildCheck;
 /// <param name="projectFilePath">Currently built project.</param>
 public abstract class AnalysisData(string projectFilePath)
 {
+    private string? _projectFileDirectory;
+
     /// <summary>
     /// Full path to the project file being built.
     /// </summary>
     public string ProjectFilePath { get; } = projectFilePath;
+
+    /// <summary>
+    /// Directory path of the file being built (the containing directory of <see cref="ProjectFilePath"/>).
+    /// </summary>
+    public string ProjectFileDirectory =>
+        _projectFileDirectory ??= Path.GetDirectoryName(ProjectFilePath)!;
 }
 
 /// <summary>

src/Build/BuildCheck/OM/ParsedItemsAnalysisData.cs

@@ -6,6 +6,7 @@
 using System.Linq;
 using Microsoft.Build.Collections;
 using Microsoft.Build.Construction;
+using Microsoft.Build.Framework;
 
 namespace Microsoft.Build.Experimental.BuildCheck;
 
@@ -64,7 +65,46 @@ public sealed class TaskInvocationAnalysisData : AnalysisData
     /// in task parameters: <see cref="Framework.ITaskItem"/>, <see cref="Framework.ITaskItem"/>[],
     /// bool, string, or anything else convertible to/from string.</param>
     /// <param name="IsOutput">True for output parameters, false for input parameters.</param>
-    public record class TaskParameter(object? Value, bool IsOutput);
+    public record class TaskParameter(object? Value, bool IsOutput)
+    {
+        /// <summary>
+        /// Enumerates all values passed in this parameter. E.g. for Param="@(Compile)", this will return
+        /// all Compile items.
+        /// </summary>
+        public IEnumerable<object> EnumerateValues()
+        {
+            if (Value is System.Collections.IList list)
+            {
+                foreach (object obj in list)
+                {
+                    yield return obj;
+                }
+            }
+            else if (Value is object obj)
+            {
+                yield return obj;
+            }
+        }
+
+        /// <summary>
+        /// Enumerates all values passed in this parameter, converted to strings. E.g. for Param="@(Compile)",
+        /// this will return all Compile item specs.
+        /// </summary>
+        public IEnumerable<string> EnumerateStringValues()
+        {
+            foreach (object obj in EnumerateValues())
+            {
+                if (obj is ITaskItem taskItem)
+                {
+                    yield return taskItem.ItemSpec;
+                }
+                else
+                {
+                    yield return obj.ToString() ?? "";
+                }
+            }
+        }
+    }
 
     internal TaskInvocationAnalysisData(
         string projectFilePath,

src/Build/Microsoft.Build.csproj

@@ -159,6 +159,7 @@
     <Compile Include="BuildCheck\Acquisition\BuildCheckAcquisitionModule.cs" />
     <Compile Include="BuildCheck\Acquisition\IBuildCheckAcquisitionModule.cs" />
     <Compile Include="BuildCheck\Analyzers\SharedOutputPathAnalyzer.cs" />
+    <Compile Include="BuildCheck\Analyzers\DoubleWritesAnalyzer.cs" />
     <Compile Include="BuildCheck\Infrastructure\BuildCheckConfigurationException.cs" />
     <Compile Include="BuildCheck\Infrastructure\BuildCheckForwardingLogger.cs" />
     <Compile Include="BuildCheck\Infrastructure\BuildEventsProcessor.cs" />