Fix oversharing of build results in ResultsCache

.editorconfig

@@ -409,3 +409,6 @@ dotnet_diagnostic.IDE0290.severity = suggestion
 dotnet_diagnostic.IDE0300.severity = suggestion
 dotnet_diagnostic.IDE0301.severity = suggestion
 dotnet_diagnostic.IDE0305.severity = suggestion
+
+# Temporarily disable SA1010 "Opening square brackets should not be preceded by a space" until https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3687 is fixed
+dotnet_diagnostic.SA1010.severity = none

documentation/wiki/ChangeWaves.md

@@ -26,6 +26,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 ### 17.12
 - [Log TaskParameterEvent for scalar parameters](https://github.com/dotnet/msbuild/pull/9908)
 - [Convert.ToString during a property evaluation uses the InvariantCulture for all types](https://github.com/dotnet/msbuild/pull/9874)
+- [Fix oversharing of build results in ResultsCache](https://github.com/dotnet/msbuild/pull/9987)
 
 ### 17.10
 - [AppDomain configuration is serialized without using BinFmt](https://github.com/dotnet/msbuild/pull/9320) - feature can be opted out only if [BinaryFormatter](https://learn.microsoft.com/en-us/dotnet/api/system.runtime.serialization.formatters.binary.binaryformatter) is allowed at runtime by editing `MSBuild.runtimeconfig.json`

src/Build.UnitTests/BackEnd/RequestedProjectState_Tests.cs

@@ -0,0 +1,208 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Collections.Generic;
+using FluentAssertions;
+using Microsoft.Build.Execution;
+using Shouldly;
+using Xunit;
+using Xunit.Abstractions;
+
+namespace Microsoft.Build.UnitTests.BackEnd
+{
+    public class RequestedProjectState_Tests
+    {
+        [Fact]
+        public void DeepCloneEmpty()
+        {
+            RequestedProjectState state = new();
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeNull();
+        }
+
+        [Fact]
+        public void DeepCloneProperties()
+        {
+            List<string> properties = ["prop1", "prop2"];
+            RequestedProjectState state = new()
+            {
+                PropertyFilters = properties,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeEquivalentTo(properties);
+            clone.ItemFilters.Should().BeNull();
+
+            // Mutating the original instance is not reflected in the clone.
+            properties.Add("prop3");
+            clone.PropertyFilters.Count.Should().NotBe(properties.Count);
+        }
+
+        [Fact]
+        public void DeepCloneItemsNoMetadata()
+        {
+            Dictionary<string, List<string>> items = new()
+            {
+                { "item1", null! },
+                { "item2", null! },
+            };
+            RequestedProjectState state = new()
+            {
+                ItemFilters = items,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeEquivalentTo(items);
+
+            // Mutating the original instance is not reflected in the clone.
+            items.Add("item3", null!);
+            clone.ItemFilters.Count.Should().NotBe(items.Count);
+        }
+
+        [Fact]
+        public void DeepCloneItemsWithMetadata()
+        {
+            Dictionary<string, List<string>> items = new()
+            {
+                { "item1", ["metadatum1", "metadatum2"] },
+                { "item2", ["metadatum3"] },
+            };
+            RequestedProjectState state = new()
+            {
+                ItemFilters = items,
+            };
+            RequestedProjectState clone = state.DeepClone();
+
+            clone.PropertyFilters.Should().BeNull();
+            clone.ItemFilters.Should().BeEquivalentTo(items);
+
+            // Mutating the original instance is not reflected in the clone.
+            items["item2"].Add("metadatum4");
+            clone.ItemFilters["item2"].Count.Should().NotBe(items["item2"].Count);
+        }
+
+        [Fact]
+        public void IsSubsetOfEmpty()
+        {
+            RequestedProjectState state1 = new();
+            RequestedProjectState state2 = new();
+
+            // Empty instances are subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeTrue();
+
+            state1.PropertyFilters = ["prop1"];
+            state1.ItemFilters = new Dictionary<string, List<string>>()
+            {
+                { "item1", null! },
+            };
+
+            // Non-empty instance is a subset of empty instance but not the other way round.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfProperties()
+        {
+            RequestedProjectState state1 = new()
+            {
+                PropertyFilters = ["prop1"],
+            };
+            RequestedProjectState state2 = new()
+            {
+                PropertyFilters = ["prop1", "prop2"],
+            };
+
+            // "prop1" is a subset of "prop1", "prop2".
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.PropertyFilters.Add("prop3");
+
+            // Disjoint sets are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.PropertyFilters.Clear();
+
+            // Empty props is a subset of anything.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfItemsNoMetadata()
+        {
+            RequestedProjectState state1 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                },
+            };
+            RequestedProjectState state2 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                    { "item2", null! },
+                },
+            };
+
+            // "item1" is a subset of "item1", "item2".
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters.Add("item3", null!);
+
+            // Disjoint sets are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters.Clear();
+
+            // Empty items is a subset of anything.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+
+        [Fact]
+        public void IsSubsetOfItemsWithMetadata()
+        {
+            RequestedProjectState state1 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", ["metadatum1"] },
+                },
+            };
+            RequestedProjectState state2 = new()
+            {
+                ItemFilters = new Dictionary<string, List<string>>()
+                {
+                    { "item1", null! },
+                },
+            };
+
+            // "item1" with "metadatum1" is a subset of "item1" with no metadata filter.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state2.ItemFilters["item1"] = ["metadatum2"];
+
+            // Disjoint metadata filters are not subsets of each other.
+            state1.IsSubsetOf(state2).Should().BeFalse();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+
+            state1.ItemFilters["item1"] = [];
+
+            // Empty metadata filter is a subset of any other metadata filter.
+            state1.IsSubsetOf(state2).Should().BeTrue();
+            state2.IsSubsetOf(state1).Should().BeFalse();
+        }
+    }
+}

src/Build.UnitTests/BackEnd/ResultsCache_Tests.cs

@@ -3,8 +3,12 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
+using System.Xml;
 using Microsoft.Build.BackEnd;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Internal;
@@ -188,6 +192,153 @@ public void TestRetrieveSubsetTargetsFromResult()
             Assert.Equal(BuildResultCode.Success, response.Results.OverallResult);
         }
 
+        [Fact]
+        public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideProjectStateAfterBuild()
+        {
+            string targetName = "testTarget1";
+            int submissionId = 1;
+            int nodeRequestId = 0;
+            int configurationId = 1;
+
+            BuildRequest requestWithNoBuildDataFlags = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.None);
+
+            BuildRequest requestWithProjectStateFlag = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.ProvideProjectStateAfterBuild);
+
+            BuildRequest requestWithNoBuildDataFlags2 = new BuildRequest(
+               submissionId,
+               nodeRequestId,
+               configurationId,
+               new string[1] { targetName } /* escapedTargets */,
+               null /* hostServices */,
+               BuildEventContext.Invalid /* parentBuildEventContext */,
+               null /* parentRequest */,
+               BuildRequestDataFlags.None);
+
+            BuildResult resultForRequestWithNoBuildDataFlags = new(requestWithNoBuildDataFlags);
+            resultForRequestWithNoBuildDataFlags.AddResultsForTarget(targetName, BuildResultUtilities.GetEmptySucceedingTargetResult());
+            ResultsCache cache = new();
+            cache.AddResult(resultForRequestWithNoBuildDataFlags);
+
+            ResultsCacheResponse cacheResponseForRequestWithNoBuildDataFlags = cache.SatisfyRequest(
+               requestWithNoBuildDataFlags,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cachedResponseForProjectState = cache.SatisfyRequest(
+               requestWithProjectStateFlag,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cacheResponseForNoBuildDataFlags2 = cache.SatisfyRequest(
+               requestWithNoBuildDataFlags2,
+               new List<string>(),
+               new List<string>(new string[] { targetName }),
+               skippedResultsDoNotCauseCacheMiss: false);
+
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cacheResponseForRequestWithNoBuildDataFlags.Type);
+
+            // Because ProvideProjectStateAfterBuildFlag was provided as a part of BuildRequest
+            Assert.Equal(ResultsCacheResponseType.NotSatisfied, cachedResponseForProjectState.Type);
+
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cacheResponseForNoBuildDataFlags2.Type);
+        }
+
+        [Fact]
+        public void TestCacheOnDifferentBuildFlagsPerRequest_ProvideSubsetOfStateAfterBuild()
+        {
+            string targetName = "testTarget1";
+            int submissionId = 1;
+            int nodeRequestId = 0;
+            int configurationId = 1;
+
+            RequestedProjectState requestedProjectState1 = new()
+            {
+                PropertyFilters = ["property1", "property2"],
+            };
+            BuildRequest requestWithSubsetFlag1 = new BuildRequest(
+                submissionId,
+                nodeRequestId,
+                configurationId,
+                new string[1] { targetName } /* escapedTargets */,
+                null /* hostServices */,
+                BuildEventContext.Invalid /* parentBuildEventContext */,
+                null /* parentRequest */,
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild,
+                requestedProjectState1);
+
+            RequestedProjectState requestedProjectState2 = new()
+            {
+                PropertyFilters = ["property1"],
+            };
+            BuildRequest requestWithSubsetFlag2 = new BuildRequest(
+                submissionId,
+                nodeRequestId,
+                configurationId,
+                new string[1] { targetName } /* escapedTargets */,
+                null /* hostServices */,
+                BuildEventContext.Invalid /* parentBuildEventContext */,
+                null /* parentRequest */,
+                BuildRequestDataFlags.ProvideSubsetOfStateAfterBuild,
+                requestedProjectState2);
+
+            BuildResult resultForRequestWithSubsetFlag1 = new(requestWithSubsetFlag1);
+            resultForRequestWithSubsetFlag1.AddResultsForTarget(targetName, BuildResultUtilities.GetEmptySucceedingTargetResult());
+
+            using TextReader textReader = new StringReader(@"
+              <Project>
+                <PropertyGroup>
+                  <property1>Value1</property1>
+                  <property2>Value2</property2>
+                </PropertyGroup>
+              </Project>
+            ");
+            using XmlReader xmlReader = XmlReader.Create(textReader);
+            resultForRequestWithSubsetFlag1.ProjectStateAfterBuild = new ProjectInstance(ProjectRootElement.Create(xmlReader)).FilteredCopy(requestedProjectState1);
+
+            ResultsCache cache = new();
+            cache.AddResult(resultForRequestWithSubsetFlag1);
+
+            ResultsCacheResponse cachedResponseWithSubsetFlag1 = cache.SatisfyRequest(
+                requestWithSubsetFlag1,
+                new List<string>(),
+                new List<string>(new string[] { targetName }),
+                skippedResultsDoNotCauseCacheMiss: false);
+
+            ResultsCacheResponse cachedResponseWithSubsetFlag2 = cache.SatisfyRequest(
+                requestWithSubsetFlag2,
+                new List<string>(),
+                new List<string>(new string[] { targetName }),
+                skippedResultsDoNotCauseCacheMiss: false);
+
+            // We used the same filter that was used for the ProjectInstance in the cache -> cache hit.
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag1.Type);
+            Assert.Equal("Value1", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
+            Assert.Equal("Value2", cachedResponseWithSubsetFlag1.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
+
+            // We used a filter that's a subset of the one used for the ProjectInstance in the cache -> cache hit.
+            Assert.Equal(ResultsCacheResponseType.Satisfied, cachedResponseWithSubsetFlag2.Type);
+            Assert.Equal("Value1", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property1"));
+            Assert.Equal("", cachedResponseWithSubsetFlag2.Results.ProjectStateAfterBuild.GetPropertyValue("property2"));
+        }
+
         [Fact]
         public void TestClearResultsCache()
         {