Auto-escape MSBuild special characters in MsBuildArgument

[Gal-lankri]

Summary

This PR adds automatic escaping of MSBuild-reserved characters in the MsBuildArgument class (e.g., ;, %, $, @, etc.), so users no longer need to manually encode these characters when passing MSBuild parameters.

Why

Previously, developers had to encode characters like semicolons manually (; → %3B). This PR improves usability and correctness when passing multiple values to MSBuild properties such as DefineConstants.

Changes

• Updated MsBuildArgument constructor to auto-escape special characters
• Added a [Theory]-based test (EscapesSpecialCharactersCorrectly) to cover:
  • Semicolons
  • Percent
  • Dollar sign
  • At-symbol
  • Parentheses
  • Asterisk
  • Normal input (no escape)

Example

new MsBuildArgument("/p:DefineConstants=DEBUG;TRACE")
// becomes:
"/p:DefineConstants=DEBUG%3BTRACE"

[Gal-lankri]

@dotnet-policy-service agree

[timcassell]

@Gal-lankri Can you please pull latest upstream master to your fork and rebase your branch? Github is picking up a lot of unrelated changes. [Edit] Or explain why <Platforms>AnyCPU;x64</Platforms> was added to all the projects?

[Gal-lankri]

@timcassell sorry that's was by mistake. I will fix that.

[timcassell]

If anyone has already escaped their arguments, this will break them. Is there a way to avoid any breaks?

[Gal-lankri]

Great point — thanks for flagging that!

You're absolutely right: blindly escaping everything could break cases where users already passed escaped values like %3B or %25.

To handle this safely, I updated the implementation to:

• Detect and preserve only escape sequences that match known MSBuild-escaped values (like %3B, %24, etc.)
• This prevents double-escaping for cases where users already encoded special characters, while still escaping unescaped input correctly

Additionally, if preferred, I can also add a bool alreadyEscaped = false parameter to the MsBuildArgument constructor. That way:

• Power users can bypass escaping entirely if they know their input is already safe
• Default behavior remains user-friendly for most use cases

Let me know if you'd prefer we add that constructor overload as well. Happy to follow up with it!

[timcassell]

It can get weird if someone puts 100%25, it's ambiguous to us whether it should be 100% or 100%25. I think it would be better to add a bool escapeSpecialChars = true param (or false default to preserve existing behavior), and don't try to parse for existing escapes.

[Gal-lankri]

Yes, good point, thank you!

You're right, there's no reliable way to guess the user's intent in edge cases like 100%25. Maybe it's smarter to let the user to decide.

I've updated the implementation to:

• Add a bool escapeSpecialChars = true parameter to the MsBuildArgument constructor
• If true, the value is escaped
• If false, the value is passed through exactly as provided (for pre-escaped input)

This avoids ambiguity and doesn't change the default behavior for existing users.

[filzrev]

This issue is tracked by #2719 also.

I though it's better to separate MSBuildArgument's TextRepresentation with argument that is used inside .bat/.sh files.
https://github.com/dotnet/BenchmarkDotNet/blob/v0.14.0/src/BenchmarkDotNet/Toolchains/DotNetCli/DotNetCliCommand.cs#L203C72-L203C90.

Because TextRepresentation is shown on Argument column on benchmark report and it's hard to read if contains escape chars.

Additionally, It seems escape is needed when passing MSBuild parameter with -p:{key}={value} and -rp:{key}={value} format.
As far as I've confirmed, Other msbuild command argument works without escaping (e.g. -getProperty:Prop1;Prop2)

[timcassell]

Because TextRepresentation is shown on Argument column on benchmark report and it's hard to read if contains escape chars.

That's a good point. MsBuildArgument can override ToString to display the original value (or add another internal method to get the escaped value, which I think is what you were getting at).

Additionally, It seems escape is needed when passing MSBuild parameter with -p:{key}={value} and -rp:{key}={value} format. As far as I've confirmed, Other msbuild command argument works without escaping (e.g. -getProperty:Prop1;Prop2)

Seems like a good reason to not escape by default. Users should opt-in rather than opt-out.