From 41f12b1d2cb74edbc25b2cf7aa8b13ee39c6ca2c Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 6 Sep 2017 10:25:33 -0700 Subject: [PATCH] Push locals of automatic variables to 'DottedScopes' when dotting script cmdlets (#4709) When dotting a script cmdlet, the locals of automatic variables from the `PSScriptCmdlet` is not set up in the current scope before parameter binding. The fix is to push the locals in `CommandProcessor.OnSetCurrentScope` and pop them in `CommandProcessor.OnRestorePreviousScope`, which will be called from `SetCurrentScopeToExecutionScope` and `RestorePreviousScope` respectively. Summary of changes: 1. When a new local scope is used, currently we set the locals for `CommandProcessor` right before parameter binding (in `BindCommandLineParametersNoValidation`); we set the locals for `ScriptCommandProcessor` in `Prepare`. I moved both to the constructor, right after the new scope is created so that the code is more consistent. 2. In `CmdletParameterBinderController.cs`, we set up the `PSBoundParameters` and `MyInvocation` variables in `HandleCommandLineDynamicParameters` again, which I think is unnecessary because this method is only called from `BindCommandLineParametersNoValidation`, where the setup is done for the first time. 3. Currently, the locals will be set for dotted script cmdlet in `EnterScope()` and `ExitScope()`. Now, that logic is moved to `OnSetCurrentScope` and `OnRestorePreviousScope` of `CommandProcessor`. This not only makes sure that locals are set before parameter binding, but also is consistent with the `ScriptCommandProcessor`. --- .../engine/CmdletParameterBinderController.cs | 12 +---- .../engine/CommandProcessor.cs | 34 +++++++++++-- .../engine/ScriptCommandProcessor.cs | 16 +++--- .../engine/runtime/CompiledScriptBlock.cs | 41 ++++++++++------ .../ParameterBinding.Tests.ps1 | 49 +++++++++++++++++++ 5 files changed, 116 insertions(+), 36 deletions(-) diff --git a/src/System.Management.Automation/engine/CmdletParameterBinderController.cs b/src/System.Management.Automation/engine/CmdletParameterBinderController.cs index bcc1cc5bdd..a50b75bcef 100644 --- a/src/System.Management.Automation/engine/CmdletParameterBinderController.cs +++ b/src/System.Management.Automation/engine/CmdletParameterBinderController.cs @@ -215,9 +215,7 @@ namespace System.Management.Automation var psCompiledScriptCmdlet = this.Command as PSScriptCmdlet; if (psCompiledScriptCmdlet != null) { - psCompiledScriptCmdlet.PrepareForBinding( - ((ScriptParameterBinder)this.DefaultParameterBinder).LocalScope, - this.CommandLineParameters); + psCompiledScriptCmdlet.PrepareForBinding(this.CommandLineParameters); } // Add the passed in arguments to the unboundArguments collection @@ -1759,14 +1757,6 @@ namespace System.Management.Automation { s_tracer.WriteLine("Getting the bindable object from the Cmdlet"); - var psCompiledScriptCmdlet = this.Command as PSScriptCmdlet; - if (psCompiledScriptCmdlet != null) - { - psCompiledScriptCmdlet.PrepareForBinding( - ((ScriptParameterBinder)this.DefaultParameterBinder).LocalScope, - this.CommandLineParameters); - } - // Now get the dynamic parameter bindable object. object dynamicParamBindableObject; diff --git a/src/System.Management.Automation/engine/CommandProcessor.cs b/src/System.Management.Automation/engine/CommandProcessor.cs index 8bc03b2e00..bb283162b5 100644 --- a/src/System.Management.Automation/engine/CommandProcessor.cs +++ b/src/System.Management.Automation/engine/CommandProcessor.cs @@ -225,6 +225,28 @@ namespace System.Management.Automation BindCommandLineParameters(); } + protected override void OnSetCurrentScope() + { + // When dotting a script cmdlet, push the locals of automatic variables to + // the 'DottedScopes' of the current scope. + PSScriptCmdlet scriptCmdlet = this.Command as PSScriptCmdlet; + if (scriptCmdlet != null && !UseLocalScope) + { + scriptCmdlet.PushDottedScope(CommandSessionState.CurrentScope); + } + } + + protected override void OnRestorePreviousScope() + { + // When dotting a script cmdlet, pop the locals of automatic variables from + // the 'DottedScopes' of the current scope. + PSScriptCmdlet scriptCmdlet = this.Command as PSScriptCmdlet; + if (scriptCmdlet != null && !UseLocalScope) + { + scriptCmdlet.PopDottedScope(CommandSessionState.CurrentScope); + } + } + /// /// Execute BeginProcessing part of command /// @@ -731,14 +753,18 @@ namespace System.Management.Automation private void Init(IScriptCommandInfo scriptCommandInfo) { - InternalCommand scriptCmdlet = - new PSScriptCmdlet(scriptCommandInfo.ScriptBlock, _useLocalScope, FromScriptFile, _context); - + var scriptCmdlet = new PSScriptCmdlet(scriptCommandInfo.ScriptBlock, UseLocalScope, FromScriptFile, _context); this.Command = scriptCmdlet; - this.CommandScope = _useLocalScope + this.CommandScope = UseLocalScope ? this.CommandSessionState.NewScope(_fromScriptFile) : this.CommandSessionState.CurrentScope; + if (UseLocalScope) + { + // Set the 'LocalsTuple' of the new scope to that of the scriptCmdlet + scriptCmdlet.SetLocalsTupleForNewScope(CommandScope); + } + InitCommon(); // If the script has been dotted, throw an error if it's from a different language mode. diff --git a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs index dcd9d7af4a..18f1e41b56 100644 --- a/src/System.Management.Automation/engine/ScriptCommandProcessor.cs +++ b/src/System.Management.Automation/engine/ScriptCommandProcessor.cs @@ -269,6 +269,12 @@ namespace System.Management.Automation _obsoleteAttribute = _scriptBlock.ObsoleteAttribute; _runOptimizedCode = _scriptBlock.Compile(optimized: _context._debuggingMode > 0 ? false : UseLocalScope); _localsTuple = _scriptBlock.MakeLocalsTuple(_runOptimizedCode); + + if (UseLocalScope) + { + Diagnostics.Assert(CommandScope.LocalsTuple == null, "a newly created scope shouldn't have it's tuple set."); + CommandScope.LocalsTuple = _localsTuple; + } } /// @@ -282,12 +288,6 @@ namespace System.Management.Automation internal override void Prepare(IDictionary psDefaultParameterValues) { - if (UseLocalScope) - { - Diagnostics.Assert(CommandScope.LocalsTuple == null, "a newly created scope shouldn't have it's tuple set."); - CommandScope.LocalsTuple = _localsTuple; - } - _localsTuple.SetAutomaticVariable(AutomaticVariable.MyInvocation, this.Command.MyInvocation, _context); _scriptBlock.SetPSScriptRootAndPSCommandPath(_localsTuple, _context); _functionContext = new FunctionContext @@ -585,6 +585,8 @@ namespace System.Management.Automation protected override void OnSetCurrentScope() { + // When dotting a script, push the locals of automatic variables to + // the 'DottedScopes' of the current scope. if (!UseLocalScope) { CommandSessionState.CurrentScope.DottedScopes.Push(_localsTuple); @@ -593,6 +595,8 @@ namespace System.Management.Automation protected override void OnRestorePreviousScope() { + // When dotting a script, pop the locals of automatic variables from + // the 'DottedScopes' of the current scope. if (!UseLocalScope) { CommandSessionState.CurrentScope.DottedScopes.Pop(); diff --git a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs index 0811b842dd..a46d6dacc4 100644 --- a/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs +++ b/src/System.Management.Automation/engine/runtime/CompiledScriptBlock.cs @@ -1864,21 +1864,11 @@ namespace System.Management.Automation private void EnterScope() { _commandRuntime.SetVariableListsInPipe(); - - if (!_useLocalScope) - { - this.Context.SessionState.Internal.CurrentScope.DottedScopes.Push(_localsTuple); - } } private void ExitScope() { _commandRuntime.RemoveVariableListsInPipe(); - - if (!_useLocalScope) - { - this.Context.SessionState.Internal.CurrentScope.DottedScopes.Pop(); - } } private void RunClause(Action clause, object dollarUnderbar, object inputToProcess) @@ -1986,12 +1976,33 @@ namespace System.Management.Automation return null; } - public void PrepareForBinding(SessionStateScope scope, CommandLineParameters commandLineParameters) + /// + /// If the script cmdlet will run in a new local scope, this method is used to set the locals to the newly created scope. + /// + internal void SetLocalsTupleForNewScope(SessionStateScope scope) + { + Diagnostics.Assert(scope.LocalsTuple == null, "a newly created scope shouldn't have it's tuple set."); + scope.LocalsTuple = _localsTuple; + } + + /// + /// If the script cmdlet is dotted, this method is used to push the locals to the 'DottedScopes' of the current scope. + /// + internal void PushDottedScope(SessionStateScope scope) + { + scope.DottedScopes.Push(_localsTuple); + } + + /// + /// If the script cmdlet is dotted, this method is used to pop the locals from the 'DottedScopes' of the current scope. + /// + internal void PopDottedScope(SessionStateScope scope) + { + scope.DottedScopes.Pop(); + } + + internal void PrepareForBinding(CommandLineParameters commandLineParameters) { - if (_useLocalScope && scope.LocalsTuple == null) - { - scope.LocalsTuple = _localsTuple; - } _localsTuple.SetAutomaticVariable(AutomaticVariable.PSBoundParameters, commandLineParameters.GetValueToBindToPSBoundParameters(), this.Context); _localsTuple.SetAutomaticVariable(AutomaticVariable.MyInvocation, MyInvocation, this.Context); diff --git a/test/powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1 b/test/powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1 index 53440baa18..dcd924f8a6 100644 --- a/test/powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1 +++ b/test/powershell/engine/ParameterBinding/ParameterBinding.Tests.ps1 @@ -280,4 +280,53 @@ $_.Exception.Message | should match "Parameter2" } } + + Context "Use automatic variables as default value for parameters" { + BeforeAll { + ## Explicit use of 'CmdletBinding' make it a script cmdlet + $test1 = @' + [CmdletBinding()] + param ($Root = $PSScriptRoot) + "[$Root]" +'@ + ## Use of 'Parameter' implicitly make it a script cmdlet + $test2 = @' + param ( + [Parameter()] + $Root = $PSScriptRoot + ) + "[$Root]" +'@ + $tempDir = Join-Path -Path $TestDrive -ChildPath "DefaultValueTest" + $test1File = Join-Path -Path $tempDir -ChildPath "test1.ps1" + $test2File = Join-Path -Path $tempDir -ChildPath "test2.ps1" + + $expected = "[$tempDir]" + $psPath = "$PSHOME\powershell" + + $null = New-Item -Path $tempDir -ItemType Directory -Force + Set-Content -Path $test1File -Value $test1 -Force + Set-Content -Path $test2File -Value $test2 -Force + } + + AfterAll { + Remove-Item -Path $tempDir -Recurse -Force -ErrorAction SilentlyContinue + } + + It "Test dot-source should evaluate '`$PSScriptRoot' for parameter default value" { + $result = . $test1File + $result | Should Be $expected + + $result = . $test2File + $result | Should Be $expected + } + + It "Test 'powershell -File' should evaluate '`$PSScriptRoot' for parameter default value" { + $result = & $psPath -NoProfile -File $test1File + $result | Should Be $expected + + $result = & $psPath -NoProfile -File $test2File + $result | Should Be $expected + } + } }