GeezerVison

I believe we should check $result.Length before $result.GetType() in all test here. This is not important in these tests, but it is a bad pattern for other tests in which the $result are calculated. (I mean that somebody can copy-paste the bad pattern).

What do you mean by the $result are calculated? Can you please elaborate a bit and give an example?

From Get-Content.Tests.ps1:

  It "should Get-Content with a variety of -Tail and -ReadCount values" {#[DRT]
    set-content -path $testPath "Hello,World","Hello2,World2","Hello3,World3","Hello4,World4"
    $result=get-content -path $testPath -readcount:-1 -tail 5
    $result.Length | Should Be 4

Can we use Should BeOfType?

To use Should BeOfType I need to wrap $result in a new array because it will be unraveled otherwise. I feel it's clearer to just use GetType().FullName in this case.

As for the bad pattern, I'm still confused. Why do you think putting $result.Length | Should Be 3 after the type check would be a bad pattern? More technically speaking, the length check should happen after we know $result is an array, otherwise we might (actually impossible) end up checking the Length property of some random type :)

Ah, I see your point. A Should Not BeNullOrEmpty check should be done in that case.

Should we test complex types? PSCustomObject?

For any array type other than [object[]], it will fall in the code path of PSToObjectArrayBinder. I just use [int[]] as a simple validation.

Sorry, I mean member types.

Oh, the member type doesn't matter for the scenario of this fix. It's OK as long as the conversion works.

The goal of the optimization was to cover @(1,2) - so maybe the original test was too general. This fix ends up adding unnecessary overhead to the optimization by cloning and adding an if test.

I think a better fix is to just constrain the optimization more - basically if (subExpr is ArrayLiteralAst).

I don't recall why I added the List<object> case.

The clone was added to address a comment raised by PetSerAl about $a = 1,2,3; @([object[]]$a) doesn't return a new array. But if we only consider if (pureExpr is ArrayLiteralAst) then the clone and the null check are both not necessary anymore.

The List<object> case seems like an optimization for CaptureAstResults, because a List<object> instance will be created to hold the objects written to pipe in CaptureAstResults.
Are you suggesting to not cover the cases of values.Type == typeof(object[]) and values.Type == typeof(List<object>) here?

Replace the cases of values.Type.IsArray and values.Type == typeof(List<object>) with if (pureExprAst is ArrayLiteralAst). So the former cases will be handled by the dynamic site.