Skip to content

Distinguish present but empty and not present metadata in item functions #8408

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.

Sign up for GitHub

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

Closed
JaynieBai wants to merge 4 commits into from
Closed

Distinguish present but empty and not present metadata in item functions #8408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
90ab81f
return null if metadata not present
JaynieBai Feb 7, 2023
4ff257f
Merge branch 'main' of https://github.com/dotnet/msbuild into jennyb/…
JaynieBai Feb 7, 2023
2cce2ed
Add test
JaynieBai Feb 7, 2023
0e5d373
Fix the failed cases
JaynieBai Feb 7, 2023
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
72 changes: 71 additions & 1 deletion src/Build.UnitTests/Evaluation/Expander_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,77 @@ public void HasMetadata()

</Project>");

logger.AssertLogContains("[One|Three|Four]");
logger.AssertLogContains("[One|Three|Four|Five]");
}

/// <summary>
/// Test metadata item functions with empty string metadata and not present metadata
/// </summary>
[Fact]
public void MetadataFuntionTestingWithEmtpyString()
{
MockLogger logger = Helpers.BuildProjectWithNewOMExpectSuccess("""
<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">

<ItemGroup>
<_Item Include="One">
<A>true</A>
</_Item>
<_Item Include="Two">
<A>false</A>
</_Item>
<_Item Include="Three">
<A></A>
</_Item>
<_Item Include="Four">
<B></B>
</_Item>
</ItemGroup>

<Target Name="Tests" DependsOnTargets="WithMetadataValueAFalse;WithMetadataValueAEmpty;WithOutMetadataValueAEmtpy;HasMetadataA;WithMetadataValueCEmpty;HasMetadataC;AnyHaveMetadataValueCEmpty;WithOutMetadataValueCEmpty" />

<Target Name="WithMetadataValueAFalse">
<Message Text="WithMetadataValueAFalse: [@(_Item->WithMetadataValue('A', 'false'), '|')]"/>
</Target>

<Target Name="WithMetadataValueAEmpty">
<Message Text="WithMetadataValueAEmpty: [@(_Item->WithMetadataValue('A', ''), '|')]"/>
</Target>

<Target Name="WithOutMetadataValueAEmtpy">
<Message Text="WithOutMetadataValueAEmpty: [@(_Item->WithOutMetadataValue('A', ''), '|')]"/>
</Target>

<Target Name="HasMetadataA">
<Message Text="HasMetadataA: [@(_Item->HasMetadata('A'), '|')]"/>
</Target>

<Target Name="WithMetadataValueCEmpty">
<Message Text="WithMetadataValueCEmpty: [@(_Item->WithMetadataValue('C', ''), '|')]"/>
</Target>

<Target Name="HasMetadataC">
<Message Text="HasMetadataC: [@(_Item->HasMetadata('C'), '|')]"/>
</Target>

<Target Name="AnyHaveMetadataValueCEmpty">
<Message Text="AnyHaveMetadataValueCEmpty: [@(_Item->AnyHaveMetadataValue('C', ''), '|')]"/>
</Target>

<Target Name="WithOutMetadataValueCEmpty">
<Message Text="WithOutMetadataValueCEmpty: [@(_Item->WithOutMetadataValue('C', ''), '|')]"/>
</Target>

</Project>
""");
logger.AssertLogContains("WithMetadataValueAFalse: [Two]");
logger.AssertLogContains("WithMetadataValueAEmpty: [Three]");
logger.AssertLogContains("WithOutMetadataValueAEmpty: [One|Two|Four]");
logger.AssertLogContains("HasMetadataA: [One|Two|Three]");
logger.AssertLogContains("WithMetadataValueCEmpty: []");
logger.AssertLogContains("HasMetadataC: []");
logger.AssertLogContains("AnyHaveMetadataValueCEmpty: [false]");
logger.AssertLogContains("WithOutMetadataValueCEmpty: [One|Two|Three|Four]");
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion src/Build/BackEnd/Components/RequestBuilder/BatchingEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ private static Dictionary<string, string> GetItemMetadataValues(
{
// This returns String.Empty for both metadata that is undefined and metadata that has
// an empty value; they are treated the same.
itemMetadataValues[metadataQualifiedName] = ((IItem)item).GetMetadataValueEscaped(metadataName);
itemMetadataValues[metadataQualifiedName] = ((IItem)item).GetMetadataValueEscaped(metadataName) ?? string.Empty;
}
catch (InvalidOperationException e)
{
Expand Down
44 changes: 40 additions & 4 deletions src/Build/Evaluation/Expander.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2579,7 +2579,7 @@ internal static IEnumerable<Pair<string, S>> Metadata(Expander<P, I> expander, I
yield return new Pair<string, S>(metadataValue, item.Value);
}
}
else if (metadataValue != String.Empty && includeNullEntries)
else if (metadataValue != null && includeNullEntries)
{
yield return new Pair<string, S>(metadataValue, item.Value);
}
Expand Down Expand Up @@ -2764,9 +2764,8 @@ internal static IEnumerable<Pair<string, S>> HasMetadata(Expander<P, I> expander
ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "CannotEvaluateItemMetadata", metadataName, ex.Message);
}

// GetMetadataValueEscaped returns empty string for missing metadata,
// but IItem specifies it should return null
if (!string.IsNullOrEmpty(metadataValue))
// GetMetadataValueEscaped returns null for missing metadata
if (metadataValue != null)
{
// return a result through the enumerator
yield return new Pair<string, S>(item.Key, item.Value);
Expand Down Expand Up @@ -2810,6 +2809,43 @@ internal static IEnumerable<Pair<string, S>> WithMetadataValue(Expander<P, I> ex
}
}

/// <summary>
/// Intrinsic function that returns those items don't have the given metadata value
/// Using a case insensitive comparison.
/// </summary>
///
internal static IEnumerable<Pair<string, S>> WithOutMetadataValue(Expander<P, I> expander, IElementLocation elementLocation, bool includeNullEntries, string functionName, IEnumerable<Pair<string, S>> itemsOfType, string[] arguments)
{
ProjectErrorUtilities.VerifyThrowInvalidProject(arguments?.Length == 2, elementLocation, "InvalidItemFunctionSyntax", functionName, arguments == null ? 0 : arguments.Length);

string metadataName = arguments[0];
string metadataValueToFind = arguments[1];

foreach (Pair<string, S> item in itemsOfType)
{
string metadataValue = null;

try
{
metadataValue = item.Value.GetMetadataValueEscaped(metadataName);
}
catch (ArgumentException ex) // Blank metadata name
{
ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "CannotEvaluateItemMetadata", metadataName, ex.Message);
}
catch (InvalidOperationException ex)
{
ProjectErrorUtilities.ThrowInvalidProject(elementLocation, "CannotEvaluateItemMetadata", metadataName, ex.Message);
}

if (!String.Equals(metadataValue, metadataValueToFind, StringComparison.OrdinalIgnoreCase))
{
// return a result through the enumerator
yield return new Pair<string, S>(item.Key, item.Value);
}
}
}

/// <summary>
/// Intrinsic function that returns a boolean to indicate if any of the items have the given metadata value
/// Using a case insensitive comparison.
Expand Down
13 changes: 7 additions & 6 deletions src/Build/Instance/ProjectItemInstance.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -555,7 +555,7 @@ string IMetadataTable.GetEscapedValueIfPresent(string itemType, string name)
{
if (itemType == null || String.Equals(itemType, _itemType, StringComparison.OrdinalIgnoreCase))
{
string value = _taskItem.GetMetadataEscaped(name);
string value = _taskItem.GetMetadataEscaped(name) ?? string.Empty;

if (value.Length > 0 || HasMetadata(name))
{
Expand Down Expand Up @@ -1253,12 +1253,13 @@ ProjectMetadataInstance IItem<ProjectMetadataInstance>.SetMetadata(ProjectMetada
/// </summary>
public string GetMetadata(string metadataName)
{
return EscapingUtilities.UnescapeAll(GetMetadataEscaped(metadataName));
return EscapingUtilities.UnescapeAll(GetMetadataEscaped(metadataName) ?? string.Empty);
}

/// <summary>
/// Returns the specified metadata value, escaped.
/// If metadata is not defined, returns empty string.
/// If metadata is not defined, returns null.
/// If metadata is defined and value is empty, returns empty.
/// </summary>
public string GetMetadataEscaped(string metadataName)
{
Expand Down Expand Up @@ -1293,7 +1294,7 @@ public string GetMetadataEscaped(string metadataName)

string value = GetBuiltInMetadataEscaped(metadataName);

return value ?? String.Empty;
return value;
}

/// <summary>
Expand Down Expand Up @@ -1810,11 +1811,11 @@ internal TaskItem DeepClone(bool isImmutable)
/// <summary>
/// Helper to get the value of a built-in metadatum with
/// the specified name, if any.
/// If value is not available, returns empty string.
/// If value is not available, returns null.
/// </summary>
private string GetBuiltInMetadataEscaped(string name)
{
string value = String.Empty;
string value = null;

if (FileUtilities.ItemSpecModifiers.IsItemSpecModifier(name))
{
Expand Down
2 changes: 1 addition & 1 deletion src/Utilities/TaskItem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ public void SetMetadata(
/// <returns>The metadata value.</returns>
public string GetMetadata(string metadataName)
{
string metadataValue = (this as ITaskItem2).GetMetadataValueEscaped(metadataName);
string metadataValue = (this as ITaskItem2).GetMetadataValueEscaped(metadataName) ?? string.Empty;
return EscapingUtilities.UnescapeAll(metadataValue);
}

Expand Down