Fix NullReferenceException when expanding property functions that ret…

…urn null (#6414) Fixes #6413 ### Context This is a regression introduced in #6128. MSBuild crashes when evaluating a project where a property function returns null and its result is concatenated with another non-empty value. ### Changes Made Add a null check. ### Testing Fixed and extended the relevant test case.
dotnet · May 6, 2021 · c040391 · c040391
1 parent fa96a2a
commit c040391
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 12 deletions.
diff --git a/src/Build.UnitTests/Evaluation/Expander_Tests.cs b/src/Build.UnitTests/Evaluation/Expander_Tests.cs
@@ -2010,13 +2010,16 @@ public void PropertyFunctionNullArgument()
         public void PropertyFunctionNullReturn()
         {
             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);
 
-            string result = expander.ExpandIntoStringLeaveEscaped("$([System.Convert]::ChangeType(,$(SomeStuff.GetType())))", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);
-
+            // The null-returning function is the only thing in the expression.
+            string result = expander.ExpandIntoStringLeaveEscaped("$([System.Environment]::GetEnvironmentVariable(`_NonExistentVar`))", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);
             Assert.Equal("", result);
+
+            // The result of the null-returning function is concatenated with a non-empty string.
+            result = expander.ExpandIntoStringLeaveEscaped("prefix_$([System.Environment]::GetEnvironmentVariable(`_NonExistentVar`))", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);
+            Assert.Equal("prefix_", result);
         }
 
         /// <summary>

diff --git a/src/Build/Evaluation/Expander.cs b/src/Build/Evaluation/Expander.cs
@@ -1200,19 +1200,22 @@ private static class PropertyExpander<T>
                             propertyValue = LookupProperty(properties, expression, propertyStartIndex + 2, propertyEndIndex - 1, elementLocation, usedUninitializedProperties);
                         }
 
-                        if (IsTruncationEnabled(options) && propertyValue != null)
+                        if (propertyValue != null)
                         {
-                            var value = propertyValue.ToString();
-                            if (value.Length > CharacterLimitPerExpansion)
+                            if (IsTruncationEnabled(options))
                             {
-                                propertyValue = value.Substring(0, CharacterLimitPerExpansion - 3) + "...";
+                                var value = propertyValue.ToString();
+                                if (value.Length > CharacterLimitPerExpansion)
+                                {
+                                    propertyValue = value.Substring(0, CharacterLimitPerExpansion - 3) + "...";
+                                }
                             }
-                        }
 
-                        // Record our result, and advance
-                        // our sourceIndex pointer to the character just after the closing
-                        // parenthesis.
-                        results.Add(propertyValue);
+                            // Record our result, and advance
+                            // our sourceIndex pointer to the character just after the closing
+                            // parenthesis.
+                            results.Add(propertyValue);
+                        }
                         sourceIndex = propertyEndIndex + 1;
                     }