Adds validation to throw MSB4259 when property references contain leading or trailing whitespace outside of conditions. (#13076)

Author: huulinhnguyen-dev

documentation/wiki/ChangeWaves.md

@@ -24,6 +24,9 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
 
 ## Current Rotation of Change Waves
 
+### 18.5
+- [Throw MSB4281 error for property references and property function calls with leading or trailing whitespace (e.g., `$( Foo )`, `$( Foo.StartsWith('Bar') )`).](https://github.com/dotnet/msbuild/pull/13076)
+
 ### 18.4
 - [Start throwing on null or empty paths in MultiProcess and MultiThreaded Task Environment Drivers.](https://github.com/dotnet/msbuild/pull/12914)
 

src/Build.UnitTests/Evaluation/Expander_Tests.cs

@@ -3854,14 +3854,71 @@ public void PropertyFunctionStaticMethodIntrinsicBitOperations()
         [Fact]
         public void PropertySimpleSpaced()
         {
-            PropertyDictionary<ProjectPropertyInstance> pg = new PropertyDictionary<ProjectPropertyInstance>();
-            pg.Set(ProjectPropertyInstance.Create("SomeStuff", "This IS SOME STUff"));
+            using (TestEnvironment env = TestEnvironment.Create())
+            {
+                env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_4.ToString());
 
-            Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(pg, FileSystems.Default);
+                PropertyDictionary<ProjectPropertyInstance> pg = new PropertyDictionary<ProjectPropertyInstance>();
+                pg.Set(ProjectPropertyInstance.Create("SomeStuff", "This IS SOME STUff"));
 
-            string result = expander.ExpandIntoStringLeaveEscaped(@"$( SomeStuff )", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);
+                Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(pg, FileSystems.Default);
 
-            Assert.Equal(String.Empty, result);
+                string result = expander.ExpandIntoStringLeaveEscaped(@"$( SomeStuff )", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);
+
+                Assert.Equal(String.Empty, result);
+            }
+        }
+
+        /// <summary>
+        /// Expand a property reference that has whitespace around the property name should throw error MSB4281
+        /// when ChangeWave 18.5 is enabled
+        /// </summary>
+        [Theory]
+        [InlineData("$( SomeStuff )")]   // Leading and trailing space
+        [InlineData("$( SomeStuff)")]    // Leading space only
+        [InlineData("$(SomeStuff )")]    // Trailing space only
+        public void PropertyWithWhitespace_ShouldThrowError_WhenChangeWaveEnabled(string expression)
+        {
+            using (TestEnvironment env = TestEnvironment.Create())
+            {
+                env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", "");
+
+                PropertyDictionary<ProjectPropertyInstance> pg = new PropertyDictionary<ProjectPropertyInstance>();
+                pg.Set(ProjectPropertyInstance.Create("SomeStuff", "This IS SOME STUff"));
+
+                Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(pg, FileSystems.Default);
+
+                InvalidProjectFileException ex = Assert.Throws<InvalidProjectFileException>(
+                    () => expander.ExpandIntoStringLeaveEscaped(expression, ExpanderOptions.ExpandProperties, MockElementLocation.Instance));
+
+                Assert.Equal("MSB4281", ex.ErrorCode);
+            }
+        }
+
+        /// <summary>
+        /// Expand a property function call that has whitespace around the property name should throw error MSB4281
+        /// when ChangeWave 18.5 is enabled
+        /// </summary>
+        [Theory]
+        [InlineData("$( SomeStuff.StartsWith('This'))")]   // Leading space only
+        [InlineData("$(SomeStuff.StartsWith('This') )")]   // Trailing space only
+        [InlineData("$( SomeStuff.StartsWith('This') )")]  // Leading and trailing space
+        public void PropertyFunctionWithWhitespace_ShouldThrowError_WhenChangeWaveEnabled(string expression)
+        {
+            using (TestEnvironment env = TestEnvironment.Create())
+            {
+                env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", "");
+
+                PropertyDictionary<ProjectPropertyInstance> pg = new PropertyDictionary<ProjectPropertyInstance>();
+                pg.Set(ProjectPropertyInstance.Create("SomeStuff", "This IS SOME STUff"));
+
+                Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(pg, FileSystems.Default);
+
+                InvalidProjectFileException ex = Assert.Throws<InvalidProjectFileException>(
+                    () => expander.ExpandIntoStringLeaveEscaped(expression, ExpanderOptions.ExpandProperties, MockElementLocation.Instance));
+
+                Assert.Equal("MSB4281", ex.ErrorCode);
+            }
         }
 
         [WindowsOnlyFact]

src/Build/Evaluation/Expander.cs

@@ -1385,7 +1385,30 @@ internal static object ExpandPropertiesLeaveTypedAndEscaped(
                         }
                         else // This is a regular property
                         {
-                            propertyValue = LookupProperty(properties, expression, propertyStartIndex + 2, propertyEndIndex - 1, elementLocation, propertiesUseTracker);
+                            int propertyNameStart = propertyStartIndex + 2;
+                            int propertyNameEnd = propertyEndIndex - 1;
+
+                            // Check for whitespace in property name - this is likely a typo
+                            // Gated behind ChangeWave 18.5 as this is a breaking change
+                            if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5))
+                            {
+                                // Check if there's leading or trailing whitespace
+                                if (Char.IsWhiteSpace(expression[propertyNameStart]) || Char.IsWhiteSpace(expression[propertyNameEnd]))
+                                {
+                                    // Find the position of the whitespace for error message
+                                    int whitespacePosition = Char.IsWhiteSpace(expression[propertyNameStart])
+                                        ? propertyNameStart
+                                        : propertyNameEnd;
+
+                                    ProjectErrorUtilities.ThrowInvalidProject(
+                                        elementLocation,
+                                        "IllFormedPropertySpaceInPropertyReference",
+                                        expression.Substring(propertyStartIndex, propertyEndIndex - propertyStartIndex + 1),
+                                        whitespacePosition - propertyStartIndex + 1);
+                                }
+                            }
+
+                            propertyValue = LookupProperty(properties, expression, propertyNameStart, propertyNameEnd, elementLocation, propertiesUseTracker);
                         }
 
                         if (propertyValue != null)
@@ -1434,7 +1457,27 @@ internal static object ExpandPropertyBody(
                 Function<T> function = null;
                 string propertyName = propertyBody;
 
-                // Trim the body for compatibility reasons:
+                // Check for whitespace in property body - this is likely a typo
+                // Gated behind ChangeWave 18.5 as this is a breaking change
+                if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave18_5))
+                {
+                    if (Char.IsWhiteSpace(propertyBody[0]) || Char.IsWhiteSpace(propertyBody[propertyBody.Length - 1]))
+                    {
+                        // Calculate the position of the whitespace for error message
+                        // Position is 1-based, relative to the full property reference $({propertyBody})
+                        int whitespacePosition = Char.IsWhiteSpace(propertyBody[0])
+                            ? 3  // Position after "$("
+                            : propertyBody.Length + 2;  // Position before ")"
+
+                        ProjectErrorUtilities.ThrowInvalidProject(
+                            elementLocation,
+                            "IllFormedPropertySpaceInPropertyReference",
+                            $"$({propertyBody})",
+                            whitespacePosition);
+                    }
+                }
+
+                // Trim the body for compatibility reasons (when ChangeWave is disabled):
                 // Spaces are not valid property name chars, but $( Foo ) is allowed, and should always expand to BLANK.
                 // Do a very fast check for leading and trailing whitespace, and trim them from the property body if we have any.
                 // But we will do a property name lookup on the propertyName that we held onto.

src/Build/Resources/Strings.resx

@@ -529,6 +529,10 @@
     <value>MSB4259: Unexpected space at position "{1}" of condition "{0}". Did you forget to remove a space?</value>
     <comment>{StrBegin="MSB4259: "}</comment>
   </data>
+  <data name="IllFormedPropertySpaceInPropertyReference" xml:space="preserve">
+    <value>MSB4281: Unexpected space at position "{1}" of property reference "{0}". Did you forget to remove a space?</value>
+    <comment>{StrBegin="MSB4281: "}</comment>
+  </data>
   <data name="IllFormedQuotedStringInCondition" xml:space="preserve">
     <value>MSB4101: Expected a closing quote after position {1} in condition "{0}".</value>
     <comment>{StrBegin="MSB4101: "}</comment>
@@ -2462,7 +2466,7 @@ Utilization:          {0} Average Utilization: {1:###.0}</value>
   <!--
         The Build message bucket is: MSB4000 - MSB4999
 
-        Next message code should be MSB4281
+        Next message code should be MSB4282
 
         Don't forget to update this comment after using a new code.
   -->