Update AssignCulture Task to support existing Culture metadata

src/Shared/Constants.cs

@@ -203,6 +203,8 @@ internal static class ItemMetadataNames
         internal const string assemblyName = "AssemblyName";
         internal const string assemblyVersion = "AssemblyVersion";
         internal const string publicKeyToken = "PublicKeyToken";
+        internal const string culture = "Culture";
+        internal const string withCulture = "WithCulture";
 
         /// <summary>
         /// The output path for a given item.

src/Tasks.UnitTests/AssignCulture_Tests.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Text;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Tasks;
 using Microsoft.Build.Utilities;
@@ -14,11 +15,9 @@ namespace Microsoft.Build.UnitTests
 {
     public sealed class AssignCulture_Tests
     {
-        /*
-        * Method:   Basic
-        *
-        * Test the basic functionality.
-        */
+        /// <summary>
+        /// Tests the basic functionality.
+        /// </summary>
         [Fact]
         public void Basic()
         {
@@ -35,11 +34,9 @@ public void Basic()
             Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
 
-        /*
-        * Method:   CultureAttributePrecedence
-        *
-        * Any pre-existing Culture attribute on the item is to be ignored
-        */
+        /// <summary>
+        /// Any pre-existing Culture attribute on the item is to be ignored
+        /// </summary>
         [Fact]
         public void CultureAttributePrecedence()
         {
@@ -57,13 +54,11 @@ public void CultureAttributePrecedence()
             Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
 
-        /*
-        * Method:   CultureAttributePrecedenceWithBogusCulture
-        *
-        * This is really a corner case.
-        * If the incoming item has a 'Culture' attribute already, but that culture is invalid,
-        * we still overwrite that culture.
-        */
+        /// <summary>
+        /// This is really a corner case.
+        /// If the incoming item has a 'Culture' attribute already, but that culture is invalid,
+        /// we still overwrite that culture.
+        /// </summary>
         [Fact]
         public void CultureAttributePrecedenceWithBogusCulture()
         {
@@ -81,14 +76,10 @@ public void CultureAttributePrecedenceWithBogusCulture()
             Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
 
-
-
-        /*
-        * Method:   AttributeForwarding
-        *
-        * Make sure that attributes set on input items are forwarded to output items.
-        * This applies to every attribute except for the one pointed to by CultureAttribute.
-        */
+        /// <summary>
+        /// Make sure that attributes set on input items are forwarded to output items.
+        /// This applies to every attribute except for the one pointed to by CultureAttribute.
+        /// </summary>
         [Fact]
         public void AttributeForwarding()
         {
@@ -108,12 +99,10 @@ public void AttributeForwarding()
         }
 
 
-        /*
-        * Method:   NoCulture
-        *
-        * Test the case where an item has no embedded culture. For example,
-        * "MyResource.resx"
-        */
+        /// <summary>
+        /// Test the case where an item has no embedded culture. For example:
+        /// "MyResource.resx"
+        /// </summary>
         [Fact]
         public void NoCulture()
         {
@@ -130,11 +119,9 @@ public void NoCulture()
             Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
 
-        /*
-        * Method:   NoExtension
-        *
-        * Test the case where an item has no extension. For example "MyResource".
-        */
+        /// <summary>
+        /// Test the case where an item has no extension. For example "MyResource".
+        /// </summary>
         [Fact]
         public void NoExtension()
         {
@@ -151,12 +138,10 @@ public void NoExtension()
             Assert.Equal("MyResource", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
 
-        /*
-        * Method:   DoubleDot
-        *
-        * Test the case where an item has two dots embedded, but otherwise looks
-        * like a well-formed item. For example "MyResource..resx".
-        */
+        /// <summary>
+        ///  Test the case where an item has two dots embedded, but otherwise looks
+        /// like a well-formed item.For example "MyResource..resx".
+        /// </summary>
         [Fact]
         public void DoubleDot()
         {
@@ -194,12 +179,11 @@ public void Regress283991()
             Assert.Single(t.AssignedFilesWithNoCulture);
         }
 
-        /*
-        * Method:   PseudoLocalization
-        *
-        * Test the usage of Windows Pseudo-Locales
-        * https://docs.microsoft.com/en-gb/windows/desktop/Intl/pseudo-locales
-        */
+        /// <summary>
+        /// Test the usage of Windows Pseudo-Locales
+        /// https://docs.microsoft.com/en-gb/windows/desktop/Intl/pseudo-locales
+        /// </summary>
+        /// <param name="culture"></param>
         [Theory]
         [InlineData("qps-ploc")]
         [InlineData("qps-plocm")]
@@ -277,5 +261,47 @@ public void Pseudolocales_CaseInsensitive()
             Assert.Equal($"MyResource.{culture}.resx", t.AssignedFiles[0].ItemSpec);
             Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
         }
+
+        /// <summary>
+        /// Any pre-existing Culture attribute on the item is to be respected
+        /// </summary>
+        [Fact]
+        public void CultureMetaDataShouldBeRespected()
+        {
+            AssignCulture t = new AssignCulture();
+            t.BuildEngine = new MockEngine();
+            ITaskItem i = new TaskItem("MyResource.fr.resx");
+            i.SetMetadata("Culture", "en-GB");
+            t.Files = new ITaskItem[] { i };
+            t.RespectAlreadyAssignedItemCulture = true;
+            t.Execute();
+
+            Assert.Single(t.AssignedFiles);
+            Assert.Single(t.CultureNeutralAssignedFiles);
+            Assert.Equal("en-GB", t.AssignedFiles[0].GetMetadata("Culture"));
+            Assert.Equal("MyResource.fr.resx", t.AssignedFiles[0].ItemSpec);
+            Assert.Equal("MyResource.fr.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
+        }
+
+        /// <summary>
+        /// Any pre-existing Culture attribute on the item is not to be respected, because culture is not set
+        /// </summary>
+        [Fact]
+        public void CultureMetaDataShouldNotBeRespected()
+        {
+            AssignCulture t = new AssignCulture();
+            t.BuildEngine = new MockEngine();
+            ITaskItem i = new TaskItem("MyResource.fr.resx");
+            i.SetMetadata("Culture", "");
+            t.Files = new ITaskItem[] { i };
+            t.RespectAlreadyAssignedItemCulture = true;
+            t.Execute();
+
+            Assert.Single(t.AssignedFiles);
+            Assert.Single(t.CultureNeutralAssignedFiles);
+            Assert.Equal("fr", t.AssignedFiles[0].GetMetadata("Culture"));
+            Assert.Equal("MyResource.fr.resx", t.AssignedFiles[0].ItemSpec);
+            Assert.Equal("MyResource.resx", t.CultureNeutralAssignedFiles[0].ItemSpec);
+        }
     }
 }

src/Tasks/AssignCulture.cs

@@ -41,6 +41,12 @@ public class AssignCulture : TaskExtension
         [Required]
         public ITaskItem[] Files { get; set; } = Array.Empty<ITaskItem>();
 
+        /// <summary>
+        /// If the flag set to 'true' the incoming list with existing Culture metadata will not be ammended and CultureNeutralAssignedFiles filename will be equal to the original.
+        /// In case the Culture metadata was not provided, the logic of RespectAlreadyAssignedItemCulture will not take any effect.
+        /// </summary>
+        public bool RespectAlreadyAssignedItemCulture { get; set; } = false;
+
         /// <summary>
         /// This outgoing list of files is exactly the same as the incoming Files
         /// list except that an attribute name "Culture" will have been added if
@@ -134,32 +140,44 @@ public override bool Execute()
                     AssignedFiles[i] = new TaskItem(Files[i]);
 
                     string dependentUpon = AssignedFiles[i].GetMetadata(ItemMetadataNames.dependentUpon);
-                    Culture.ItemCultureInfo info = Culture.GetItemCultureInfo(
-                            AssignedFiles[i].ItemSpec,
-                            dependentUpon,
-                            // If 'WithCulture' is explicitly set to false, treat as 'culture-neutral' and keep the original name of the resource.
-                            // https://github.com/dotnet/msbuild/issues/3064
-                            ConversionUtilities.ValidBooleanFalse(AssignedFiles[i].GetMetadata("WithCulture")));
-
-                    if (!string.IsNullOrEmpty(info.culture))
+                    string existingCulture = AssignedFiles[i].GetMetadata(ItemMetadataNames.culture);
+
+                    if (RespectAlreadyAssignedItemCulture && !string.IsNullOrEmpty(existingCulture))
                     {
-                        AssignedFiles[i].SetMetadata("Culture", info.culture);
-                        AssignedFiles[i].SetMetadata("WithCulture", "true");
+                        AssignedFiles[i].SetMetadata(ItemMetadataNames.withCulture, "true");
                         cultureList.Add(AssignedFiles[i]);
+
+                        CultureNeutralAssignedFiles[i] = new TaskItem(AssignedFiles[i]);
                     }
                     else
                     {
-                        noCultureList.Add(AssignedFiles[i]);
-                        AssignedFiles[i].SetMetadata("WithCulture", "false");
-                    }
-
-                    CultureNeutralAssignedFiles[i] =
+                        Culture.ItemCultureInfo info = Culture.GetItemCultureInfo(
+                            AssignedFiles[i].ItemSpec,
+                            dependentUpon,
+                            // If 'WithCulture' is explicitly set to false, treat as 'culture-neutral' and keep the original name of the resource.
+                            // https://github.com/dotnet/msbuild/issues/3064
+                            ConversionUtilities.ValidBooleanFalse(AssignedFiles[i].GetMetadata(ItemMetadataNames.withCulture)));
+
+                        if (!string.IsNullOrEmpty(info.culture))
+                        {
+                            AssignedFiles[i].SetMetadata(ItemMetadataNames.culture, info.culture);
+                            AssignedFiles[i].SetMetadata(ItemMetadataNames.withCulture, "true");
+                            cultureList.Add(AssignedFiles[i]);
+                        }
+                        else
+                        {
+                            noCultureList.Add(AssignedFiles[i]);
+                            AssignedFiles[i].SetMetadata(ItemMetadataNames.withCulture, "false");
+                        }
+
+                        CultureNeutralAssignedFiles[i] =
                         new TaskItem(AssignedFiles[i]) { ItemSpec = info.cultureNeutralFilename };
+                    }
 
                     Log.LogMessageFromResources(
                         MessageImportance.Low,
                         "AssignCulture.Comment",
-                        AssignedFiles[i].GetMetadata("Culture"),
+                        AssignedFiles[i].GetMetadata(ItemMetadataNames.culture),
                         AssignedFiles[i].ItemSpec);
                 }
                 catch (ArgumentException e)

src/Tasks/Microsoft.Common.CurrentVersion.targets

@@ -3244,6 +3244,10 @@ Copyright (C) Microsoft Corporation. All rights reserved.
       Name="SplitResourcesByCulture"
       DependsOnTargets="AssignTargetPaths">
 
+    <PropertyGroup>
+      <RespectAlreadyAssignedItemCulture Condition="'$(RespectAlreadyAssignedItemCulture)' == ''">false</RespectAlreadyAssignedItemCulture>
+    </PropertyGroup>
+
     <Warning Condition="'@(ResxWithNoCulture)'!=''" Code="MSB9000" Text="ResxWithNoCulture item type is deprecated. Use EmbeddedResource items instead."/>
     <Warning Condition="'@(ResxWithCulture)'!=''" Code="MSB9001" Text="ResxWithCulture item type is deprecated. Use EmbeddedResource items instead."/>
     <Warning Condition="'@(NonResxWithCulture)'!=''" Code="MSB9002" Text="NonResxWithCulture item type is deprecated. Use EmbeddedResource items instead."/>
@@ -3261,7 +3265,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
       </EmbeddedResource>
     </ItemGroup>
 
-    <AssignCulture Files="@(EmbeddedResource)" Condition="'%(Extension)'!='.licx'">
+    <AssignCulture Files="@(EmbeddedResource)" Condition="'%(Extension)'!='.licx'"  RespectAlreadyAssignedItemCulture="$(RespectAlreadyAssignedItemCulture)">
       <!-- Create the list of culture resx and embedded resource files -->
       <Output TaskParameter="AssignedFilesWithCulture" ItemName="_MixedResourceWithCulture"/>
       <!-- Create the list of non-culture resx and embedded resource files -->