From 8662d66f7bb8cb20c54feb9273158b8aeae77c9e Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Fri, 29 Dec 2017 23:36:19 -0800 Subject: [PATCH 1/2] [Feature] Make minor fixes in Compiler to properly handly void type expression --- .../engine/parser/Compiler.cs | 17 ++++++++-- .../engine/parser/ast.cs | 2 +- .../Language/Scripting/Array.Tests.ps1 | 32 +++++++++++++++++++ .../Scripting/Scripting.Followup.Tests.ps1 | 12 +++++++ 4 files changed, 59 insertions(+), 4 deletions(-) create mode 100644 test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 diff --git a/src/System.Management.Automation/engine/parser/Compiler.cs b/src/System.Management.Automation/engine/parser/Compiler.cs index 65b94436723..27db6741cc4 100644 --- a/src/System.Management.Automation/engine/parser/Compiler.cs +++ b/src/System.Management.Automation/engine/parser/Compiler.cs @@ -3039,6 +3039,12 @@ public object VisitPipeline(PipelineAst pipelineAst) input = GetRangeEnumerator(firstCommandExpr.Expression) ?? Compile(firstCommandExpr.Expression); } + + if (input.Type == typeof(void)) + { + input = Expression.Block(input, ExpressionCache.AutomationNullConstant); + } + i = 1; commandsInPipe = pipeElements.Count - 1; } @@ -5525,7 +5531,7 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) if (values.Type == typeof(void)) { // A dynamic site can't take void - but a void value is just an empty array. - return Expression.NewArrayInit(typeof(object)); + return Expression.Block(values, Expression.NewArrayInit(typeof(object))); } return DynamicExpression.Dynamic(PSToObjectArrayBinder.Get(), typeof(object[]), values); @@ -5533,8 +5539,13 @@ public object VisitArrayExpression(ArrayExpressionAst arrayExpressionAst) public object VisitArrayLiteral(ArrayLiteralAst arrayLiteralAst) { - return Expression.NewArrayInit(typeof(object), - arrayLiteralAst.Elements.Select(elem => Compile(elem).Cast(typeof(object)))); + List elementValues = new List(arrayLiteralAst.Elements.Count); + foreach (var element in arrayLiteralAst.Elements) + { + var eValue = Compile(element); + elementValues.Add(eValue.Type != typeof(void) ? eValue.Cast(typeof(object)) : Expression.Block(eValue, ExpressionCache.AutomationNullConstant)); + } + return Expression.NewArrayInit(typeof(object), elementValues); } private IEnumerable BuildHashtable(ReadOnlyCollection keyValuePairs, ParameterExpression temp, bool ordered) diff --git a/src/System.Management.Automation/engine/parser/ast.cs b/src/System.Management.Automation/engine/parser/ast.cs index 63dca76ca86..36f660c0fef 100644 --- a/src/System.Management.Automation/engine/parser/ast.cs +++ b/src/System.Management.Automation/engine/parser/ast.cs @@ -9301,7 +9301,7 @@ public ArrayLiteralAst(IScriptExtent extent, IList elements) } /// - /// The non-empty collection of asts of the elements of the array, or null if no elements were specified (e.g. @()). + /// The non-empty collection of asts of the elements of the array. /// public ReadOnlyCollection Elements { get; private set; } diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index c66ce5fc216..7c90153fdce 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -72,4 +72,36 @@ Describe "ArrayExpression Tests" -Tags "CI" { $result.Length | Should Be 1 $result[0] | Should Be $null } + + It "@([void](New-Item)) should create file" { + try { + $testFile = "$TestDrive\test.txt" + $result = @([void](New-Item $testFile -ItemType File)) + ## file should be created + $testFile | Should Exist + ## the array should be empty + $result.Count | Should Be 0 + } finally { + Remove-Item $testFile -Force -ErrorAction SilentlyContinue + } + } +} + +Describe "ArrayLiteral Tests" -Tags "CI" { + It "'[void](New-Item),2,3' should return a 3-element array and first element is AutomationNull" { + try { + $testFile = "$TestDrive\test.txt" + $result = [void](New-Item $testFile -ItemType File), 2, 3 + ## file should be created + $testFile | Should Exist + ## the array should contain 3 items + $result.Count | Should Be 3 + + ## the first item should be AutomationNull + $result[0] | ForEach-Object { "YES" } | Should Be $null + $result | Measure-Object | ForEach-Object -MemberName Count | Should Be 2 + } finally{ + Remove-Item $testFile -Force -ErrorAction SilentlyContinue + } + } } diff --git a/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 b/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 new file mode 100644 index 00000000000..536f23e1035 --- /dev/null +++ b/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 @@ -0,0 +1,12 @@ +Describe "Scripting.Followup.Tests" -Tags "CI" { + It "'[void](New-Item) | ' should work and behave like passing AutomationNull to the pipe" { + try { + $testFile = "$TestDrive\test.txt" + [void](New-Item $testFile -ItemType File) | ForEach-Object { "YES" } | Should Be $null + ## file should be created + $testFile | Should Exist + } finally { + Remove-Item $testFile -Force -ErrorAction SilentlyContinue + } + } +} From 462df49b499305a0c2f730aa34b765916195055d Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 1 Jan 2018 15:12:24 -0800 Subject: [PATCH 2/2] Address comments --- .../Language/Scripting/Array.Tests.ps1 | 39 ++++++++++++++++++- .../Scripting/Scripting.Followup.Tests.ps1 | 20 +++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/test/powershell/Language/Scripting/Array.Tests.ps1 b/test/powershell/Language/Scripting/Array.Tests.ps1 index 7c90153fdce..7006cec9303 100644 --- a/test/powershell/Language/Scripting/Array.Tests.ps1 +++ b/test/powershell/Language/Scripting/Array.Tests.ps1 @@ -75,7 +75,7 @@ Describe "ArrayExpression Tests" -Tags "CI" { It "@([void](New-Item)) should create file" { try { - $testFile = "$TestDrive\test.txt" + $testFile = Join-Path $TestDrive (New-Guid) $result = @([void](New-Item $testFile -ItemType File)) ## file should be created $testFile | Should Exist @@ -90,7 +90,7 @@ Describe "ArrayExpression Tests" -Tags "CI" { Describe "ArrayLiteral Tests" -Tags "CI" { It "'[void](New-Item),2,3' should return a 3-element array and first element is AutomationNull" { try { - $testFile = "$TestDrive\test.txt" + $testFile = Join-Path $TestDrive (New-Guid) $result = [void](New-Item $testFile -ItemType File), 2, 3 ## file should be created $testFile | Should Exist @@ -104,4 +104,39 @@ Describe "ArrayLiteral Tests" -Tags "CI" { Remove-Item $testFile -Force -ErrorAction SilentlyContinue } } + + It "'[void]1, [void](New-Item), [void]2' should return a 3-AutomationNull-element array" { + try { + $testFile = Join-Path $TestDrive (New-Guid) + $result = [void]1, [void](New-Item $testFile -ItemType File), [void]2 + ## file should be created + $testFile | Should Exist + ## the array should contain 3 items + $result.Count | Should Be 3 + + ## all items should be AutomationNull + $result | ForEach-Object { "YES" } | Should Be $null + } finally { + Remove-Item $testFile -Force -ErrorAction SilentlyContinue + } + } + + It "'[void]`$arraylist1.Add(1), `$arraylist2.Clear()' should return a 2-AutomationNull-element array" { + $arraylist1 = [System.Collections.ArrayList]::new() + $arraylist2 = [System.Collections.ArrayList]::new() + + $arraylist2.Add(2) > $null + $arraylist2.Count | Should Be 1 + + ## first item is a non-void method call + ## second item is a void method call + $result = [void]$arraylist1.Add(1), $arraylist2.Clear() + $result.Count | Should Be 2 + $result | ForEach-Object { "YES" } | Should Be $null + + $arraylist1.Count | Should Be 1 + $arraylist1[0] | Should Be 1 + + $arraylist2.Count | Should Be 0 + } } diff --git a/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 b/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 index 536f23e1035..0b850626269 100644 --- a/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 +++ b/test/powershell/Language/Scripting/Scripting.Followup.Tests.ps1 @@ -1,7 +1,7 @@ Describe "Scripting.Followup.Tests" -Tags "CI" { It "'[void](New-Item) | ' should work and behave like passing AutomationNull to the pipe" { try { - $testFile = "$TestDrive\test.txt" + $testFile = Join-Path $TestDrive (New-Guid) [void](New-Item $testFile -ItemType File) | ForEach-Object { "YES" } | Should Be $null ## file should be created $testFile | Should Exist @@ -9,4 +9,22 @@ Describe "Scripting.Followup.Tests" -Tags "CI" { Remove-Item $testFile -Force -ErrorAction SilentlyContinue } } + + ## cast non-void method call to [void] + It "'[void]`$arraylist.Add(1) | ' should work and behave like passing AutomationNull to the pipe" { + $arraylist = [System.Collections.ArrayList]::new() + [void]$arraylist.Add(1) | ForEach-Object { "YES" } | Should Be $null + ## $arraylist.Add(1) should be executed + $arraylist.Count | Should Be 1 + $arraylist[0] | Should Be 1 + } + + ## void method call + It "'`$arraylist2.Clear() | ' should work and behave like passing AutomationNull to the pipe" { + $arraylist = [System.Collections.ArrayList]::new() + $arraylist.Add(1) > $null + $arraylist.Clear() | ForEach-Object { "YES" } | Should Be $null + ## $arraylist.Clear() should be executed + $arraylist.Count | Should Be 0 + } }