MatchOnMetadata PR feedback

dotnet · Apr 17, 2020 · ea92b7f · ea92b7f
1 parent f1c5d97
commit ea92b7f
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 36 deletions.
diff --git a/ref/Microsoft.Build/net/Microsoft.Build.cs b/ref/Microsoft.Build/net/Microsoft.Build.cs
@@ -508,6 +508,10 @@ public GlobResult(Microsoft.Build.Construction.ProjectItemElement itemElement, S
         public Microsoft.Build.Globbing.IMSBuildGlob MsBuildGlob { get { throw null; } set { } }
         public System.Collections.Generic.IEnumerable<string> Removes { get { throw null; } set { } }
     }
+    public static partial class MatchOnMetadataConstants
+    {
+        public const Microsoft.Build.Evaluation.MatchOnMetadataOptions MatchOnMetadataOptionsDefaultValue = Microsoft.Build.Evaluation.MatchOnMetadataOptions.CaseSensitive;
+    }
     public enum MatchOnMetadataOptions
     {
         CaseInsensitive = 1,

diff --git a/ref/Microsoft.Build/netstandard/Microsoft.Build.cs b/ref/Microsoft.Build/netstandard/Microsoft.Build.cs
@@ -508,6 +508,10 @@ public GlobResult(Microsoft.Build.Construction.ProjectItemElement itemElement, S
         public Microsoft.Build.Globbing.IMSBuildGlob MsBuildGlob { get { throw null; } set { } }
         public System.Collections.Generic.IEnumerable<string> Removes { get { throw null; } set { } }
     }
+    public static partial class MatchOnMetadataConstants
+    {
+        public const Microsoft.Build.Evaluation.MatchOnMetadataOptions MatchOnMetadataOptionsDefaultValue = Microsoft.Build.Evaluation.MatchOnMetadataOptions.CaseSensitive;
+    }
     public enum MatchOnMetadataOptions
     {
         CaseInsensitive = 1,

diff --git a/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs b/src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/ItemGroupIntrinsicTask.cs
@@ -84,7 +84,7 @@ internal override void ExecuteTask(Lookup lookup)
                             HashSet<string> keepMetadata = null;
                             HashSet<string> removeMetadata = null;
                             HashSet<string> matchOnMetadata = null;
-                            MatchOnMetadataOptions matchOnMetadataOptions = MatchOnMetadataOptions.CaseSensitive;
+                            MatchOnMetadataOptions matchOnMetadataOptions = MatchOnMetadataConstants.MatchOnMetadataOptionsDefaultValue;
 
                             if (!String.IsNullOrEmpty(child.KeepMetadata))
                             {

diff --git a/src/Build/Evaluation/ItemSpec.cs b/src/Build/Evaluation/ItemSpec.cs
@@ -89,36 +89,20 @@ public override bool IsMatch(string itemToMatch)
 
             public override bool IsMatchOnMetadata(IItem item, IEnumerable<string> metadata, MatchOnMetadataOptions options)
             {
-                switch (options)
-                {
-                    case MatchOnMetadataOptions.CaseSensitive:
-                        return IsCaseSensitiveMetadataMatch(item, metadata, ReferencedItems);
-                    case MatchOnMetadataOptions.CaseInsensitive:
-                        return IsCaseInsensitiveMetadataMatch(item, metadata, ReferencedItems);
-                    case MatchOnMetadataOptions.PathLike:
-                        return IsPathLikeMetadataMatch(item, metadata, ReferencedItems, ProjectDirectory);
-                    default:
-                        ErrorUtilities.ThrowInternalErrorUnreachable();
-                        return false;
-                }
-            }
-
-            private static bool IsCaseSensitiveMetadataMatch(IItem item, IEnumerable<string> metadata, List<ReferencedItem> referencedItems)
-            {
-                return referencedItems.Any(referencedItem =>
-                            metadata.All(m => item.GetMetadataValue(m).Equals(referencedItem.Item.GetMetadataValue(m)) && !item.GetMetadataValue(m).Equals(string.Empty)));
-            }
-
-            private static bool IsCaseInsensitiveMetadataMatch(IItem item, IEnumerable<string> metadata, List<ReferencedItem> referencedItems)
-            {
-                return referencedItems.Any(referencedItem =>
-                    metadata.All(m => String.Equals(item.GetMetadataValue(m), referencedItem.Item.GetMetadataValue(m), StringComparison.OrdinalIgnoreCase) && !item.GetMetadataValue(m).Equals(string.Empty)));
+                return ReferencedItems.Any(referencedItem =>
+                        metadata.All(m => !item.GetMetadataValue(m).Equals(string.Empty) && MetadataComparer(options, item.GetMetadataValue(m), referencedItem.Item.GetMetadataValue(m))));
             }
 
-            private static bool IsPathLikeMetadataMatch(IItem item, IEnumerable<string> metadata, List<ReferencedItem> referencedItems, string projectDirectory)
+            private bool MetadataComparer(MatchOnMetadataOptions options, string itemMetadata, string referencedItemMetadata)
             {
-                return referencedItems.Any(referencedItem =>
-                    metadata.All(m => !item.GetMetadataValue(m).Equals(string.Empty) && FileUtilities.ComparePathsNoThrow(item.GetMetadataValue(m), referencedItem.Item.GetMetadataValue(m), projectDirectory)));
+                if (options.Equals(MatchOnMetadataOptions.PathLike))
+                {
+                    return FileUtilities.ComparePathsNoThrow(itemMetadata, referencedItemMetadata, ProjectDirectory);
+                }
+                else 
+                {
+                    return String.Equals(itemMetadata, referencedItemMetadata, options.Equals(MatchOnMetadataOptions.CaseInsensitive) ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal);
+                }
             }
 
             public override IMSBuildGlob ToMSBuildGlob()

diff --git a/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs b/src/Build/Evaluation/LazyItemEvaluator.RemoveOperation.cs
@@ -89,4 +89,8 @@ public enum MatchOnMetadataOptions
         CaseInsensitive,
         PathLike
     }
+
+    public static class MatchOnMetadataConstants {
+        public const MatchOnMetadataOptions MatchOnMetadataOptionsDefaultValue = MatchOnMetadataOptions.CaseSensitive;
+    }
 }
diff --git a/src/Shared/UnitTests/FileUtilities_Tests.cs b/src/Shared/UnitTests/FileUtilities_Tests.cs
@@ -287,14 +287,7 @@ public void HasExtension_UsesOrdinalIgnoreCase()
 
                 var result = FileUtilities.HasExtension("foo.ini", new string[] { ".INI" });
 
-                if (FileUtilities.GetIsFileSystemCaseSensitive())
-                {
-                    result.ShouldBeFalse();
-                }
-                else
-                {
-                    result.ShouldBeTrue();
-                }
+                result.ShouldBe(!FileUtilities.GetIsFileSystemCaseSensitive());
             }
             finally
             {