Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect whitespace in property name #5672

Merged
merged 12 commits into from
Nov 4, 2020
31 changes: 31 additions & 0 deletions src/Build.UnitTests/Scanner_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

using Microsoft.Build.Evaluation;
using Microsoft.Build.Exceptions;
using Microsoft.Build.Utilities;
using Xunit;


Expand Down Expand Up @@ -100,6 +101,36 @@ public void IllFormedProperty()
Assert.Equal("IllFormedPropertyOpenParenthesisInCondition", lexer.GetErrorResource());
}

/// <summary>
/// Tests the space errors case
/// </summary>
[Fact]
public void SpaceProperty()
{
Scanner lexer = new Scanner("$(x )", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Equal("IllFormedPropertySpaceInCondition", lexer.GetErrorResource());

lexer = new Scanner("$( x)", ParserOptions.AllowProperties);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth adding a test for "$(x yz" and "$(x y z")?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you really mean "$(x yz" and "$(x y z") or actually "$(x yz)" and "$(x y z)"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter 😊

AdvanceToScannerError(lexer);
Assert.Equal("IllFormedPropertySpaceInCondition", lexer.GetErrorResource());
}

[Fact]
public void SpacePropertyOptOutWave16_10()
{
using TestEnvironment env = TestEnvironment.Create();
env.SetChangeWave(ChangeWaves.Wave16_10);

Scanner lexer = new Scanner("$(x )", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Null(lexer.UnexpectedlyFound);

lexer = new Scanner("$( x)", ParserOptions.AllowProperties);
AdvanceToScannerError(lexer);
Assert.Null(lexer.UnexpectedlyFound);
}

/// <summary>
/// Tests the special errors for "@(" and "@x" and similar cases.
/// </summary>
Expand Down
33 changes: 29 additions & 4 deletions src/Build/Evaluation/Conditionals/Scanner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;

using Microsoft.Build.Shared;
using Microsoft.Build.Utilities;

namespace Microsoft.Build.Evaluation
{
Expand Down Expand Up @@ -283,8 +284,17 @@ private string ParsePropertyOrItemMetadata()
return null;
}

_parsePoint = ScanForPropertyExpressionEnd(_expression, _parsePoint++);
var result = ScanForPropertyExpressionEnd(_expression, _parsePoint++, out int indexResult);
if (!result)
{
_errorState = true;
_errorPosition = indexResult;
_errorResource = "IllFormedPropertySpaceInCondition";
_unexpectedlyFound = Convert.ToString(_expression[indexResult], CultureInfo.InvariantCulture);
return null;
}

_parsePoint = indexResult;
// Maybe we need to generate an error for invalid characters in property/metadata name?
// For now, just wait and let the property/metadata evaluation handle the error case.
if (_parsePoint >= _expression.Length)
Expand All @@ -303,9 +313,15 @@ private string ParsePropertyOrItemMetadata()
/// <summary>
/// Scan for the end of the property expression
/// </summary>
private static int ScanForPropertyExpressionEnd(string expression, int index)
/// <param name="expression">property expression to parse</param>
/// <param name="index">current index to start from</param>
/// <param name="indexResult">If successful, the index corresponds to the end of the property expression.
/// In case of scan failure, it is the error position index.</param>
/// <returns>result indicating whether or not the scan was successful.</returns>
private static bool ScanForPropertyExpressionEnd(string expression, int index, out int indexResult)
{
int nestLevel = 0;
bool whitespaceCheck = false;

unsafe
{
Expand All @@ -317,18 +333,26 @@ private static int ScanForPropertyExpressionEnd(string expression, int index)
if (character == '(')
{
nestLevel++;
whitespaceCheck = true;
}
else if (character == ')')
{
nestLevel--;
whitespaceCheck = false;
}
else if (whitespaceCheck && char.IsWhiteSpace(character) && ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10))
{
indexResult = index;
return false;
}

// We have reached the end of the parenthesis nesting
// this should be the end of the property expression
// If it is not then the calling code will determine that
if (nestLevel == 0)
{
return index;
indexResult = index;
return true;
}
else
{
Expand All @@ -337,7 +361,8 @@ private static int ScanForPropertyExpressionEnd(string expression, int index)
}
}
}
return index;
indexResult = index;
return true;
}

/// <summary>
Expand Down
4 changes: 4 additions & 0 deletions src/Build/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,10 @@
<value>MSB4110: Expected a property at position {1} in condition "{0}". Did you forget the opening parenthesis after the '$'? To use a literal '$', use '%24' instead.</value>
<comment>{StrBegin="MSB4110: "}</comment>
</data>
<data name="IllFormedPropertySpaceInCondition" xml:space="preserve">
<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="IllFormedQuotedStringInCondition" xml:space="preserve">
<value>MSB4101: Expected a closing quote after position {1} in condition "{0}".</value>
<comment>{StrBegin="MSB4101: "}</comment>
Expand Down
5 changes: 5 additions & 0 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.

5 changes: 5 additions & 0 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.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.en.xlf

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

5 changes: 5 additions & 0 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.

5 changes: 5 additions & 0 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.

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.it.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ja.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ko.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.pl.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.pt-BR.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.ru.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.tr.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.zh-Hans.xlf

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

5 changes: 5 additions & 0 deletions src/Build/Resources/xlf/Strings.zh-Hant.xlf

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