Add support for named argument with .NET Methods

[jborean93]

PR Summary

Adds support for using named arguments when calling .NET methods.
For example:

[System.IO.Path]::GetFullPath(path: "test")

# Both of the below are the same
[string]::new('a', count: 20)
[string]::new(
    c: 'a',
    count: 20)
# aaaaaaaaaaaaaaaaaaaa

Through named args we can provide optional parameters without relying on the position or by specifying all preceding defaults:

Add-Type -TypeDefinition @'
using System;

public class TestClass
{
    public static string Method(string arg1, string default1 = "foo", string default2 = "bar")
    {
        return $"{arg1}-{default1}-{default2}";
    }
}
'@

[TestClass]::Method("test", default2: "other")
# test-foo-other

What is not in scope of this PR

• Support for named arguments for other method binders (COM) - COM is already piped in, other binder can work already if they support CallInfo.ArgumentNames.
• This only works for a .NET method (PSMethod)
  • PSScriptMethod could be expanded in the future to support passing in args through a parameter name
  • PSCodeMethod could be expanded in the future to support the same optional argument passing
  • Currently the names are just ignored for everything except PSMethod, this aligns to how Generics are handled for these method invocations
• PowerShell classes can use named arguments but optional values cannot be omitted
  • This is covered by a different issue Make methods in custom classes support optional parameters #9701
• Autocompletion support
  • Maybe @MartinGC94 can provide some hints/help there :)

PR Context

Issue to track this has closed due to no activity but it is still relevant #13307 and #7506 (sans splatting).

The changes here are done through

• New AST class LabeledExpressionAst which is only parsed on method invocations
• Argument names are provided to the binder through the CallInfo.ArgumentNames property
• Selecting the .NET overload now takes into account the caller supplied argument name

Assumptions Made

Named labels only support the simple name rules in PowerShell

A simple label supports the a subset of the rules as a C# identifier.
A quick test shows chars in the Nl, Pc, Mn, Mc, and Cf Unicode categories might be invalid.

Future work could be introduced to expand support these invalid chars through an escape char like \u0000, \U00000000, or the backtick ```u{}`` syntax but considering the rarity of these identifiers I'm not sure it's worth the effort.

Named labels cannot be following by unnamed/positional argument

When specifying a named argument, any subsequent args must also be named.

[System.IO.FileStream]::new(
    "file.txt",
    mode: 'Open',
    access: 'Read',
    share: 'ReadWrite')

While C# allows named arguments before positional arguments it is only if the name also aligns with the position.
Supporting such a scenario is not feasible in PowerShell as:

• CallInfo.ArgumentNames only allows names at the end of the arguments
• The way C# works is more compiler magic, using a dynamic class doesn't allow this syntax due to the above
• Supporting this in the parser just isn't feasible in PowerShell as both the method and overload selection is reliant on runtime values in the majority of cases

Named argument can be in any order but positional takes priority

Further to the above, the order of the named args do not matter but they will only apply after the positional arguments are checked.
For example the overload

public FileStream (string path, System.IO.FileMode mode, System.IO.FileAccess access, System.IO.FileShare share);

[System.IO.FileStream]::new(
    "file.txt",
    'Open',
    mode: 'Read',
    share: 'ReadWrite')

The above will not work because file.txt will be provided to path, Open will be set to mode, making the next named argument invalid because mode is already specified.

[System.IO.FileStream]::new(
    "file.txt",
    access: 'Read',
    share: 'ReadWrite',
    mode: 'Open')

Will work as the first positional argument is set to the path argument while the rest are all named and relate to the remaining args.

Named arguments are case insensitive

Edit: This was originally case sensitive but was changed in a recent commit.

This follows the normal standard in PowerShell where most things are case insensitive allowing you to do

[System.IO.Path]::Combine(path1: 'foo', path2: 'bar')
[System.IO.Path]::Combine(Path1: 'foo', pAth2: 'bar')
...

This only applies to the .NET binder, COM will be case sensitive as well as any other customer binder that is used as the original case is preserved in the AST and it's up to the binder to decide on how to treat them.

Unnamed arguments in .NET are called argx

It is possible to use reflection to define a method where each argument has no name.
The current code will automatically translate that to arg$idx where $idx corresponds to the parameter index (starting from 0).
This allows labels to still be used for this code like

$class.Method(arg0: 'foo', arg1: 'bar')

Name conflicts are resolved by adding _ suffix

While rare it is possible to have a collision with the argument names when

• The method uses arguments that differ in cases
• The method was defined with reflection with the same name
• The method was defined without a name and the automatic arg$idx calculation conflicts with an explicitly named one

These scenarios would be quite rare but it's still something that should be considered.
In the case of a conflict the code will keep on adding a _ suffix until the argument name is unique enough.
For example in the below case where reflection would be used to create this definition where all args are named arg in the metata:

public static void Method(string arg, string arg, string arg)

PowerShell can call this with

[Type]::Method(arg: 'foo', arg_: 'bar', arg__: 'test')

For case sensitive conflicts the below would apply

Add-Type -TypeDefinition @'
using System;

public class TestClass
{
    public static string Method(string arg, string Arg) => $"{arg}-{Arg}";
}
'@

[TestClass]::Method(arg: "1", arg_: "2")
# 1-2

As arguments are parsed left to right, the leftmost one will have no suffix.
For example with the below where the first argument is unnamed (...) in the metadata while the second arg uses the automatic name value the code here will use:

public static void Method(string ..., string arg0);

PowerShell can call this with

[Type]::Method(arg0: 'foo', arg0_: 'bar')

A named param argument must be set as an array if multiple values are specified

When specifying a value for a params argument through a named arg the value supplied can only be provided through the one argument value.
Just like a normal param specification this value can be a single value which is casted to the array at runtime or as the array itself.

[System.IO.Path]::Combine(paths: 'a')
[System.IO.Path]::Combine(paths: [string[]]@('a', 'b'))

This behaviour is the same as how a positional params argument can be supplied when only as one argument.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and opened an issue in the relevant tool repository. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode
      (which runs in a different PS Host).
      • Issue filed:
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
      • Issue filed:
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
      • Issue filed:
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).
      • Issue filed:

[jborean93]

Looks like this affects COM, will have to test it out a bit more to ensure that unnamed argument continue to work as before.

[jborean93]

After testing it seems like the existing COM code can handle named arguments already it just needed to be plumbed through. I've updated the logic for populating the CallInfo.ArgumentNames value based on the logic .NET uses for it's binder. This has enabled COM to just work when providing named arguments.

[MartinGC94]

Awesome, I've wanted this in PS ever since I learned that it was a thing in C#. With tab completion this will make it much easier to call methods in the CLI because I no longer have to remember all the different arguments. It's a bit unfortunate that it's case sensitive though. I understand the reasoning but I'm not sure it's the right choice. PS is case insensitive in every other area, including .NET type name/member resolution and I haven't seen many complaints about that despite the flaws in the implementation:

That's probably because people in practice rarely use the same name for 2 different things because it's confusing.

[jborean93]

It's a bit unfortunate that it's case sensitive though. I understand the reasoning but I'm not sure it's the right choice

I can definitely understand the pros of making it case insensitive. If the pwsh team favours that over case sensitivity I can update the PR to be so but we would have to determine what happens when such a method is encountered, e.g.

• Should it favour the first or last defined argument on a case insensitive conflict
• Should it try and define some fallback option in case subsequent arguments conflict with the previous names or should it just not be identifiable at all
  • This can cause problems as if an argument cannot be labeled than previous argument can't be labeled at all (names must always be used after the first named argument)
  • If there is a fallback, what happens when a real argument also conflict with that
  • We could favour a case sensitive check then an insensitive one if there is no match but I'm not sure what the performance implications of that would be

I do think the probabilities of such a scenario happening are pretty small so it's worth considering whether to make it case sensitive, we just need to make sure we don't paint ourselves into a corner here.

[jborean93]

Looks like it is possible to have reflection build a method with arguments of the same name so we'll have to solve the name conflict problem in any case.

[jborean93]

I've just pushed a commit that makes it case insensitive as it looked like we needed to deal with collisions anyway so having it fit in with PowerShell's case insensitivity made more sense. When there is a conflict the overload selector will just append the _ suffix until the name is unique enough. For example the below now works (arg names can be in any case)

Add-Type -TypeDefinition @'
using System;

public class TestClass
{
    public static string Method(string arg, string Arg, string aRg) => $"{arg}{Arg}{aRg}";
}
'@

[TestClass]::Method(arg: 'foo', arg_: 'bar', arg__: 'test')

I did not touch COM as that's complicated as heck. It only works because of the code copied from .NET already supported named args from the binder, so that stays case sensitive.

I'll update the summary to include this assumption and the behaviour around conflicts.

[jborean93]

I give up on trying to deal with CI, will rebase when #21463 is merged but that's the last remaining failure which is unrelated to the changes here.

[MartinGC94]

@jborean93 I've made a PR with some change suggestions for the parsing here: jborean93#2 feel free to take a look.

[MartinGC94]

I've added completion for this in my own branch here: https://github.com/MartinGC94/PowerShell/tree/NamedArgCompletion that people can test out if they want to. I'll create a PR for it once this one gets merged.

GitHub

GitHub - MartinGC94/PowerShell at NamedArgCompletion

PowerShell for every system! Contribute to MartinGC94/PowerShell development by creating an account on GitHub.

[jborean93]

Awesome, I'll have to try it out and see how I go. It's a bit hard at the moment to really test out console things on Linux as PowerShell is still tracking an old preview release so suffers from a problem rendering on the console once you reach 80 or so chars.

[vexx32]

I really don't think I have any notes on this one. This looks really damn good. Fantastic job @jborean93 💜

[SeeminglyScience]

Just an FYI, the Engine WG discussed this today but we need more time now that we've fully dug into the meat of it, and will continue discussions in the next WG meeting.

[powercode]

In general, I think this looks good.

Regarding naming: I think we can come up with a better name than LabeledExpession.

Compare where we have a NamedAttributeArgumentAst. Label makes me think of goto :).

How about NamedMethodArgumentExpressionAst?

Do we need to add to add an AstVisitor3 and ICustomAstVisitor3?
Edit: I read again and saw the calls to Default(...). We should be good.

[jborean93]

@powercode thanks for having a look through the PR!

Regarding naming: I think we can come up with a better name than LabeledExpession.

While right now the Ast parser only creates this for named arguments my thinking was that this Ast could be re-used in the future for any other expression location where we might want to support a name/label. I went with label here because we somewhat already use it as a term for loops where they can have a label attached to the loop itself

If we wanted to restrict this Ast to method argument expressions only then I’m happy with your suggestion NamedMethodArgumentExpressionAst.
But I do think we should consider if we ever want to reuse this in other places for future functionality.

[powercode]

I think they are semantically different and that you want separate asts for different semantics.

Consider an AstSearcher, where you want to find certain expressions. Reused asts then becomes a hindrance, not a help.

Overall, however, I concur with Rain - great work

[SeeminglyScience]

Oh almost forgot, also we agree that LabelExpressionAst should be NamedMethodArgumentAst

[jborean93]

Oh almost forgot, also we agree that LabelExpressionAst should be NamedMethodArgumentAst

Will do, thanks for confirming.

---

Because of potential conflicts and just additional complexity we would prefer to not support specifying arguments that have no name.

I'm happy to remove the unnamed and duplicate argument support but I would argue it potentially makes the implementation more complex. The current code uses a dictionary as a way to map the value to the invoke argument indexes. https://github.com/PowerShell/PowerShell/pull/21487/files#diff-53ef05349dd26a08b5a495fb5924ec6a95533841e9d9dab40335c5f7fbd5154aR1704-R1725

By removing the fallback naming logic I would either have to generate a random id, maybe a GUID, for such arguments to allow the existing logic or move to using a list with an O(n) instead of O(1) when looking up the keys. It's certainly a very uncommon scenario that I think the existing implementation is ok but I'll leave that up to the WG to decide.

---

We would also request this to be wrapped as an experimental feature.

I must strongly object to the proposal of wrapping this as an experimental feature. I believe the working group should reconsider for several reasons:

1. Already Opt-In by Nature

   The new behavior is inherently an opt-in feature, as users must explicitly provide named arguments to activate it.
   This existing opt-in mechanism reduces the need for an additional experimental flag, which would unnecessarily complicate the process.

2. Increased Code Complexity

   Introducing this feature as experimental adds unnecessary complexity to an already intricate codebase as both the old and new code paths need to be kept.
   It introduces additional overhead for runtime checks and questions about when and how to apply these checks.

3. Lack of Testing Support

   Experimental features typically lack comprehensive testing support, leading to potential coverage gaps.
   The current code and proposed changes have been thoroughly tested for various argument binding and overload scenarios. Any missed scenarios should be addressed in review and subsequent testing phases.

4. Early Testing in Preview Versions

   Releasing the feature early in preview versions can help identify and address potential issues that were missed in the implementation and review before the final release.
   Delaying this by making it experimental increases the risk of undiscovered bugs, potentially impacting the final release quality.
   With 7.5 being a non-LTS version I also believe now is the best time to implement such a feature. Implementing this feature in 7.6 means that there is less time for this to be proven in the wide before people move onto the next LTS release.

5. Barrier to Adoption

   Experimental features can act as a barrier, making it harder for users to discover and adopt the feature.
   Users already opt-in by using named arguments, so adding another layer of opt-in is redundant and counterproductive.

6. Maintenance Burden

   Maintaining dual code paths (existing behavior and new behavior behind an experimental flag) significantly increases maintenance burden.
   It complicates the parsing and compiler phases, making future modifications more challenging.

7. Ambiguity and Overhead

   Introducing an experimental flag raises questions about error handling and behavior when the flag is disabled but named arguments are used.
   Deciding where to handle errors (parser, compiler, or binder) adds complexity and potential overhead.
   Supporting named arguments in a remote target through Start-Job or Invoke-Command when the feature is not enabled in the current process is another question that needs to be answered

8. Developer Experience and Testing

   The current lack of documentation and support for implementing and testing experimental features hampers developer productivity.
   Testing both enabled and disabled states of an experimental feature is cumbersome and often leads to inadequate test coverage.

Ultimately, the disadvantages of making this an experimental feature outweigh the benefits. Given that named arguments are already an opt-in feature, the main benefit of an experimental feature—safeguarding against breaking changes—is largely redundant. If the working group is still concerned about potential breaking changes, a compromise could be to use the old overload selection code when no named arguments are specified and the new code when they are. However, this still introduces more complexity and maintenance burden in the long run.

I appreciate the time and effort put into reviewing this complex PR and hope these points help in reconsidering the decision to avoid an experimental feature.

GitHub

Add support for named argument with .NET Methods by jborean93 · Pull Request #21487 · PowerShell/PowerShell

PR Summary Adds support for using named arguments when calling .NET methods. For example: [System.IO.Path]::GetFullPath(path: "test")

Both of the below are the same