Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions documentation/wiki/ChangeWaves.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ 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)

Expand Down
67 changes: 5 additions & 62 deletions src/Build.UnitTests/Evaluation/Expander_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3854,71 +3854,14 @@ public void PropertyFunctionStaticMethodIntrinsicBitOperations()
[Fact]
public void PropertySimpleSpaced()
{
using (TestEnvironment env = TestEnvironment.Create())
{
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave18_4.ToString());

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(@"$( 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"));
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);
Expander<ProjectPropertyInstance, ProjectItemInstance> expander = new Expander<ProjectPropertyInstance, ProjectItemInstance>(pg, FileSystems.Default);

InvalidProjectFileException ex = Assert.Throws<InvalidProjectFileException>(
() => expander.ExpandIntoStringLeaveEscaped(expression, ExpanderOptions.ExpandProperties, MockElementLocation.Instance));
string result = expander.ExpandIntoStringLeaveEscaped(@"$( SomeStuff )", ExpanderOptions.ExpandProperties, MockElementLocation.Instance);

Assert.Equal("MSB4281", ex.ErrorCode);
}
Assert.Equal(String.Empty, result);
}

[WindowsOnlyFact]
Expand Down
47 changes: 2 additions & 45 deletions src/Build/Evaluation/Expander.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1385,30 +1385,7 @@ internal static object ExpandPropertiesLeaveTypedAndEscaped(
}
else // This is a regular property
{
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);
propertyValue = LookupProperty(properties, expression, propertyStartIndex + 2, propertyEndIndex - 1, elementLocation, propertiesUseTracker);
}

if (propertyValue != null)
Expand Down Expand Up @@ -1457,27 +1434,7 @@ internal static object ExpandPropertyBody(
Function<T> function = null;
string propertyName = propertyBody;

// 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):
// Trim the body for compatibility reasons:
// 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.
Expand Down
6 changes: 1 addition & 5 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -529,10 +529,6 @@
<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>
Expand Down Expand Up @@ -2466,7 +2462,7 @@ Utilization: {0} Average Utilization: {1:###.0}</value>
<!--
The Build message bucket is: MSB4000 - MSB4999

Next message code should be MSB4282
Next message code should be MSB4281

Don't forget to update this comment after using a new code.
-->
Expand Down
15 changes: 5 additions & 10 deletions src/Build/Resources/xlf/Strings.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 5 additions & 10 deletions src/Build/Resources/xlf/Strings.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 5 additions & 10 deletions src/Build/Resources/xlf/Strings.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 5 additions & 10 deletions src/Build/Resources/xlf/Strings.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading