Permit throwaway parameters

[Forgind]

Fixes #

Context

This is to permit users to use functions with out parameters and ref parameters as long as they don't care about the values that come out of them. Specifically, if a parameter is specified by '_', it will look for any method with the right name from the right assembly and try to match based on number of parameters. If there are multiple options, it will throw; if there is only one, it will execute it after tweaking all the _ to have the right type.

Changes Made

Testing

I used this project file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net9.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
  </PropertyGroup>

  <PropertyGroup>
    <FooProp>$([System.Int32]::TryParse("bah", _))</FooProp>
  </PropertyGroup>

  <Target Name="Foo">
    <Message Importance="High" Text="Found value $(FooProp)" />
  </Target>

</Project>

This printed out 'Found value False' when built with /t:Foo. I replaced 'bah' with '3', and it printed 'Found value True' instead.

Notes

/cc: @baronfel @rainersigwald This is currently a draft for three reasons:

1. I'm not a huge fan of requiring outParamType, but I'm not sure if there's a better way of doing this. I figure it would be good to get feedback on that and/or think about it more. This is cleaner now.
2. I haven't tested this all that much.
3. I'd like to make it so you can specify a property, and we'll take the output parameter and stuff it in that. That might be hard, though, so I'm not fully committed to that yet. rainersigwald suggested I shouldn't worry about this, and baronfel said it was fine to leave out, at least for a first iteration.

[AR-May]

If there are multiple options, it will throw; if there is only one, it will execute it after tweaking all the _ to have the right type.

I am a bit concerned about this behavior. What if the msbuild project will be written assuming the only overload of some function exists and this changes - for example a new overload of the function will be added, having the same parameters except for the out parameter? This will break build.

If it were a C# code, the error would be thrown by a compiler about ambiguity when the dependency version is upgraded. In this case the project will stop correctly build with all higher msbuild versions, if I understand it correctly.

[Forgind]

If there are multiple options, it will throw; if there is only one, it will execute it after tweaking all the _ to have the right type.

I am a bit concerned about this behavior. What if the msbuild project will be written assuming the only overload of some function exists and this changes - for example a new overload of the function will be added, having the same parameters except for the out parameter? This will break build.

If it were a C# code, the error would be thrown by a compiler about ambiguity when the dependency version is upgraded. In this case the project will stop correctly build with all higher msbuild versions, if I understand it correctly.

I think your concern here is totally valid. In fact, the first way I implemented this got around that as a potential problem by having the user specify which type they wanted for every out parameter, so it would look like this:
$([System.Int32]::TryParse($(VariableThatMightBeAnInteger), outParamSystem.Int32))

The positive side of that is that it would resolve any possible ambiguities by having the user resolve them for us. The downside is that that notation is ugly and nonintuitive. Since _ is what C# uses, that's more intuitive as well as being much cleaner.

I honestly don't know how common this would be as a problem. Since C# permits you to specify out parameters with _, I would hope that they don't have ambiguous terms like that built into the language—would adding one count as a breaking change for C#?—but they may exist. I did put in a custom error message that would hopefully be clear enough that any user encountering it would know what went wrong and how to fix it, but I know error messages are never as clear to users as they are to the people who created them.

One saving grace for this PR is that it explicitly does not break anything that currently works. You can't currently specify a parameter with _ in MSBuild. The only code path that this PR affects is if you start doing that. So although you're right that someone could start relying on this change, then be suddenly broken by a change in C# that adds a new overload for a method, I believe that's the only way they could be broken—they won't be broken by this change until they decide to use it.

I'm honestly ok going down either path: pretty parameter or no possibility of a change in some assembly breaking someone's build through this feature.

[KirillOsenkov]

Looks OK to me, should be harmless and adds a bit of value

[JanKrivanek]

Nice improvement.

Let's please request the https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions documentation update (via https://github.com/dotnet/docs/issues, or direct PR in https://github.com/MicrosoftDocs/visualstudio-docs-pr) and make it explicit there is a limitation with possible backwards compatibility issues

[Forgind]

Nice improvement.

Let's please request the https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions documentation update (via https://github.com/dotnet/docs/issues, or direct PR in https://github.com/MicrosoftDocs/visualstudio-docs-pr) and make it explicit there is a limitation with possible backwards compatibility issues

Done!