Fix missing project dependency detection (#421)

* Add multiple target frameworks to ProjectA and ProjectB

This demonstrates that project dependency detection has a bug and makes ShouldDetectProjectChanges fail.

* Fix missing project dependency detection

When multiple target frameworks are defined, the dictionary of source file to project must keep all the project ids. Choosing just one with `.First()` can lead to failure because the project id might correspond to a target framework that does not match the target framework of the referencing project.

---------

Co-authored-by: Aaron Stannard <aaron@petabridge.com>

Author: 0xced

src/Incrementalist.Cmd/Commands/EmitDependencyGraphTask.cs

@@ -115,20 +115,20 @@ private BuildAnalysisResult FilterBuildResult(BuildAnalysisResult original, IRea
             }
 
             // Log the breakdown of modified files by type
-            var modifiedSourceFiles = affectedFiles.Count(x => x.Value.FileType == FileType.Code);
-            var modifiedProjectFiles = affectedFiles.Count(x => x.Value.FileType == FileType.Project);
-            var modifiedSolutionFiles = affectedFiles.Count(x => x.Value.FileType == FileType.Solution);
-            var modifiedScriptFiles = affectedFiles.Count(x => x.Value.FileType == FileType.Script);
-            var modifiedOtherFiles = affectedFiles.Count(x => x.Value.FileType == FileType.Other);
+            var modifiedSourceFiles = affectedFiles.Count(x => x.Value.Any(f => f.FileType == FileType.Code));
+            var modifiedProjectFiles = affectedFiles.Count(x => x.Value.Any(f => f.FileType == FileType.Project));
+            var modifiedSolutionFiles = affectedFiles.Count(x => x.Value.Any(f => f.FileType == FileType.Solution));
+            var modifiedScriptFiles = affectedFiles.Count(x => x.Value.Any(f => f.FileType == FileType.Script));
+            var modifiedOtherFiles = affectedFiles.Count(x => x.Value.Any(f => f.FileType == FileType.Other));
             Logger.LogInformation(
                 "Modified files breakdown: {SourceFiles} source files, {ProjectFiles} project files, {SolutionFiles} solution files, {ScriptFiles} script files, {OtherFiles} other files",
                 modifiedSourceFiles, modifiedProjectFiles, modifiedSolutionFiles, modifiedScriptFiles,
                 modifiedOtherFiles);
 
             // Check if any of the affected files require a solution-wide build
             Logger.LogInformation("Analyzing solution-wide impact...");
-            var projectFiles = allFiles.Where(x => x.Value.FileType == FileType.Project)
-                .Select(pair => new SlnFileWithPath(pair.Key, pair.Value))
+            var projectFiles = allFiles.Where(x => x.Value is [{ FileType: FileType.Project }])
+                .Select(pair => new SlnFileWithPath(pair.Key, pair.Value[0]))
                 .ToList();
             var projectImports = ProjectImportsFinder.FindProjectImports(projectFiles);
             var importDetector = new SolutionWideChangeDetector(projectImports);
@@ -140,7 +140,7 @@ private BuildAnalysisResult FilterBuildResult(BuildAnalysisResult original, IRea
             }
 
             // Get the list of affected project files directly
-            var directlyAffectedProjects = affectedFiles.Where(x => x.Value.FileType == FileType.Project)
+            var directlyAffectedProjects = affectedFiles.Where(x => x.Value.Any(f => f.FileType == FileType.Project))
                 .Select(x => x.Key)
                 .ToList();
 

src/Incrementalist.Tests/Dependencies/EmitDependencyGraphSpecs.cs

@@ -49,13 +49,18 @@ private static TestSolutionModel CreateSolution()
             .AddFolder("src", f1Builder =>
             {
                 f1Builder.AddProject(ProjectA,
-                    (_, p1Builder) => { p1Builder.WithFile("HelloWorld.cs", CsharpSamples.HelloClass); });
+                    (_, p1Builder) =>
+                    {
+                        p1Builder.WithFile("HelloWorld.cs", CsharpSamples.HelloClass);
+                        p1Builder.WithTargetFrameworks([TargetFramework.Net8, TargetFramework.Net9, TargetFramework.NetStandard2_0, TargetFramework.NetStandard2_1]);
+                    });
 
                 f1Builder.AddProject(ProjectB, (otherProjects, p2Builder) =>
                 {
                     p2Builder.WithFile("GoodBye.cs", CsharpSamples.GoodbyeClassWithNamespace);
                     var projectA = otherProjects.First(p => p.NameWithoutExtension == ProjectA);
                     p2Builder.WithProjectReference(projectA);
+                    p2Builder.WithTargetFrameworks([TargetFramework.Net8, TargetFramework.Net9, TargetFramework.NetStandard2_0, TargetFramework.NetStandard2_1]);
                 });
 
                 // a third project, C, with no references to anyone else
@@ -69,6 +74,7 @@ private static TestSolutionModel CreateSolution()
                     p3Builder.WithFile("HelloWorldTests.cs", CsharpSamples.BarClass);
                     var projectB = otherProjects.First(p => p.NameWithoutExtension == ProjectB);
                     p3Builder.WithProjectReference(projectB);
+                    p3Builder.WithTargetFrameworks([TargetFramework.Net9]);
                 });
             })
             .Build();

src/Incrementalist.Tests/Dependencies/ProjectImportsTrackingSpecs.cs

@@ -75,9 +75,9 @@ public async Task Should_mark_project_as_changed_when_only_imported_file_changed
 
             var cmd = new FilterAffectedProjectFilesCmd(new TestOutputLogger(_outputHelper), CancellationToken.None,
                 Repository.BasePath, "master");
-            var solutionFiles = new Dictionary<AbsolutePath, SlnFile>()
+            var solutionFiles = new Dictionary<AbsolutePath, SlnFile[]>()
             {
-                [projectFilePath] = new SlnFile(FileType.Project, ProjectId.CreateNewId())
+                [projectFilePath] = [new SlnFile(FileType.Project, ProjectId.CreateNewId())]
             };
             var filteredAffectedFiles = await cmd.Process(Task.FromResult(solutionFiles));
             Assert.Single(filteredAffectedFiles);

src/Incrementalist/ProjectSystem/Cmds/ComputeDependencyGraphCmd.cs

@@ -20,7 +20,7 @@ namespace Incrementalist.ProjectSystem.Cmds
     ///     and emits a topologically sorted set of project file names to be used during testing.
     /// </summary>
     public sealed class
-        ComputeDependencyGraphCmd : BuildCommandBase<Dictionary<AbsolutePath, SlnFile>,
+        ComputeDependencyGraphCmd : BuildCommandBase<Dictionary<AbsolutePath, SlnFile[]>,
         Dictionary<AbsolutePath, ICollection<AbsolutePath>>>
     {
         private readonly Solution _solution;
@@ -32,7 +32,7 @@ public ComputeDependencyGraphCmd(ILogger logger, CancellationToken cancellationT
         }
 
         protected override async Task<Dictionary<AbsolutePath, ICollection<AbsolutePath>>> ProcessImpl(
-            Task<Dictionary<AbsolutePath, SlnFile>> previousTask)
+            Task<Dictionary<AbsolutePath, SlnFile[]>> previousTask)
         {
             var affectedSlnFiles = await previousTask;
 
@@ -51,9 +51,9 @@ protected override async Task<Dictionary<AbsolutePath, ICollection<AbsolutePath>
              * We have to gather up each unique project file separately in this case.
              */
             List<ProjectId> additionalProjectIds = [];
-            if (affectedSlnFiles.Any(x => x.Value.FileType == FileType.Project))
+            if (affectedSlnFiles.Any(x => x.Value.Any(f => f.FileType == FileType.Project)))
             {
-                foreach (var proj in affectedSlnFiles.Where(x => x.Value.FileType == FileType.Project))
+                foreach (var proj in affectedSlnFiles.Where(x => x.Value.Any(f => f.FileType == FileType.Project)))
                     additionalProjectIds.AddRange(_solution.Projects
                         .Where(x => x.FilePath != null && x.FilePath.Equals(proj.Key.Path)).Select(x => x.Id));
             }
@@ -78,8 +78,8 @@ protected override async Task<Dictionary<AbsolutePath, ICollection<AbsolutePath>
             }
 
             var uniqueProjectIds = affectedSlnFiles
-                .Where(c => c.Value.ProjectId != null)
-                .Select(x => x.Value.ProjectId!).Concat(additionalProjectIds)
+                .Where(c => c.Value.All(f => f.ProjectId != null))
+                .SelectMany(x => x.Value.Select(f => f.ProjectId!)).Concat(additionalProjectIds)
                 .Distinct().ToList();
 
             Logger.LogDebug("Evaluating {Count} unique project IDs.", uniqueProjectIds.Count);

src/Incrementalist/ProjectSystem/Cmds/FilterAffectedProjectFilesCmd.cs

@@ -21,8 +21,8 @@ namespace Incrementalist.ProjectSystem.Cmds
     ///     they were touched via the GitDiff.
     /// </summary>
     public sealed class
-        FilterAffectedProjectFilesCmd : BuildCommandBase<Dictionary<AbsolutePath, SlnFile>,
-        Dictionary<AbsolutePath, SlnFile>>
+        FilterAffectedProjectFilesCmd : BuildCommandBase<Dictionary<AbsolutePath, SlnFile[]>,
+        Dictionary<AbsolutePath, SlnFile[]>>
     {
         private readonly string _targetGitBranch;
         private readonly AbsolutePath _workingDirectory;
@@ -35,8 +35,8 @@ public FilterAffectedProjectFilesCmd(ILogger logger, CancellationToken cancellat
             _targetGitBranch = targetGitBranch;
         }
 
-        protected override async Task<Dictionary<AbsolutePath, SlnFile>> ProcessImpl(
-            Task<Dictionary<AbsolutePath, SlnFile>> previousTask)
+        protected override async Task<Dictionary<AbsolutePath, SlnFile[]>> ProcessImpl(
+            Task<Dictionary<AbsolutePath, SlnFile[]>> previousTask)
         {
             var fileDictObj = await previousTask;
 
@@ -45,28 +45,28 @@ protected override async Task<Dictionary<AbsolutePath, SlnFile>> ProcessImpl(
             {
                 Logger.LogError("Unable to find Git repository located in {WorkingDirectory}. Shutting down.",
                     _workingDirectory);
-                return new Dictionary<AbsolutePath, SlnFile>();
+                return [];
             }
 
             // validate the target branch
             if (!DiffHelper.HasBranch(repo, _targetGitBranch))
             {
                 Logger.LogError("Current git repository doesn't have any branch named [{TargetBranch}]. Shutting down.",
                     _targetGitBranch);
-                return new Dictionary<AbsolutePath, SlnFile>();
+                return [];
             }
 
             var affectedFiles = DiffHelper.ChangedFiles(repo, _targetGitBranch).ToList();
 
-            var projectFiles = fileDictObj.Where(x => x.Value.FileType == FileType.Project).ToList();
+            var projectFiles = fileDictObj.Where(x => x.Value is [{ FileType: FileType.Project }]).ToList();
             var projectFolders = projectFiles.Where(x => Path.GetDirectoryName(x.Key.Path) is not null)
                 .ToLookup(x => new AbsolutePath(Path.GetDirectoryName(x.Key.Path)!), v => Tuple.Create(v.Key, v.Value));
             var projectImports =
                 ProjectImportsFinder.FindProjectImports(projectFiles.Select(pair =>
-                    new SlnFileWithPath(pair.Key, pair.Value)));
+                    new SlnFileWithPath(pair.Key, pair.Value[0])));
 
             // filter out any files that aren't affected by the diff
-            var newDict = new Dictionary<AbsolutePath, SlnFile>();
+            var newDict = new Dictionary<AbsolutePath, SlnFile[]>();
             foreach (var file in affectedFiles)
             {
                 Logger.LogDebug("Affected file: {FilePath}", file);
@@ -86,7 +86,7 @@ protected override async Task<Dictionary<AbsolutePath, SlnFile>> ProcessImpl(
                     if (SolutionWideChangeDetector.IsSolutionWideFile(file))
                     {
                         Logger.LogInformation("Adding solution-wide file {File} to the set of affected files.", file);
-                        newDict[file] = new SlnFile(FileType.Other, null);
+                        newDict[file] = [new SlnFile(FileType.Other, null)];
                         continue;
                     }
 
@@ -110,7 +110,7 @@ protected override async Task<Dictionary<AbsolutePath, SlnFile>> ProcessImpl(
                     // Mark all dependant as affected
                     foreach (var dependentProject in import.DependentProjects)
                     {
-                        newDict[dependentProject.Path] = dependentProject.File;
+                        newDict[dependentProject.Path] = [dependentProject.File];
                     }
                 }
             }

src/Incrementalist/ProjectSystem/Cmds/GatherAllFilesInSolutionCmd.cs

@@ -17,7 +17,7 @@ namespace Incrementalist.ProjectSystem.Cmds
     /// <summary>
     ///     Gathers all of the files in a solution and categorizes them.
     /// </summary>
-    public sealed class GatherAllFilesInSolutionCmd : BuildCommandBase<Solution, Dictionary<AbsolutePath, SlnFile>>
+    public sealed class GatherAllFilesInSolutionCmd : BuildCommandBase<Solution, Dictionary<AbsolutePath, SlnFile[]>>
     {
         private readonly AbsolutePath _workingDirectory;
 
@@ -28,7 +28,7 @@ public GatherAllFilesInSolutionCmd(ILogger logger, CancellationToken cancellatio
             _workingDirectory = workingDirectory;
         }
 
-        protected override async Task<Dictionary<AbsolutePath, SlnFile>> ProcessImpl(Task<Solution> previousTask)
+        protected override async Task<Dictionary<AbsolutePath, SlnFile[]>> ProcessImpl(Task<Solution> previousTask)
         {
             var slnObject = await previousTask;
             Contract.Assert(slnObject is not null,

src/Incrementalist/ProjectSystem/SolutionAnalyzer.cs

@@ -44,7 +44,7 @@ public static class SolutionAnalyzer
         /// <param name="sln">The Solution file.</param>
         /// <param name="workingFolder"></param>
         /// <returns>A flattened list of all files inside the solution.</returns>
-        public static Dictionary<AbsolutePath, SlnFile> AllSolutionFiles(Solution sln, AbsolutePath workingFolder)
+        public static Dictionary<AbsolutePath, SlnFile[]> AllSolutionFiles(Solution sln, AbsolutePath workingFolder)
         {
             // throw if the solution's file path is null
             ArgumentNullException.ThrowIfNull(sln.FilePath, nameof(sln.FilePath));
@@ -55,17 +55,17 @@ public static Dictionary<AbsolutePath, SlnFile> AllSolutionFiles(Solution sln, A
                     document => new SlnFile(
                         document.SourceCodeKind == SourceCodeKind.Regular ? FileType.Code : FileType.Script,
                         document.Project.Id))
-                .ToDictionary(x => new AbsolutePath(Path.GetFullPath(x.Key!)), x => x.First()).ToList()
+                .ToDictionary(x => new AbsolutePath(Path.GetFullPath(x.Key!)), x => x.ToArray())
                 .Concat(sln.Projects.Where(x => x.FilePath != null).Select(x =>
-                        new KeyValuePair<AbsolutePath, SlnFile>(new AbsolutePath(Path.GetFullPath(x.FilePath!)),
-                            new SlnFile(FileType.Project, x.Id)))
+                        new KeyValuePair<AbsolutePath, SlnFile[]>(new AbsolutePath(Path.GetFullPath(x.FilePath!)),
+                            [new SlnFile(FileType.Project, x.Id)]))
                     .Concat([
-                        new KeyValuePair<AbsolutePath, SlnFile>
-                            (new AbsolutePath(Path.GetFullPath(sln.FilePath)), new SlnFile(FileType.Solution, null))
+                        new KeyValuePair<AbsolutePath, SlnFile[]>
+                            (new AbsolutePath(Path.GetFullPath(sln.FilePath)), [new SlnFile(FileType.Solution, null)])
                     ]));
 
             // need to de-duplicate
-            var finalFiles = new Dictionary<AbsolutePath, SlnFile>();
+            var finalFiles = new Dictionary<AbsolutePath, SlnFile[]>();
             foreach (var file in allPossibleFiles)
             {
                 finalFiles[file.Key] = file.Value;