From 8e68c421028a85378adecdfaf6b778b67e33f35b Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Thu, 8 Aug 2019 17:42:13 -0300 Subject: [PATCH 1/5] mark -parallel as reserved for foreach and switch --- .../engine/parser/SemanticChecks.cs | 33 +++++++++++-------- .../resources/ParserStrings.resx | 4 +-- .../Scripting/ForeachParallel.Tests.ps1 | 25 +++++++------- .../Scripting/SwitchParallel.Tests.ps1 | 27 +++++++++++++++ 4 files changed, 62 insertions(+), 27 deletions(-) create mode 100644 test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 diff --git a/src/System.Management.Automation/engine/parser/SemanticChecks.cs b/src/System.Management.Automation/engine/parser/SemanticChecks.cs index 54c211e6043..b93701c9762 100644 --- a/src/System.Management.Automation/engine/parser/SemanticChecks.cs +++ b/src/System.Management.Automation/engine/parser/SemanticChecks.cs @@ -458,13 +458,11 @@ public override AstVisitAction VisitSwitchStatement(SwitchStatementAst switchSta // Parallel flag not allowed if ((switchStatementAst.Flags & SwitchFlags.Parallel) == SwitchFlags.Parallel) { - bool reportError = !switchStatementAst.IsInWorkflow(); - if (reportError) - { - _parser.ReportError(switchStatementAst.Extent, - nameof(ParserStrings.ParallelNotSupported), - ParserStrings.ParallelNotSupported); - } + _parser.ReportError(switchStatementAst.Extent, + nameof(ParserStrings.KeywordParameterReservedForFutureUse), + ParserStrings.KeywordParameterReservedForFutureUse, + "switch", + "parallel"); } return AstVisitAction.Continue; @@ -494,13 +492,20 @@ public override AstVisitAction VisitForEachStatement(ForEachStatementAst forEach // Parallel flag not allowed if ((forEachStatementAst.Flags & ForEachFlags.Parallel) == ForEachFlags.Parallel) { - bool reportError = !forEachStatementAst.IsInWorkflow(); - if (reportError) - { - _parser.ReportError(forEachStatementAst.Extent, - nameof(ParserStrings.ParallelNotSupported), - ParserStrings.ParallelNotSupported); - } + _parser.ReportError(forEachStatementAst.Extent, + nameof(ParserStrings.KeywordParameterReservedForFutureUse), + ParserStrings.KeywordParameterReservedForFutureUse, + "foreach", + "parallel"); + } + + if (forEachStatementAst.ThrottleLimit != null) + { + _parser.ReportError(forEachStatementAst.Extent, + nameof(ParserStrings.KeywordParameterReservedForFutureUse), + ParserStrings.KeywordParameterReservedForFutureUse, + "foreach", + "throttlelimit"); } // Throttle limit must be combined with Parallel flag diff --git a/src/System.Management.Automation/resources/ParserStrings.resx b/src/System.Management.Automation/resources/ParserStrings.resx index adaa86dbcb7..4644fa5aa3b 100644 --- a/src/System.Management.Automation/resources/ParserStrings.resx +++ b/src/System.Management.Automation/resources/ParserStrings.resx @@ -392,8 +392,8 @@ Possible matches are The switch statement was incomplete. - - The '-parallel' parameter can be used only within a workflow. + + The {0} '-{1}' parameter is reserved for future use. Cannot process the 'switch' statement because of a missing file name argument to the -file option. diff --git a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 index c0941a36269..93a37469b69 100644 --- a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 @@ -2,19 +2,12 @@ # Licensed under the MIT License. Describe "Parallel foreach syntax" -Tags "CI" { - Context 'Should be able to retrieve AST of parallel foreach, error in regular case' { - $errors = @() + Context 'Should be able to retrieve AST of parallel foreach' { $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'foreach -parallel ($foo in $bar) {}', [ref] $null, [ref] $errors) - It '$errors.Count' { $errors.Count | Should -Be 1 } + 'foreach -parallel ($foo in $bar) {}', [ref] $null, [ref] $null) It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } } - It 'Should be able to retrieve AST of parallel foreach, works in JobDefinition case' -Skip:$IsCoreCLR { - . .\TestsOnWinFullOnly.ps1 - Run-TestOnWinFull "ForeachParallel:ASTOfParallelForeachOnWorkflow" - } - Context 'Supports newlines before and after' { $errors = @() $ast = [System.Management.Automation.Language.Parser]::ParseInput( @@ -31,11 +24,21 @@ Describe "Parallel foreach syntax" -Tags "CI" { It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -BeExactly 'InvalidForeachFlag' } } - Context 'Generate an error on -parallel that is not a workflow' { + Context 'Generate an error on -parallel' { $errors = @() $ast = [System.Management.Automation.Language.Parser]::ParseInput( 'foreach -parallel ($input in $bar) { }', [ref]$null, [ref]$errors) It '$errors.Count' { $errors.Count | Should -Be 1 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -BeExactly 'ParallelNotSupported' } + It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } } + + Context 'Generate an error on -throttlelimit' { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'foreach -throttlelimit 2 ($input in $bar) { }', [ref]$null, [ref]$errors) + It '$errors.Count' { $errors.Count | Should -Be 2 } + It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } + It '$errors[1].ErrorId' { $errors[1].ErrorId | Should -Be 'ThrottleLimitRequiresParallelFlag' } + } + } \ No newline at end of file diff --git a/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 new file mode 100644 index 00000000000..a0eb2b5e1dd --- /dev/null +++ b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 @@ -0,0 +1,27 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. +Describe "Parallel switch syntax" -Tags "CI" { + + Context 'Should be able to retrieve AST of parallel switch' { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -parallel ($foo) {1 {break}}', [ref] $null, [ref] $null) + It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } + } + + Context 'Generates an error on invalid parameter' { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -bogus ($foo) {1 {break}}', [ref]$null, [ref]$errors) + It '$errors.Count' { $errors.Count | Should -Be 1 } + It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -BeExactly 'InvalidSwitchFlag' } + } + + Context 'Generate an error on -parallel' { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -parallel ($foo) {1 {break}}', [ref]$null, [ref]$errors) + It '$errors.Count' { $errors.Count | Should -Be 1 } + It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } + } + +} \ No newline at end of file From e277989ec1c22202148dd8c1bce36d0cc9d76a23 Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Thu, 8 Aug 2019 17:44:28 -0300 Subject: [PATCH 2/5] add trailing newline --- test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 | 4 ++-- test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 index 93a37469b69..7ccb70038d1 100644 --- a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 @@ -2,7 +2,7 @@ # Licensed under the MIT License. Describe "Parallel foreach syntax" -Tags "CI" { - Context 'Should be able to retrieve AST of parallel foreach' { + Context 'Should be able to retrieve AST of parallel foreach' { $ast = [System.Management.Automation.Language.Parser]::ParseInput( 'foreach -parallel ($foo in $bar) {}', [ref] $null, [ref] $null) It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } @@ -41,4 +41,4 @@ Describe "Parallel foreach syntax" -Tags "CI" { It '$errors[1].ErrorId' { $errors[1].ErrorId | Should -Be 'ThrottleLimitRequiresParallelFlag' } } -} \ No newline at end of file +} diff --git a/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 index a0eb2b5e1dd..f31eb0c93f5 100644 --- a/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 @@ -2,7 +2,7 @@ # Licensed under the MIT License. Describe "Parallel switch syntax" -Tags "CI" { - Context 'Should be able to retrieve AST of parallel switch' { + Context 'Should be able to retrieve AST of parallel switch' { $ast = [System.Management.Automation.Language.Parser]::ParseInput( 'switch -parallel ($foo) {1 {break}}', [ref] $null, [ref] $null) It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } @@ -24,4 +24,4 @@ Describe "Parallel switch syntax" -Tags "CI" { It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } } -} \ No newline at end of file +} From 663cc76a3afc9ff94cc4945dac5d292b11ca2e6f Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Thu, 8 Aug 2019 20:21:54 -0300 Subject: [PATCH 3/5] CodeFactor changes --- .../engine/parser/SemanticChecks.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/parser/SemanticChecks.cs b/src/System.Management.Automation/engine/parser/SemanticChecks.cs index b93701c9762..66a6e4f8054 100644 --- a/src/System.Management.Automation/engine/parser/SemanticChecks.cs +++ b/src/System.Management.Automation/engine/parser/SemanticChecks.cs @@ -458,7 +458,8 @@ public override AstVisitAction VisitSwitchStatement(SwitchStatementAst switchSta // Parallel flag not allowed if ((switchStatementAst.Flags & SwitchFlags.Parallel) == SwitchFlags.Parallel) { - _parser.ReportError(switchStatementAst.Extent, + _parser.ReportError( + switchStatementAst.Extent, nameof(ParserStrings.KeywordParameterReservedForFutureUse), ParserStrings.KeywordParameterReservedForFutureUse, "switch", @@ -492,7 +493,8 @@ public override AstVisitAction VisitForEachStatement(ForEachStatementAst forEach // Parallel flag not allowed if ((forEachStatementAst.Flags & ForEachFlags.Parallel) == ForEachFlags.Parallel) { - _parser.ReportError(forEachStatementAst.Extent, + _parser.ReportError( + forEachStatementAst.Extent, nameof(ParserStrings.KeywordParameterReservedForFutureUse), ParserStrings.KeywordParameterReservedForFutureUse, "foreach", @@ -501,7 +503,8 @@ public override AstVisitAction VisitForEachStatement(ForEachStatementAst forEach if (forEachStatementAst.ThrottleLimit != null) { - _parser.ReportError(forEachStatementAst.Extent, + _parser.ReportError( + forEachStatementAst.Extent, nameof(ParserStrings.KeywordParameterReservedForFutureUse), ParserStrings.KeywordParameterReservedForFutureUse, "foreach", @@ -512,7 +515,8 @@ public override AstVisitAction VisitForEachStatement(ForEachStatementAst forEach if ((forEachStatementAst.ThrottleLimit != null) && ((forEachStatementAst.Flags & ForEachFlags.Parallel) != ForEachFlags.Parallel)) { - _parser.ReportError(forEachStatementAst.Extent, + _parser.ReportError( + forEachStatementAst.Extent, nameof(ParserStrings.ThrottleLimitRequiresParallelFlag), ParserStrings.ThrottleLimitRequiresParallelFlag); } From 75774f1e9166598c1903d880c8cd9cb09e3bb743 Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Fri, 9 Aug 2019 10:45:39 -0300 Subject: [PATCH 4/5] clean up formatting of Pester tests --- .../Scripting/ForeachParallel.Tests.ps1 | 92 +++++++++++++------ .../Scripting/SwitchParallel.Tests.ps1 | 51 +++++++--- 2 files changed, 102 insertions(+), 41 deletions(-) diff --git a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 index 7ccb70038d1..b5e264a27eb 100644 --- a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 @@ -1,44 +1,84 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -Describe "Parallel foreach syntax" -Tags "CI" { + +Describe 'Parallel foreach syntax' -Tags 'CI' { Context 'Should be able to retrieve AST of parallel foreach' { - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'foreach -parallel ($foo in $bar) {}', [ref] $null, [ref] $null) - It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } + BeforeAll { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'foreach -parallel ($foo in $bar) {}', [ref]$null, [ref]$null) + } + + It '$ast.EndBlock.Statements[0].Flags' { + $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' + } } Context 'Supports newlines before and after' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - "foreach `n-parallel `n(`$foo in `$bar) {}", [ref] $null, [ref] $null) - It '$errors.Count' { $errors.Count | Should -Be 0 } - It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + "foreach `n-parallel `n(`$foo in `$bar) {}", [ref] $null, [ref] $null) + } + + It '$errors.Count' + $errors.Count | Should -Be 0 + } + + It '$ast.EndBlock.Statements[0].Flags' { + $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' + } } Context 'Generates an error on invalid parameter' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'foreach -bogus ($input in $bar) { }', [ref]$null, [ref]$errors) - It '$errors.Count' { $errors.Count | Should -Be 1 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -BeExactly 'InvalidForeachFlag' } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'foreach -bogus ($input in $bar) { }', [ref]$null, [ref]$errors) + } + + It '$errors.Count' { + $errors.Count | Should -Be 1 + } + + It '$errors[0].ErrorId' { + $errors[0].ErrorId | Should -BeExactly 'InvalidForeachFlag' + } } Context 'Generate an error on -parallel' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'foreach -parallel ($input in $bar) { }', [ref]$null, [ref]$errors) - It '$errors.Count' { $errors.Count | Should -Be 1 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'foreach -parallel ($input in $bar) { }', [ref]$null, [ref]$errors) + } + + It '$errors.Count' { + $errors.Count | Should -Be 1 + } + + It '$errors[0].ErrorId' { + $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' + } } Context 'Generate an error on -throttlelimit' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'foreach -throttlelimit 2 ($input in $bar) { }', [ref]$null, [ref]$errors) - It '$errors.Count' { $errors.Count | Should -Be 2 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } - It '$errors[1].ErrorId' { $errors[1].ErrorId | Should -Be 'ThrottleLimitRequiresParallelFlag' } - } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'foreach -throttlelimit 2 ($input in $bar) { }', [ref]$null, [ref]$errors) + } + It '$errors.Count' { + $errors.Count | Should -Be 2 + } + + It '$errors[0].ErrorId' { + $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' + } + + It '$errors[1].ErrorId' { + $errors[1].ErrorId | Should -Be 'ThrottleLimitRequiresParallelFlag' + } + } } diff --git a/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 index f31eb0c93f5..9af1cf5b8ed 100644 --- a/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/SwitchParallel.Tests.ps1 @@ -1,27 +1,48 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -Describe "Parallel switch syntax" -Tags "CI" { + +Describe 'Parallel switch syntax' -Tags 'CI' { Context 'Should be able to retrieve AST of parallel switch' { - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'switch -parallel ($foo) {1 {break}}', [ref] $null, [ref] $null) - It '$ast.EndBlock.Statements[0].Flags' { $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' } + BeforeAll { + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -parallel ($foo) {1 {break}}', [ref] $null, [ref] $null) + } + + It '$ast.EndBlock.Statements[0].Flags' { + $ast.EndBlock.Statements[0].Flags | Should -BeExactly 'Parallel' + } } Context 'Generates an error on invalid parameter' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'switch -bogus ($foo) {1 {break}}', [ref]$null, [ref]$errors) - It '$errors.Count' { $errors.Count | Should -Be 1 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -BeExactly 'InvalidSwitchFlag' } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -bogus ($foo) {1 {break}}', [ref]$null, [ref]$errors) + } + + It '$errors.Count' { + $errors.Count | Should -Be 1 + } + + It '$errors[0].ErrorId' { + $errors[0].ErrorId | Should -BeExactly 'InvalidSwitchFlag' + } } Context 'Generate an error on -parallel' { - $errors = @() - $ast = [System.Management.Automation.Language.Parser]::ParseInput( - 'switch -parallel ($foo) {1 {break}}', [ref]$null, [ref]$errors) - It '$errors.Count' { $errors.Count | Should -Be 1 } - It '$errors[0].ErrorId' { $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' } - } + BeforeAll { + $errors = @() + $ast = [System.Management.Automation.Language.Parser]::ParseInput( + 'switch -parallel ($foo) {1 {break}}', [ref]$null, [ref]$errors) + } + + It '$errors.Count' { + $errors.Count | Should -Be 1 + } + It '$errors[0].ErrorId' { + $errors[0].ErrorId | Should -Be 'KeywordParameterReservedForFutureUse' + } + } } From 03ffbdcfb099f9db300f6fca456497e8ae9ad111 Mon Sep 17 00:00:00 2001 From: Kirk Munro Date: Fri, 9 Aug 2019 12:12:44 -0300 Subject: [PATCH 5/5] add missing open brace (was removed accidentally) --- test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 index b5e264a27eb..c820e947fe0 100644 --- a/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 +++ b/test/powershell/Language/Scripting/ForeachParallel.Tests.ps1 @@ -21,7 +21,7 @@ Describe 'Parallel foreach syntax' -Tags 'CI' { "foreach `n-parallel `n(`$foo in `$bar) {}", [ref] $null, [ref] $null) } - It '$errors.Count' + It '$errors.Count' { $errors.Count | Should -Be 0 }