Add @@ syntax for inline splatting as hashtable as new experimental feature PSGeneralizedSplatting

[bergmeister]

PR Summary

This is the first feature of RFC002 for enhanced splatting.
As agreed in the accepted RFC, one should not need to declare a variable in order to splat. This PR adds the ability to definite the splatted hashtable in the same line as the command. As discussed, the @@ has to be introduced to differentiate from the scenario where the 1st argument is a hashtable.

The scenario that this PR enables is:

Get-ChildItem @@{Path='C:\'; Filter=$filter}

This PR adds the new At TokenKind in order to be able the parse the syntax and adds a public Splatted property to HashTableAst so that the compiler knows at runtime that the argument is splatted.

The experimental feature PSGeneralizedSplatting is created for it. Although there are switch statements using the new At that are not guarded by an if check for this experimental feature (which was not possible due to the flow analysis of C# resulting in a compiler error), this is OK because the experimental feature flag prevents the tokenizer from creating an At token in the first place.

A semantic check was added so that the only implemented use case is using @@{ as an argument to a command, therefore the user will get an actionable error when trying to use it in another context, e.g. for a .Net method or a hashtable as @@{'key='value'} to conform with previous behaviour (where the tokenizer returned an error in those cases). I've also checked it does not impact using @@ inside a function name (even before a function name could not start with @@ so no change here).

This PR does not add tab completion functionality to the keys of the splatted hashtable. This is something that is not present at the moment anyway with splatted variables and is a separate enhancement that one could add in another PR, as well as other features in the RFC. The goal of this RFC is to implement just 1 feature in this RFC that adds value, therefore making it a minimum viable feature.

I added tests for existing and new splatting functionality and error cases.

PR Context

https://github.com/PowerShell/PowerShell-RFC/blob/master/2-Draft-Accepted/RFC0002-Generalized-Splatting.md

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: PSGeneralizedSplatting
• 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

[TylerLeonhardt]

This should probably be marked as an experimental feature.

[bergmeister]

• I fear that if it is an experimental feature, it will not get much test coverage or usage. Usage will be limited to people can write scripts that run on v7 only anyway.
• Editor syntax highlighting and code analysis in VS-Code will be cumbersome because one would need to enable the experimental feature just for viewing the code (which might be the code from another repo). Figuring out the cause of this, will not be clear to most people (the parser gives a UnrecognizedToken error : Unrecognized token in source text.).
• We still have a few previews to go until PS 7 goes RTM and the code change is a very subtle addition without explicitly altering current behaviour, therefore I see it as low risk anyway.

[KirkMunro]

I'd also like it marked as experimental. Experimental isn't just about risk. It's about acceptance of syntax, workflow, etc., and most importantly, it lets users know that the design of this feature is not yet complete.

As expressed here I think @@ and @$ are really bad additions to the PowerShell syntax, and proposed what I think are better alternatives to each of those.

[KirkMunro]

Also, experimental does not mean it's not going to make it into v7 either. It simply means we're experimenting with some syntax/commands/etc. and want to be able to allow the broader community kick the tires on those things before we sign-off on them as supported.

[SteveL-MSFT]

@KirkMunro is correct that Experimental means that the design can change and would not be a breaking change.

[bergmeister]

Ok, we have 3 changes here:

• Adding the new entry to the TokenKind enum and the new HashTableAst property: as far I understand the ExperimentalFeature implementation does not allow to exclude those additions, for the property the best I can do is return false if the feature flag is off
• Usage of the added TokenKind in the Parser and Tokenizer. What should the parser do if the feature is not enabled? Should they throw explicitly if usage of the new TokenKind is detected or just be a no-op?
• Compiler (runtime): Usage of ExperimentalFeature is clear.

Happy to add the experimental feature fences around those section after the first review approves the current format.

[TylerLeonhardt]

@daxian-dbw can probably supply advice here on proper Experimental Feature usage.

[rjmholt]

There are some nice first steps here I think but also some fixes required.

I'm personally skeptical about implementing half a language feature (since there is always a risk that the other half never makes it in and partial grammar changes often break the full scenario from being completed), although looking at the splatting RFC it seems that it does break nicely into two things.

Another thing that should come in as part of this change is completions on the splatted hashtable itself, since completions are such a core part of the PowerShell experience.

The testing probably needs to be built out significantly to cover things like:

• If hashtables are tokenised differently, it will need to cover @@ used in other places
• Failure semantics if the splatted hashtable expression produces a runtime error
• Errors produced if a spatted hashtable is used not as a command argument
• What happens if you try splatting a hashtable to a native command
• Likely a number of other cases I haven't thought about

[rjmholt]

Is this actually a new keyword? If @@splat is not expected to appear as a literal token in PowerShell, I don't think this should be here.

[lzybkr]

I was thinking of @ as a unary operator.

Thinking that way, one could write @ @{...} or @@{...} to mean the same thing, though the later would likely be preferred.

Given that, I wouldn't expect new tokens of the form @@( or @$ or @@{.

[bergmeister]

I would've put it into the Punctuators section, similar to AtCurly but because it's range for the TokenKind enum is already full, I just decided to put it at the end. Shall I put it into the operators section then or put it into the Punctuators section but keep the current enum value? I've later changed the token to be just @@ only.

[rjmholt]

Rather than creating this as a new public property on HashtableAst, I think it should be on ExpressionAst, since splatting is intended to be generalised.

Also, AST types in PowerShell are almost always immutable, so there shouldn't be a setter ideally, unless it must be changed after the AST is constructed (this is usually the consequence of contextual information required by a grammar feature that PowerShell's LL(k) recursive descent parser can't deal with naturally)

[bergmeister]

Since HashtableAst inherits from ExpressionAst, we can always change that later without a breaking change. I think we should only increase the areas where it is exposed on a need by need basis unless you can think of a case where the change in this PR could make it useful to have the property on ExpressionAst as well.

[rjmholt]

FYI SkipToken() serves this purpose

[bergmeister]

I found that it did not work when using SkipToken as it crashes some other logic in PowerShell just when typing @{{

[rjmholt]

Here where we've seen AtAtCurly and we now skip a token, there's a problem because @@ might be followed by other tokens and we don't check. This causes, for example:

My recommendation is to restructure the tokens, which I've discussed in a comment in the tokeniser.

But also there are two important things to consider:

• An @@{ token should only be valid as an argument to a command. This should be a semantic check.
• Adding a new token, error checking and recovery is critical, since otherwise the grammar may have new pitfalls in it. The easiest solution here is to make @@{ its own whole token.

[bergmeister]

OK, I've changed it to perform a look-ahead so that @{{ is correctly parsed without ambiguity.

[bergmeister]

@rjmholt @lzybkr I've reacted to all the comments and implemented changes: I've changed it to perform a look-ahead so that it correctly recognises @{{ only as AtAtCurly.
I am aware that implementing the experimental feature and more complex tests are still outstanding but would like to get feedback or sign off on the current design implementation before doing that at the end of this PR. In terms of future timeline I'd like to keep this PR as minimal as possible. For example, at the moment people already use splatting despite that lack of tab completion or being able to inline it. The aim of this PR is to add only the in-lining capability to get this into PS 7, which is the most important, minimum viable aspect IMHO. Any hints on how to implement tab completion later would be appreciated though and I could potentially look at doing them later.

[bergmeister]

It should be said that both PRs do not implement all the features in the RFC, just a subset of it that are the most important ones for me and the other author, which I think is OK in terms of being minimum, viable.
Also: This PR follows the @@ syntax that was agreed in the RFC exactly, the other PR proposes a different approach via common parameters.

I'd be happy as long as either of the 2 PRs get accepted. When deciding between the 2 PRs, I'd like the @PowerShell/powershell-committee to also consider what is best in terms of follow up PRs if other people contribute e.g. the remaining features in the RFC or tab completion for the inline-splatted arguments.

[joeyaiello]

We in the @PowerShell/powershell-committee are going to holistically review this vs. #10984 next week in our RFC meeting. This one seems to match the RFC we approved (at least skimming through), while @KirkMunro's does not, but we want to also consider the -splat parameter as a possibility.

That being said, we did review the original RFC0002 and its feedback very extensively before accepting.

[codykonior]

I like -splat, only because @@{ feels so visually jarring and so much like a typo.

[vexx32]

Personally, I don't like either very much. -Splat feels out of place / could use a better name, perhaps. @@{ ... } as you say, feels more like a typo than useful syntax, though its brevity is nice.

I don't have a compelling alternative, but I wish I did.

[johlju]

I prefer @@ since that would be more intuitive how splatting was done prior. Since there is an RFC that was approved for using @@ then a PR that closest match that RFC should be merged. If there is anything more to be gained by instead use -splat that would need to be explained in another RFC. IMHO, until then the -splat PR can be closed.

It would be very strange to merge a PR that contradict an approved RFC without having another RFC to back it. 🤔

[codykonior]

People on Twitter literally requested that we weigh in on what we’d prefer, so that’s what we’ve done.

[rkeithhill]

I prefer @@{...} as well. I'd loathe seeing more ubiquitous parameters added. There are already plenty I have to wade through in completion lists. I think there should be a very high bar for any new ubiquitous parameters. To me @@{...} makes sense because I use the splat operator @ to splat a hashtable.

[vexx32]

The one real criticism I have for @@{ ... } as a syntax is that it is likely to be prone to typos, and if you happen to be using hashtables as actual parameters the difference between @{ ... } and @@{ ... } is a very easy mistake to make.

[ChrisWarwick]

I like -splat, only because @@{ feels so visually jarring and so much like a typo.

I dislike @@{ for the same reason; this is starting to look very Perl-like; very mysterious and very un-PowerShell. My humble opinion is that -splat is the better way.

(Throwing in my knee-jerk reaction after seeing this brought up on Twitter)

[felixfbecker]

@@{ } also is very intuitive to me. @ is already the splat operator, we're just making it also accept a hastable expression @{ }.

An extra parameter, that magically affects other parameters is a lot harder to wrap my head around. It is also less clear to me in terms of precedence - i.e. what if you provide some parameters but also the -Splat parameter in different order. Currently the order of named parameters in PowerShell doesn't matter and that is a great invariant. It wouldn't be clear to me anymore whether that is the case or not with a -Splat parameter.

I see the point of saying "explicit, spelled out is better than a symbol" but @ is already the splat operator. Consistency in language design is important. And the meaning of the word "splat" really is not obvious to newcomers either.

[ChrisWarwick]

Currently the order of named parameters in PowerShell doesn't matter and that is a great invariant

A very good point, and something my knee-jerk reaction hadn't considered.

I still shudder at the proposed syntax. I get the point the you and Keith make about @ being the splatting operator already (so splatting a hashtable follows). I also imagine that the actual real-world use may mean that beginners aren't heavily impacted. I guess I'm just subliminally recalling those occasions when I opened a Perl script that I'd written and wondered if I'd been stoned at the time. It's the aesthetics rather than the basic lexicography.

[vexx32]

@jamesBru mentioned briefly during the weekly PowerLunch community chat a possibility that I like a little more; Command-Name -@ @{ ... } (he can correct me if I misunderstood his idea in any way ^^)

It's a nice medium between this implementation and -splat, doesn't occlude an otherwise potentially valid parameter name, and keeps most of the look of this implementation. It may be a little more complicated to work into parsing, however, since it would need to branch off from the usual command parameter parsing (-@ is not currently parsed as a valid parameter, but is considered a valid positional argument currently.)

[oising]

I prefer @@{ ... } -- it's more intuitive to me.

[rjmholt]

I have to admit that -@ has the elegance of only applying to commands, so we don't have to think about what @@{ ... } means when not being passed to a command as a parameter.

However, I think my personal preference is for a splat sigil or operator:

• Ubiquitous parameters that do special strange things conflate syntax and parameter value passing too much for my liking
• We already have @var, which doesn't square well with -@; the different sigil implies to me that the shape of the peg has changed, whereas the -@ implies that the shape of the hole has. My mental schema of splatting is now inconsistent juggling both at once
• We also support multiple splatting today (example below). This makes it harder for -@, since currently PowerShell does not allow duplicate parameter names.

> $x = 1,2,3
> $y = @{ d = 'a'; e = 'b' }
> $z = @{ f = 'Duck'; g = 'Banana' }
> function Test($a, $b, $c, $d, $e, $f, $g) { $a; $b; $c; $d; $e; $f; $g }
> Test @x @y @z
1
2
3
a
b
Duck
Banana

[vexx32]

@rjmholt -@ is actually never parsed as a parameter name currently, so it would need changes to parsing as it is, there shouldn't be any additional issues compared to implementing @@{...}. It parses as a positional value only at the moment. If anything, it can easily take a shortcut out of the parameter parsing mode since -@ is already an invalid parameter name. 🙂

In my opinion -@ would be nicer & potentially clearer, but I understand everyone has their own preferences. ^^

[rjmholt]

-@ is actually never parsed as a parameter name currently

Yeah I get that, saw above you said it currently parses as a bareword literal. But my concerns are both more and less technical than that:

• Changing parsing would need to be done either in the parser (probably the wrong place) or the parameter binder (more likely), and that would be difficult to implement correctly.
• More importantly, irrespective of how the implementation treats it, the language is a human/computer interaction interface, and the question in my mind is how users who are used to PowerShell will parse it. It looks like any other parameter, just with a strange name. But to what extent will it obey the existing parameter conventions? I think the fact we would need a very different code path to deal with it is a warning sign that it would probably open up a number of cases where what users expect and what it does differ.

[KirkMunro]

We also support multiple splatting today (example below). This makes it harder for -@, since currently PowerShell does not allow duplicate parameter names.

@rjmholt That wouldn't be an issue if -@ accepted an array, like the -splat PR already does (IIRC -- it's been a while).

[KirkMunro]

Also this discussion goes a little further than simply the syntactical differences between @@{...} and -splat @{}.

The latter also supports splatting properties or method results, such as:

function Do-This {
    [CmdletBinding()]
    param(
        [string]$Value
    )
    $sb = {
        # $PSBoundParameters is empty in this child scope, but $PSCmdlet.MyInvocation.BoundParameters is not
        Invoke-Something -splat $PSCmdlet.MyInvocation.BoundParameters
    }
    & $sb
}

In general, the -splat syntax supports more options, and if we accept -@ as an alias, replace -splat with -@, or just leave it as -splat, I feel we're making something that is much better for the non-developer and less experienced population of PowerShell scripters.

Also related, for splatting properties or method results, there is a separate PR (#11003) that supports doing that with the normal splat operator that we all use now (e.g. Invoke-Something @PSCmdlet.MyInvocation.BoundParameters). It's been so long I forgot that I had that other PR open independent of the -splat PR. Just calling that out to raise some awareness for anyone reading this PR discussion about the options that are available.

[bergmeister]

@PoshChan please retry macos

[PoshChan]

@bergmeister, did not find matching build context: PowerShell-CI-macOS; allowed contexts: PowerShell-CI-SSH

[SteveL-MSFT]

@PowerShell/powershell-committee discussed this in context of RFC0002 where we decided that we would withdraw RFC0002 with the exception of fixing the issue to allow explicit parameters to override a splatted hashtable and discuss named parameters for .NET method invocation as a separate issue. So in light of that, we will be closing this PR as discussed with @bergmeister

[SeidChr]

no -splat, no -@, no @@ yet, will there be anything like that in the future? It seems like all approaches to improve splatting and thus usability are declined. Is there anything else in the work?