Infer variables assigned in ArrayLiterals and ParenExpressions #25317
Conversation
| $TestVar1, $TestVar2 = "s1", "s2", "s3" | ||
| $TestVar1 | ||
| $TestVar2 | ||
| }.Ast.FindAll({param($ast) $ast -is [Language.VariableExpressionAst]}, $true) | Select-Object -Last 2 |
There was a problem hiding this comment.
I played with:
$Asts = {
$TestVar1, $TestVar2, $TestVar3 = $(New-Guid), "s2", 1, 2, 3
$TestVar1
$TestVar2
$TestVar3
}.Ast.FindAll({param($ast) $ast -is [Language.VariableExpressionAst]}, $true) | Select-Object -Last 3and got System.Object, System.Object and System.Object[] but expected System.Guid, System.String and System.Int32[].
There was a problem hiding this comment.
That's working as expected. The RHS is evaluated as a whole, and because it's a mixed set of objects, we consider it an Object[] that is then deconstructed to Object for the first set of variables.
I don't think it's worth the effort to fix this. We could only feasibly fix this for constants like in your example, but if you are working with constants then there's no reason to declare them like this.
There was a problem hiding this comment.
These can be not only constants, but also everything that type inference works for.
It is a common in pwsh that the right side is a list of objects of the same type, but it is in the decomposition scenario that we rather expect these objects to be of different types. Decomposition has great features in pwsh. I think it deserves more support.
Since we iterate over variables in a loop, we can easily check the type at the appropriate position on the right side (if it supports indexing, of course) and even slice to get the rest.
There was a problem hiding this comment.
I have added it for array literals on the RHS. There is no practical way to do this without making a lot of changes to the rest of the inference code to handle dynamic expressions on the RHS, so this:
$Array = "string", 42
$String, $Number = $Array
Would infer the variables as object and object[] just like before.
There was a problem hiding this comment.
This raises the question of usefulness and expediency. The current approach is rather a workaround for a limited number of scenarios. In other cases, it provides useless information or even false information. Even if I accept your first commit, I'm afraid it will raise questions about why it doesn't always work. Perhaps this is the case when it is better to do nothing at all if it is impossible to provide deconstruction support with reasonable efforts.
There was a problem hiding this comment.
This is not a feature I use, but for the people that do use it, do you think it's common to mix types like that where it would cause issues? I'd imagine the most common use case would be something like: $FirstItem, $TheRest = Get-Something or maybe something like: $Index, $OtherNumbers = 1..10.
I don't imagine people tend to mix and match completely unrelated types in one array that they then split up with this syntax.
If people do want to mix and match, they can just add a type constraint and it will work as expected (thanks to the changes in this PR):
$Array = "string", 42
[string]$String, [int]$Number = $Array
There was a problem hiding this comment.
do you think it's common to mix types like that where it would cause issues?
I can't say for sure, but that was the first thing I thought of when looking at your new tests. It can also be used to pass anonymous parameters to a function, followed by deconstruction. Finally, it can be used in Lisp-style programming. I don't think all this is super popular, but it's possible just because of the weak support in the pwsh ecosystem.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
PR Summary
Similar fix as this one: #25303 this time it's for the type inference so that variables assigned in ArrayLiterals like:
$Var1, $Var2 = 1..10are inferred properly. And the same for variables inside ParenExpressions:($Var1) = 1..10.PR Context
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header