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

[Bug]: ExpressionShredder.GetReferencedItemNamesAndMetadata expression parser does not handle nested quotes #9940

Open
atykhyy opened this issue Mar 27, 2024 · 0 comments
Labels
Area: Language Issues impacting the MSBuild programming language. backlog bug Priority:3 Work that is nice to have triaged

Comments

@atykhyy
Copy link

atykhyy commented Mar 27, 2024

Issue Description

Task batching on item metadata does not handle complicated expressions like @(a->'$(...)') properly, leading to misdetection of metadata to batch on and/or build errors. The following minimal reproducible example illustrates the problem. I put additional explanations and analysis in MRE comments.

Steps to Reproduce

Uncomment the fragment creating d items to see the error.

<Project>
<ItemGroup>
<a Include='a1;a2'>
  <n>m0</n>
</a>
<a Include='a3'>
  <n>m1</n>
</a>
<b Include='b1'>
  <n>m0</n>
  <m>x1</m>
</b>
<b Include='b2'>
  <n>m1</n>
</b>
</ItemGroup>

<Target Name="Build" >
  <ItemGroup>
    <c Include="@(a)" Condition="'%(n)' != ''">
      <!-- 'm' is not picked up as a metadata item to batch on, because it is in quotes -->
      <m>@(b->'%(m)')</m>
    </c>
  </ItemGroup>
  <ItemGroup>
    <c>
      <!-- this is the sort of thing I want to be able to do without the extra step of adding unchanged
            'm' metadata from matching 'b' items to every 'c' item -->
      <!-- note that if GetReferencedItemNamesAndMetadata's expression parser is fixed to no longer
            pick up metadata names from inside the expression, one might need to add a dummy condition
            to the item declaration to force batching on correct metadata(s) -->
      <m>@(c->'$([System.String]::new('%(m)').Replace('x',%(n)))')</m>
    </c>
  </ItemGroup>
  <ItemGroup>
    <d Include="@(a)" Condition="'%(n)' != ''">
      <!-- 'm' is picked up as a metadata item to batch on, leading to error as 'a' items don't have it -->
      <!-- the reason for this is that GetReferencedItemNamesAndMetadata stops parsing a quoted expression
            at the first quote, does not find the closing brace there, restarts skipping the leading '@',
            and keeps going until it finds the '%(m)' in the middle of the expression -->
      <!--m>@(b->'$([System.String]::new('%(m)'))')</m-->
    </d>
  </ItemGroup>
</Target>
</Project>

Expected Behavior

ExpressionShredder.GetReferencedItemNamesAndMetadata correctly skips the whole quoted expression and does not pick up m as a metadata to batch on.

Actual Behavior

ExpressionShredder.GetReferencedItemNamesAndMetadata mangles the expression and picks up m as a metadata to batch on. In the above MRE this produces a build error because a items do not have the m metadata.

Analysis

The reason for this is that ExpressionShredder.GetReferencedItemNamesAndMetadata stops parsing a quoted expression at the first closing quote, which happens to be in the middle of an expression. It does not find the closing brace there, restarts skipping the leading @, and keeps skipping leading characters of the expression until it finds and parses the %(m) in the middle of the expression.

Versions & Configurations

Verified with msbuild 17.8.5 and with msbuild built from HEAD (2e6f2ff).

@atykhyy atykhyy added the bug label Mar 27, 2024
@AR-May AR-May added Area: Language Issues impacting the MSBuild programming language. backlog Priority:3 Work that is nice to have triaged labels Apr 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Language Issues impacting the MSBuild programming language. backlog bug Priority:3 Work that is nice to have triaged
Projects
None yet
Development

No branches or pull requests

2 participants