Optimization neglected to include condition (#5927)

Optimization for RemoveOperation neglected to consider condition. Fixes that and adds a test. Fixes #5766 Introduced by #5350
dotnet · Dec 3, 2020 · cd29721 · cd29721
1 parent 27cef46
commit cd29721
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 1 deletion.
diff --git a/src/Build.OM.UnitTests/Definition/ProjectItem_Tests.cs b/src/Build.OM.UnitTests/Definition/ProjectItem_Tests.cs
@@ -2774,6 +2774,17 @@ public void UpdateShouldImportMetadataFromReferencedItem()
             ObjectModelHelpers.AssertItemHasMetadata(expectedMetadataC, items[4]);
         }
 
+        [Fact]
+        public void OptimizedRemoveOperationRespectsCondition()
+        {
+            string content = @"<TheItem Include=""InitialValue"" />
+                               <TheItem Remove=""@(TheItem)"" Condition=""false"" />
+                               <TheItem Include=""ReplacedValue"" Condition=""false"" /> ";
+            IList<ProjectItem> items = ObjectModelHelpers.GetItemsFromFragment(content, true);
+
+            items[0].EvaluatedInclude.ShouldBe("InitialValue");
+        }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]

diff --git a/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs
@@ -38,7 +38,7 @@ protected override void ApplyImpl(ImmutableList<ItemData>.Builder listBuilder, I
                     new BuildEventFileInfo(string.Empty),
                     "OM_MatchOnMetadataIsRestrictedToOnlyOneReferencedItem");
 
-                if (_matchOnMetadata.IsEmpty && ItemspecContainsASingleBareItemReference(_itemSpec, _itemElement.ItemType))
+                if (_matchOnMetadata.IsEmpty && ItemspecContainsASingleBareItemReference(_itemSpec, _itemElement.ItemType) && _conditionResult)
                 {
                     // Perf optimization: If the Remove operation references itself (e.g. <I Remove="@(I)"/>)
                     // then all items are removed and matching is not necessary