diff --git a/cgmanifest.json b/cgmanifest.json index 30a342907d8..96137367dd2 100644 --- a/cgmanifest.json +++ b/cgmanifest.json @@ -685,7 +685,7 @@ "Type": "nuget", "Nuget": { "Name": "StyleCop.Analyzers.Unstable", - "Version": "1.2.0.354" + "Version": "1.2.0.376" } }, "DevelopmentDependency": true diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index d9d54539bac..c642e30e885 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -382,17 +382,6 @@ public void Dispose() private Exception _taskCollectionException; private string _currentLocationPath; - // List of Foreach-Object command names and aliases. - // TODO: Look into using SessionState.Internal.GetAliasTable() to find all user created aliases. - // But update Alias command logic to maintain reverse table that lists all aliases mapping - // to a single command definition, for performance. - private static readonly string[] forEachNames = new string[] - { - "ForEach-Object", - "foreach", - "%" - }; - private void InitParallelParameterSet() { // The following common parameters are not (yet) supported in this parameter set. @@ -423,8 +412,7 @@ private void InitParallelParameterSet() _usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesForEachParallel( scriptBlock: Parallel, isTrustedInput: allowUsingExpression, - context: this.Context, - foreachNames: forEachNames); + context: this.Context); // Validate using values map, which is a map of '$using:' variables referenced in the script. // Script block variables are not allowed since their behavior is undefined outside the runspace diff --git a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs index fc1dba4aed8..efa604eb21e 100644 --- a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs +++ b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs @@ -324,13 +324,11 @@ internal static PowerShell Convert(ScriptBlockAst body, /// Scriptblock to search. /// True when input is trusted. /// Execution context. - /// List of foreach command names and aliases. /// Dictionary of using variable map. internal static Dictionary GetUsingValuesForEachParallel( ScriptBlock scriptBlock, bool isTrustedInput, - ExecutionContext context, - string[] foreachNames) + ExecutionContext context) { // Using variables for Foreach-Object -Parallel use are restricted to be within the // Foreach-Object -Parallel call scope. This will filter the using variable map to variables @@ -350,7 +348,7 @@ internal static Dictionary GetUsingValuesForEachParallel( for (int i = 0; i < usingAsts.Count; ++i) { usingAst = (UsingExpressionAst)usingAsts[i]; - if (IsInForeachParallelCallingScope(usingAst, foreachNames)) + if (IsInForeachParallelCallingScope(scriptBlock.Ast, usingAst)) { var value = Compiler.GetExpressionValue(usingAst.SubExpression, isTrustedInput, context); string usingAstKey = PsUtils.GetUsingExpressionKey(usingAst); @@ -382,17 +380,61 @@ internal static Dictionary GetUsingValuesForEachParallel( return usingValueMap; } + // List of Foreach-Object command names and aliases. + // TODO: Look into using SessionState.Internal.GetAliasTable() to find all user created aliases. + // But update Alias command logic to maintain reverse table that lists all aliases mapping + // to a single command definition, for performance. + private static readonly string[] forEachNames = new string[] + { + "ForEach-Object", + "foreach", + "%" + }; + + private static bool FindForEachInCommand(CommandAst commandAst) + { + // Command name is always the first element in the CommandAst. + // e.g., 'foreach -parallel {}' + var commandNameElement = (commandAst.CommandElements.Count > 0) ? commandAst.CommandElements[0] : null; + if (commandNameElement is StringConstantExpressionAst commandName) + { + bool found = false; + foreach (var foreachName in forEachNames) + { + if (commandName.Value.Equals(foreachName, StringComparison.OrdinalIgnoreCase)) + { + found = true; + break; + } + } + + if (found) + { + // Verify this is foreach-object with parallel parameter set. + var bindingResult = StaticParameterBinder.BindCommand(commandAst); + if (bindingResult.BoundParameters.ContainsKey("Parallel")) + { + return true; + } + } + } + + return false; + } + /// /// Walks the using Ast to verify it is used within a foreach-object -parallel command /// and parameter set scope, and not from within a nested foreach-object -parallel call. /// + /// Scriptblock Ast containing this using Ast /// Using Ast to check. - /// List of foreach-object command names. /// True if using expression is in current call scope. private static bool IsInForeachParallelCallingScope( - UsingExpressionAst usingAst, - string[] foreachNames) + Ast scriptblockAst, + UsingExpressionAst usingAst) { + Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null."); + /* Example: $Test1 = "Hello" @@ -405,54 +447,23 @@ private static bool IsInForeachParallelCallingScope( } } */ - Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null."); // Search up the parent Ast chain for 'Foreach-Object -Parallel' commands. Ast currentParent = usingAst.Parent; - int foreachNestedCount = 0; - while (currentParent != null) + while (currentParent != scriptblockAst) { // Look for Foreach-Object outer commands - if (currentParent is CommandAst commandAst) + if (currentParent is CommandAst commandAst && + FindForEachInCommand(commandAst)) { - foreach (var commandElement in commandAst.CommandElements) - { - if (commandElement is StringConstantExpressionAst commandName) - { - bool found = false; - foreach (var foreachName in foreachNames) - { - if (commandName.Value.Equals(foreachName, StringComparison.OrdinalIgnoreCase)) - { - found = true; - break; - } - } - - if (found) - { - // Verify this is foreach-object with parallel parameter set. - var bindingResult = StaticParameterBinder.BindCommand(commandAst); - if (bindingResult.BoundParameters.ContainsKey("Parallel")) - { - foreachNestedCount++; - break; - } - } - } - } - } - - if (foreachNestedCount > 1) - { - // This using expression Ast is outside the original calling scope. + // Using Ast is outside the invoking foreach scope. return false; } currentParent = currentParent.Parent; } - return foreachNestedCount == 1; + return true; } /// diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 994d71601a7..fb7438608d1 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -87,6 +87,185 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand' } + It 'Verifies using variables with passed-in script block object' { + + $var = "Hello" + $varArray = "Hello","There" + $sBlock = [scriptblock]::Create('$using:var; $using:varArray[1]') + $result = 1..1 | ForEach-Object -Parallel $sBlock + $result[0] | Should -BeExactly $var + $result[1] | Should -BeExactly $varArray[1] + } + + It 'Verifies using variables with passed-in {} script block object' { + + $var = "Hello" + $varArray = "Hello","There" + $sBlock = { $using:var; $using:varArray[1] } + $result = 1..1 | ForEach-Object -Parallel $sBlock + $result[0] | Should -BeExactly $var + $result[1] | Should -BeExactly $varArray[1] + } + + It 'Verifies in scope using same-name variables in nested calls for passed-in script block objects' { + + $Test1 = "Test1" + $sBlock = [scriptblock]::Create(@' + $using:Test1 + $Test2 = "Test2" + $sBlock2 = [scriptblock]::Create('$using:Test2') + 1..2 | ForEach-Object -Parallel $sBlock2 +'@) + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using same-name variables in nested calls for passed-in {} script block objects' { + + $Test1 = "Test1" + $sBlock = { + $using:Test1 + $Test2 = "Test2" + $sBlock2 = [scriptblock]::Create('$using:Test2') + 1..2 | ForEach-Object -Parallel $sBlock2 + } + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using same-name variables in nested calls for mixed script block objects' { + + $Test1 = "Test1" + $sBlock = [scriptblock]::Create(@' + $using:Test1 + $Test2 = "Test2" + 1..2 | ForEach-Object -Parallel { $using:Test2 } +'@) + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using same-name variables in nested calls for mixed script block {} objects' { + + $Test1 = "Test1" + $sBlock = { + $using:Test1 + $Test2 = "Test2" + 1..2 | ForEach-Object -Parallel { $using:Test2 } + } + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using different-name variables in nested calls for passed-in script block objects' { + + $Test1 = "Test1" + $sBlock = [scriptblock]::Create(@' + $using:Test1 + $Test2 = "Test2" + $sBlock2 = [scriptblock]::Create('$using:Test2') + 1..2 | ForEach-Object -Parallel $sBlock2 +'@) + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using different-name variables in nested calls for passed-in script {} block objects' { + + $Test1 = "Test1" + $sBlock = { + $using:Test1 + $Test2 = "Test2" + $sBlock2 = [scriptblock]::Create('$using:Test2') + 1..2 | ForEach-Object -Parallel $sBlock2 + } + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using different-name variables in nested calls for mixed script block objects' { + + $Test1 = "Test1" + $sBlock = [scriptblock]::Create(@' + $using:Test1 + $Test2 = "Test2" + 1..2 | ForEach-Object -Parallel { $using:Test2 } +'@) + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies in scope using different-name variables in nested calls for mixed script block {} objects' { + + $Test1 = "Test1" + $sBlock = { + $using:Test1 + $Test2 = "Test2" + 1..2 | ForEach-Object -Parallel { $using:Test2 } + } + $results = 1..2 | ForEach-Object -Parallel $sBlock + $results.Count | Should -BeExactly 6 + $groups = $results | Group-Object -AsHashTable + $groups['Test1'].Count | Should -BeExactly 2 + $groups['Test2'].Count | Should -BeExactly 4 + } + + It 'Verifies expected error for out of scope using variable in nested calls for passed-in script block objects' { + + $Test = "TestZ" + $sBlock = [scriptblock]::Create(@' + $using:Test + $sBlock2 = [scriptblock]::Create('$using:Test') + 1..1 | ForEach-Object -Parallel $sBlock2 +'@) + 1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null + $usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + + It 'Verifies expected error for out of scope using variable in nested calls for passed-in {} script block objects' { + + $Test = "TestZ" + $sBlock = [scriptblock]::Create(@' + $using:Test + $sBlock2 = { $using:Test } + 1..1 | ForEach-Object -Parallel $sBlock2 +'@) + 1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null + $usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + + It 'Verifies expected error for out of scope using variable in nested calls for mixed script block objects' { + + $Test = "TestZ" + $sBlock = [scriptblock]::Create(@' + $using:Test + 1..1 | ForEach-Object -Parallel { $using:Test } +'@) + 1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null + $usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand' + } + It 'Verifies terminating error streaming' { $result = 1..1 | ForEach-Object -Parallel { throw 'Terminating Error!'; "Hello" } 2>&1 @@ -150,7 +329,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' { $pr[0].StatusDescription | Should -Be Running $ps.Dispose() } - + It 'Verifies information data streaming' { $actualInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1