Log warnings for skipped STR resource keys instead of failing the build

src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs

@@ -1665,6 +1665,227 @@ public void STRWithResourcesNamespaceAndSTRNamespaceVB()
         {
             Utilities.STRNamespaceTestHelper("VB", "MyResourcesNamespace", "MySTClassNamespace", _output);
         }
+
+        /// <summary>
+        /// When a resource key matches a reserved name,
+        /// the STR property is skipped and a warning MSB3827 is logged.
+        /// The task should still succeed.
+        /// </summary>
+        [Fact]
+        public void StronglyTypedResources_ReservedName_LogsWarning()
+        {
+            GenerateResource t = Utilities.CreateTask(_output);
+            try
+            {
+                // Create a resx with a "ResourceManager" and "Culture" keys, which collides with the
+                // reserved property name added by StronglyTypedResourceBuilder.
+                string resxFile = Utilities.WriteTestResX(false, null,
+                    "  <data name=\"ResourceManager\">\r\n" +
+                    "    <value>ShouldBeSkipped</value>\r\n" +
+                    "  </data>\r\n" +
+                    "  <data name=\"Culture\">\r\n" +
+                    "    <value>ShouldBeSkipped</value>\r\n" +
+                    "  </data>\r\n");
+
+                t.Sources = new ITaskItem[] { new TaskItem(resxFile) };
+                t.StronglyTypedLanguage = "CSharp";
+                t.StateFile = new TaskItem(Utilities.GetTempFileName(".cache"));
+
+                Utilities.ExecuteTask(t);
+
+                // The file should still be generated — only the reserved-name properties are skipped.
+                File.Exists(t.StronglyTypedFileName).ShouldBeTrue();
+                string generatedCode = File.ReadAllText(t.StronglyTypedFileName);
+
+                // "MyString" (the standard test resource) should still appear.
+                generatedCode.ShouldContain("MyString");
+
+                // The skipped resources should NOT have a GetString accessor in the generated code.
+                generatedCode.ShouldNotContain("GetString(\"ResourceManager\"");
+                generatedCode.ShouldNotContain("GetString(\"Culture\"");
+
+                // Warning about the reserved name should be logged.
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedReservedName", "ResourceManager", "ResourceManager, Culture");
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedReservedName", "Culture", "ResourceManager, Culture");
+
+                // Exactly 2 warnings (one per reserved name), no errors.
+                ((MockEngine)t.BuildEngine).Warnings.ShouldBe(2);
+                ((MockEngine)t.BuildEngine).Errors.ShouldBe(0);
+            }
+            finally
+            {
+                File.Delete(t.Sources[0].ItemSpec);
+                if (t.StronglyTypedFileName != null)
+                {
+                    FileUtilities.DeleteNoThrow(t.StronglyTypedFileName);
+                }
+
+                foreach (ITaskItem item in t.FilesWritten)
+                {
+                    FileUtilities.DeleteNoThrow(item.ItemSpec);
+                }
+            }
+        }
+
+        /// <summary>
+        /// When a resource key cannot be converted to a valid identifier (e.g. "=", "`"),
+        /// the STR property is skipped and a warning MSB3828 is logged.
+        /// The "=" characters are not in the s_charsToReplace list, so they survive
+        /// character replacement, and cannot become a valid C# identifier even after
+        /// CreateValidIdentifier and underscore-prepend attempts.
+        /// </summary>
+        [Fact]
+        public void StronglyTypedResources_InvalidIdentifier_LogsWarning()
+        {
+            GenerateResource t = Utilities.CreateTask(_output);
+            try
+            {
+                string resxFile = Utilities.WriteTestResX(false, null,
+                    "  <data name=\"=\">\r\n" +
+                    "    <value>EqualSign</value>\r\n" +
+                    "  </data>\r\n" +
+                    "  <data name=\"`\">\r\n" +
+                    "    <value>Backtick</value>\r\n" +
+                    "  </data>\r\n");
+
+                t.Sources = new ITaskItem[] { new TaskItem(resxFile) };
+                t.StronglyTypedLanguage = "CSharp";
+                t.StateFile = new TaskItem(Utilities.GetTempFileName(".cache"));
+
+                Utilities.ExecuteTask(t);
+
+                File.Exists(t.StronglyTypedFileName).ShouldBeTrue();
+                string generatedCode = File.ReadAllText(t.StronglyTypedFileName);
+                generatedCode.ShouldContain("MyString");
+
+                // The skipped resource should NOT have a GetString accessor in the generated code.
+                generatedCode.ShouldNotContain("GetString(\"=\"");
+                generatedCode.ShouldNotContain("GetString(\"`\"");
+
+                // Warning about invalid identifier should be logged.
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedInvalidIdentifier", "=");
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedInvalidIdentifier", "`");
+
+                // Exactly 2 warnings, no errors.
+                ((MockEngine)t.BuildEngine).Warnings.ShouldBe(2);
+                ((MockEngine)t.BuildEngine).Errors.ShouldBe(0);
+            }
+            finally
+            {
+                File.Delete(t.Sources[0].ItemSpec);
+                if (t.StronglyTypedFileName != null)
+                {
+                    FileUtilities.DeleteNoThrow(t.StronglyTypedFileName);
+                }
+
+                foreach (ITaskItem item in t.FilesWritten)
+                {
+                    FileUtilities.DeleteNoThrow(item.ItemSpec);
+                }
+            }
+        }
+
+        /// <summary>
+        /// When two resource keys collide after identifier normalization (e.g. "foo-bar" and "foo_bar"),
+        /// both are skipped and a warning MSB3829 is logged for each, mentioning the other colliding key.
+        /// </summary>
+        [Fact]
+        public void StronglyTypedResources_NameCollision_LogsWarning()
+        {
+            GenerateResource t = Utilities.CreateTask(_output);
+            try
+            {
+                // "foo-bar" normalizes to "foo_bar", which collides with "foo_bar".
+                string resxFile = Utilities.WriteTestResX(false, null,
+                    "  <data name=\"foo-bar\">\r\n" +
+                    "    <value>Value1</value>\r\n" +
+                    "  </data>\r\n" +
+                    "  <data name=\"foo_bar\">\r\n" +
+                    "    <value>Value2</value>\r\n" +
+                    "  </data>\r\n");
+
+                t.Sources = new ITaskItem[] { new TaskItem(resxFile) };
+                t.StronglyTypedLanguage = "CSharp";
+                t.StateFile = new TaskItem(Utilities.GetTempFileName(".cache"));
+
+                Utilities.ExecuteTask(t);
+
+                File.Exists(t.StronglyTypedFileName).ShouldBeTrue();
+                string generatedCode = File.ReadAllText(t.StronglyTypedFileName);
+
+                // "MyString" should still be generated.
+                generatedCode.ShouldContain("MyString");
+
+                // Neither colliding key should produce a property.
+                generatedCode.ShouldNotContain("foo_bar");
+
+                // Both colliding keys should have a warning referencing the other.
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedNameCollision", "foo-bar", "foo_bar");
+                Utilities.AssertLogContainsResource(t, "GenerateResource.STRPropertySkippedNameCollision", "foo_bar", "foo-bar");
+
+                // Exactly 2 warnings (one per colliding key), no errors.
+                ((MockEngine)t.BuildEngine).Warnings.ShouldBe(2);
+                ((MockEngine)t.BuildEngine).Errors.ShouldBe(0);
+            }
+            finally
+            {
+                File.Delete(t.Sources[0].ItemSpec);
+                if (t.StronglyTypedFileName != null)
+                {
+                    FileUtilities.DeleteNoThrow(t.StronglyTypedFileName);
+                }
+
+                foreach (ITaskItem item in t.FilesWritten)
+                {
+                    FileUtilities.DeleteNoThrow(item.ItemSpec);
+                }
+            }
+        }
+
+        /// <summary>
+        /// When all resources are valid, no skip warnings are logged —
+        /// verifies that warnings are only emitted when needed.
+        /// </summary>
+        [Fact]
+        public void StronglyTypedResources_NoProblematicKeys_NoSkipWarnings()
+        {
+            GenerateResource t = Utilities.CreateTask(_output);
+            try
+            {
+                string textFile = Utilities.WriteTestText(null, null);
+                t.Sources = new ITaskItem[] { new TaskItem(textFile) };
+                t.StronglyTypedLanguage = "CSharp";
+                t.StateFile = new TaskItem(Utilities.GetTempFileName(".cache"));
+
+                Utilities.ExecuteTask(t);
+
+                File.Exists(t.StronglyTypedFileName).ShouldBeTrue();
+
+                // None of the skip warnings should appear.
+                string log = ((MockEngine)t.BuildEngine).Log;
+                log.ShouldNotContain("MSB3827");
+                log.ShouldNotContain("MSB3828");
+                log.ShouldNotContain("MSB3829");
+                log.ShouldNotContain("MSB3830");
+
+                // No warnings or errors at all.
+                ((MockEngine)t.BuildEngine).Warnings.ShouldBe(0);
+                ((MockEngine)t.BuildEngine).Errors.ShouldBe(0);
+            }
+            finally
+            {
+                File.Delete(t.Sources[0].ItemSpec);
+                if (t.StronglyTypedFileName != null)
+                {
+                    FileUtilities.DeleteNoThrow(t.StronglyTypedFileName);
+                }
+
+                foreach (ITaskItem item in t.FilesWritten)
+                {
+                    FileUtilities.DeleteNoThrow(item.ItemSpec);
+                }
+            }
+        }
     }
 
     public sealed class TransformationErrors

src/Tasks/GenerateResource.cs

@@ -3443,7 +3443,7 @@ private void CreateStronglyTypedResources(ReaderInfo reader, String outFile, Str
                 _logger.LogMessageFromResources("GenerateResource.CreatingSTR", _stronglyTypedFilename);
 
                 // Generate the STR class
-                String[] errors;
+                StronglyTypedResourceBuilder.SkippedResource[] skippedResources;
                 bool generateInternalClass = !_stronglyTypedClassIsPublic;
                 // StronglyTypedResourcesNamespace can be null and this is ok.
                 // If it is null then the default namespace (=stronglyTypedNamespace) is used.
@@ -3454,28 +3454,31 @@ private void CreateStronglyTypedResources(ReaderInfo reader, String outFile, Str
                         _stronglyTypedResourcesNamespace,
                         provider,
                         generateInternalClass,
-                        out errors);
+                        out skippedResources);
 
                 CodeGeneratorOptions codeGenOptions = new CodeGeneratorOptions();
                 using (TextWriter output = new StreamWriter(_stronglyTypedFilename))
                 {
                     provider.GenerateCodeFromCompileUnit(ccu, output, codeGenOptions);
                 }
 
-                if (errors.Length > 0)
+                // The STR class file was generated (possibly with some properties skipped).
+                // Log warnings for any skipped resources and mark the file as successfully created.
+                foreach (StronglyTypedResourceBuilder.SkippedResource skipped in skippedResources)
                 {
-                    _logger.LogErrorWithCodeFromResources("GenerateResource.ErrorFromCodeDom", inputFileName);
-                    foreach (String error in errors)
+                    string messageKey = skipped.Reason switch
                     {
-                        _logger.LogErrorWithCodeFromResources("GenerateResource.CodeDomError", error);
-                    }
-                }
-                else
-                {
-                    // No errors, and no exceptions - we presumably did create the STR class file
-                    // and it should get added to FilesWritten. So set a flag to indicate this.
-                    _stronglyTypedResourceSuccessfullyCreated = true;
+                        StronglyTypedResourceBuilder.SkipReason.CodeDomError => "GenerateResource.CodeDomError",
+                        StronglyTypedResourceBuilder.SkipReason.ReservedName => "GenerateResource.STRPropertySkippedReservedName",
+                        StronglyTypedResourceBuilder.SkipReason.VoidType => "GenerateResource.STRPropertySkippedVoidType",
+                        StronglyTypedResourceBuilder.SkipReason.InvalidIdentifier => "GenerateResource.STRPropertySkippedInvalidIdentifier",
+                        StronglyTypedResourceBuilder.SkipReason.NameCollision => "GenerateResource.STRPropertySkippedNameCollision",
+                        _ => throw new InvalidOperationException($"Unknown SkipReason {skipped.Reason}"),
+                    };
+                    _logger.LogWarningWithCodeFromResources(null, Path.GetFullPath(inputFileName), 0, 0, 0, 0, messageKey, [skipped.Key, .. skipped.AdditionalMessageArgs]);
                 }
+
+                _stronglyTypedResourceSuccessfullyCreated = true;
             }
             finally
             {

src/Tasks/Resources/Strings.resx

@@ -1179,6 +1179,23 @@
     <comment>{StrBegin="MSB3826: "}</comment>
   </data>
 
+  <data name="GenerateResource.STRPropertySkippedReservedName">
+    <value>MSB3827: Skipping strongly typed resource property generation for resource "{0}": the name conflicts with a reserved name. Reserved names: {1}.</value>
+    <comment>{StrBegin="MSB3827: "}</comment>
+  </data>
+  <data name="GenerateResource.STRPropertySkippedInvalidIdentifier">
+    <value>MSB3828: Skipping strongly typed resource property generation for resource "{0}": the name cannot be converted to a valid identifier.</value>
+    <comment>{StrBegin="MSB3828: "}</comment>
+  </data>
+  <data name="GenerateResource.STRPropertySkippedNameCollision">
+    <value>MSB3829: Skipping strongly typed resource property generation for resource "{0}": a name collision exists with resource "{1}" after identifier normalization.</value>
+    <comment>{StrBegin="MSB3829: "}</comment>
+  </data>
+  <data name="GenerateResource.STRPropertySkippedVoidType">
+    <value>MSB3830: Skipping strongly typed resource property generation for resource "{0}": the resource value type is void, which cannot be used as a property type.</value>
+    <comment>{StrBegin="MSB3830: "}</comment>
+  </data>
+
   <!--
         The GetAssemblyIdentity message bucket is: MSB3441 - MSB3450