From e987ef18adf284402665d691dce612cdc7896285 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 19 Jul 2017 11:42:10 -0700 Subject: [PATCH 1/8] Fix array expression to not return null or throw error --- .../engine/parser/Compiler.cs | 30 +++++++++++----- .../Language/Scripting/Array.Tests.ps1 | 35 +++++++++++++++++++ 2 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 test/powershell/Language/Scripting/Array.Tests.ps1 diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 9972e7ff1f8..584c014f3bc 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -37,6 +37,8 @@ internal static class CachedReflectionInfo typeof(List).GetConstructor(PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ObjectList_ToArray = typeof(List).GetMethod(nameof(List.ToArray), PSTypeExtensions.EmptyTypes); + internal static readonly MethodInfo ObjectArray_Clone = + typeof(object[]).GetMethod(nameof(Array.Clone), PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ArrayOps_GetMDArrayValue = typeof(ArrayOps).GetMethod(nameof(ArrayOps.GetMDArrayValue), staticFlags); @@ -5500,16 +5502,28 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) } values = values ?? CaptureAstResults(subExpr, CaptureAstContext.Enumerable); - if (values.Type.IsArray) + if (values.Type == typeof(object[]) || values.Type == typeof(List)) { - // If the result is already an array, don't wrap the array. - return values; - } - if (values.Type == typeof(List)) - { - return Expression.Call(values, CachedReflectionInfo.ObjectList_ToArray); + Expression toArrayExpr = null; + if (values.Type == typeof(object[])) + { + toArrayExpr = Expression.Call(values, CachedReflectionInfo.ObjectArray_Clone).Cast(typeof(object[])); + } + else + { + toArrayExpr = Expression.Call(values, CachedReflectionInfo.ObjectList_ToArray); + } + + var temp = Expression.Variable(typeof(object[])); + var expr = Expression.Block( + new[] { temp }, + Expression.IfThenElse( + Expression.Equal(values, ExpressionCache.NullConstant), + Expression.Assign(temp, Expression.NewArrayInit(typeof(object), ExpressionCache.NullConstant)), + Expression.Assign(temp, toArrayExpr)), + temp); } - if (values.Type.GetTypeInfo().IsPrimitive || values.Type == typeof(string)) + if (values.Type.IsPrimitive || values.Type == typeof(string)) { // Slight optimization - no need for a dynamic site. We could special case other // types as well, but it's probably not worth it. diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 new file mode 100644 index 00000000000..086f77f38a5 --- /dev/null +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -0,0 +1,35 @@ +Describe "ArrayExpression Tests" { + It "@([int[]](1,2,3)) should return an array of object[]" { + $result = @([int[]](1,2,3)) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 3 + } + + It "@([object[]]`$null) should return an array of object[]" { + $result = @([object[]]$null) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 1 + $result[0] | Should Be $null + } + + It "@([int[]]`$null) should return an array of object[]" { + $result = @([int[]]$null) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 1 + $result[0] | Should Be $null + } + + It "@([object[]][System.Management.Automation.Internal.AutomationNull]::Value) should return an array of object[]" { + $result = @([object[]][System.Management.Automation.Internal.AutomationNull]::Value) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 1 + $result[0] | Should Be $null + } + + It "@([int[]][System.Management.Automation.Internal.AutomationNull]::Value) should return an array of object[]" { + $result = @([int[]][System.Management.Automation.Internal.AutomationNull]::Value) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 1 + $result[0] | Should Be $null + } +} \ No newline at end of file From be0a266e4d01ac88b533621f78e4cd239c0783a4 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 19 Jul 2017 11:59:21 -0700 Subject: [PATCH 2/8] Update tests --- test/powershell/Language/Scripting/Array.Tests.ps1 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index 086f77f38a5..f5f62decbc6 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -1,4 +1,10 @@ Describe "ArrayExpression Tests" { + It "@([object[]](1,2,3)) should return an array of object[]" { + $result = @([object[]](1,2,3)) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Count | Should Be 3 + } + It "@([int[]](1,2,3)) should return an array of object[]" { $result = @([int[]](1,2,3)) $result.GetType().FullName | Should Be "System.Object[]" @@ -32,4 +38,4 @@ Describe "ArrayExpression Tests" { $result.Count | Should Be 1 $result[0] | Should Be $null } -} \ No newline at end of file +} From 50b55cdf2b170dfc808d1179c362ebbace682f0a Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 19 Jul 2017 12:01:51 -0700 Subject: [PATCH 3/8] Add '-Tags CI' --- test/powershell/Language/Scripting/Array.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index f5f62decbc6..a1f6257bf99 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -1,4 +1,4 @@ -Describe "ArrayExpression Tests" { +Describe "ArrayExpression Tests" -Tags "CI" { It "@([object[]](1,2,3)) should return an array of object[]" { $result = @([object[]](1,2,3)) $result.GetType().FullName | Should Be "System.Object[]" From bdbe68d9803e720272e9b38f4f74b2a5467f6396 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 19 Jul 2017 12:19:58 -0700 Subject: [PATCH 4/8] Update tests again --- .../Language/Scripting/Array.Tests.ps1 | 37 +++++++++++++------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index a1f6257bf99..d2ea6075742 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -1,41 +1,54 @@ Describe "ArrayExpression Tests" -Tags "CI" { - It "@([object[]](1,2,3)) should return an array of object[]" { + It "@([object[]](1,2,3)) should return a 3-element array of object[]" { $result = @([object[]](1,2,3)) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 3 + $result.Length | Should Be 3 } - It "@([int[]](1,2,3)) should return an array of object[]" { + It "@([int[]](1,2,3)) should return a 3-element array of object[]" { $result = @([int[]](1,2,3)) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 3 + $result.Length | Should Be 3 } - It "@([object[]]`$null) should return an array of object[]" { + It "@([object[]]`$null) should return a 1-element(`$null) array of object[]" { $result = @([object[]]$null) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 1 + $result.Length | Should Be 1 $result[0] | Should Be $null } - It "@([int[]]`$null) should return an array of object[]" { + It "@([int[]]`$null) should return a 1-element(`$null) array of object[]" { $result = @([int[]]$null) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 1 + $result.Length | Should Be 1 $result[0] | Should Be $null } - It "@([object[]][System.Management.Automation.Internal.AutomationNull]::Value) should return an array of object[]" { + It "@([object[]][System.Management.Automation.Internal.AutomationNull]::Value) should return a 1-element(`$null) array of object[]" { $result = @([object[]][System.Management.Automation.Internal.AutomationNull]::Value) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 1 + $result.Length | Should Be 1 $result[0] | Should Be $null } - It "@([int[]][System.Management.Automation.Internal.AutomationNull]::Value) should return an array of object[]" { + It "@([int[]][System.Management.Automation.Internal.AutomationNull]::Value) should return a 1-element(`$null) array of object[]" { $result = @([int[]][System.Management.Automation.Internal.AutomationNull]::Value) $result.GetType().FullName | Should Be "System.Object[]" - $result.Count | Should Be 1 + $result.Length | Should Be 1 $result[0] | Should Be $null } + + It "@(`$null) should return a 1-element(`$null) array of object[]" { + $result = @($null) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Length | Should Be 1 + $result[0] | Should Be $null + } + + It "@([System.Management.Automation.Internal.AutomationNull]::Value) should return an empty array of object[]" { + $result = @([System.Management.Automation.Internal.AutomationNull]::Value) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Length | Should Be 0 + } } From 33b5461569584e13135a50fb58aa9aaaf22c0584 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 24 Jul 2017 15:56:35 -0700 Subject: [PATCH 5/8] Address comment --- .../engine/parser/Compiler.cs | 32 +++++-------------- 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 584c014f3bc..390adadc84c 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -37,8 +37,6 @@ internal static class CachedReflectionInfo typeof(List).GetConstructor(PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ObjectList_ToArray = typeof(List).GetMethod(nameof(List.ToArray), PSTypeExtensions.EmptyTypes); - internal static readonly MethodInfo ObjectArray_Clone = - typeof(object[]).GetMethod(nameof(Array.Clone), PSTypeExtensions.EmptyTypes); internal static readonly MethodInfo ArrayOps_GetMDArrayValue = typeof(ArrayOps).GetMethod(nameof(ArrayOps.GetMDArrayValue), staticFlags); @@ -5479,6 +5477,8 @@ public object VisitInvokeMemberExpression(InvokeMemberExpressionAst invokeMember public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) { Expression values = null; + ExpressionAst pureExprAst = null; + var subExpr = arrayExpressionAst.SubExpression; if (subExpr.Traps == null) { @@ -5487,10 +5487,10 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) var pipelineBase = subExpr.Statements[0] as PipelineBaseAst; if (pipelineBase != null) { - var exprAst = pipelineBase.GetPureExpression(); - if (exprAst != null) + pureExprAst = pipelineBase.GetPureExpression(); + if (pureExprAst != null) { - values = Compile(exprAst); + values = Compile(pureExprAst); } } } @@ -5502,26 +5502,10 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) } values = values ?? CaptureAstResults(subExpr, CaptureAstContext.Enumerable); - if (values.Type == typeof(object[]) || values.Type == typeof(List)) + if (pureExprAst is ArrayLiteralAst) { - Expression toArrayExpr = null; - if (values.Type == typeof(object[])) - { - toArrayExpr = Expression.Call(values, CachedReflectionInfo.ObjectArray_Clone).Cast(typeof(object[])); - } - else - { - toArrayExpr = Expression.Call(values, CachedReflectionInfo.ObjectList_ToArray); - } - - var temp = Expression.Variable(typeof(object[])); - var expr = Expression.Block( - new[] { temp }, - Expression.IfThenElse( - Expression.Equal(values, ExpressionCache.NullConstant), - Expression.Assign(temp, Expression.NewArrayInit(typeof(object), ExpressionCache.NullConstant)), - Expression.Assign(temp, toArrayExpr)), - temp); + // If the pure expression is ArrayLiteralAst, just return the result. + return values; } if (values.Type.IsPrimitive || values.Type == typeof(string)) { From 88a68ab6347b53a9b45872c8e225da69225a0af7 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 24 Jul 2017 16:13:53 -0700 Subject: [PATCH 6/8] Add 2 more tests --- .../Language/Scripting/Array.Tests.ps1 | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index d2ea6075742..4d6189a80f3 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -51,4 +51,20 @@ Describe "ArrayExpression Tests" -Tags "CI" { $result.GetType().FullName | Should Be "System.Object[]" $result.Length | Should Be 0 } + + It "@([object[]]`$a) should return a new array" { + $a = 1,2,3 + $result = @([object[]]$a) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Length | Should Be 3 + [object]::ReferenceEquals($a, $result) | Should Be $false + } + + It "@([int[]]`$a) should return a new array" { + $a = 1,2,3 + $result = @([int[]]$a) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Length | Should Be 3 + [object]::ReferenceEquals($a, $result) | Should Be $false + } } From 237c2bb037b5c29a7f5628eabc6179bc3deefc52 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 24 Jul 2017 20:14:37 -0700 Subject: [PATCH 7/8] Change 2 tests --- test/powershell/Language/Scripting/Array.Tests.ps1 | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index 4d6189a80f3..ceed3001152 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -57,7 +57,6 @@ Describe "ArrayExpression Tests" -Tags "CI" { $result = @([object[]]$a) $result.GetType().FullName | Should Be "System.Object[]" $result.Length | Should Be 3 - [object]::ReferenceEquals($a, $result) | Should Be $false } It "@([int[]]`$a) should return a new array" { @@ -65,6 +64,5 @@ Describe "ArrayExpression Tests" -Tags "CI" { $result = @([int[]]$a) $result.GetType().FullName | Should Be "System.Object[]" $result.Length | Should Be 3 - [object]::ReferenceEquals($a, $result) | Should Be $false } } From ce4d2d9306b242136cf5cea980e5c986dce8919f Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 24 Jul 2017 21:27:47 -0700 Subject: [PATCH 8/8] Add a test about @([System.Collections.Generic.List[object]]$null) --- test/powershell/Language/Scripting/Array.Tests.ps1 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index ceed3001152..c66ce5fc216 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -65,4 +65,11 @@ Describe "ArrayExpression Tests" -Tags "CI" { $result.GetType().FullName | Should Be "System.Object[]" $result.Length | Should Be 3 } + + It "@([System.Collections.Generic.List[object]]`$null) should return a 1-element(`$null) array of object[]" { + $result = @([System.Collections.Generic.List[object]]$null) + $result.GetType().FullName | Should Be "System.Object[]" + $result.Length | Should Be 1 + $result[0] | Should Be $null + } }