From 565bb13dd7d4f1b18703d06e95c099ef24da700c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 22 Mar 2018 17:26:57 -0700 Subject: [PATCH 1/9] Add error handling for interactive #requires --- .../engine/AutomationEngine.cs | 8 ++++++++ .../engine/parser/tokenizer.cs | 3 +++ .../resources/ParserStrings.resx | 3 +++ 3 files changed, 14 insertions(+) diff --git a/src/System.Management.Automation/engine/AutomationEngine.cs b/src/System.Management.Automation/engine/AutomationEngine.cs index 279bab545ab..41bbcf13604 100644 --- a/src/System.Management.Automation/engine/AutomationEngine.cs +++ b/src/System.Management.Automation/engine/AutomationEngine.cs @@ -76,6 +76,14 @@ internal ScriptBlock ParseScriptBlock(string script, string fileName, bool inter if (interactiveCommand) { + if (ast.ScriptRequirements != null) + { + errors = new [] + { + new ParseError(ast.Extent, nameof(ParserStrings.RequiresCannotBeUsedInInteractiveConsole), + ParserStrings.RequiresCannotBeUsedInInteractiveConsole) + }; + } EngineParser.SetPreviousFirstLastToken(Context); } diff --git a/src/System.Management.Automation/engine/parser/tokenizer.cs b/src/System.Management.Automation/engine/parser/tokenizer.cs index 1a1045afc5a..b5d73758a33 100644 --- a/src/System.Management.Automation/engine/parser/tokenizer.cs +++ b/src/System.Management.Automation/engine/parser/tokenizer.cs @@ -492,6 +492,7 @@ internal class TokenizerState internal string Script; internal int TokenStart; internal int CurrentIndex; + internal Token FirstToken; internal Token LastToken; internal BitArray SkippedCharOffsets; internal List TokenList; @@ -687,6 +688,7 @@ internal TokenizerState StartNestedScan(UnscannedSubExprToken nestedText) NestedTokensAdjustment = _nestedTokensAdjustment, Script = _script, TokenStart = _tokenStart, + FirstToken = FirstToken, LastToken = LastToken, SkippedCharOffsets = _skippedCharOffsets, TokenList = TokenList, @@ -708,6 +710,7 @@ internal void FinishNestedScan(TokenizerState ts) _nestedTokensAdjustment = ts.NestedTokensAdjustment; _script = ts.Script; _tokenStart = ts.TokenStart; + FirstToken = ts.FirstToken; LastToken = ts.LastToken; _skippedCharOffsets = ts.SkippedCharOffsets; TokenList = ts.TokenList; diff --git a/src/System.Management.Automation/resources/ParserStrings.resx b/src/System.Management.Automation/resources/ParserStrings.resx index 5f9b75345fe..cc0fe63372b 100644 --- a/src/System.Management.Automation/resources/ParserStrings.resx +++ b/src/System.Management.Automation/resources/ParserStrings.resx @@ -1458,4 +1458,7 @@ ModuleVersion : Version of module to import. If used, ModuleName must represent Conflict in using PsDscRunAsCredential for Resource {0} because it already specifies PsDscRunAsCredential value. We can only use one PsDscRunAsCredential for the composite resource. + + A #requires statement cannot be used in an interactive console + From b7a773f87dc29ca8de29f23a4fd0ea375781a477 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 23 Mar 2018 14:00:56 -0700 Subject: [PATCH 2/9] Remove new error on interactive #requires parse --- .../engine/AutomationEngine.cs | 18 +++++------------- .../resources/ParserStrings.resx | 3 --- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/System.Management.Automation/engine/AutomationEngine.cs b/src/System.Management.Automation/engine/AutomationEngine.cs index 41bbcf13604..58e490759a6 100644 --- a/src/System.Management.Automation/engine/AutomationEngine.cs +++ b/src/System.Management.Automation/engine/AutomationEngine.cs @@ -62,28 +62,20 @@ internal string Expand(string s) /// Compile a piece of text into a parse tree for later execution. /// /// The text to parse - /// + /// true iff the scriptblock will be added to history /// The parse text as a parsetree node. - internal ScriptBlock ParseScriptBlock(string script, bool interactiveCommand) + internal ScriptBlock ParseScriptBlock(string script, bool addToHistory) { - return ParseScriptBlock(script, null, interactiveCommand); + return ParseScriptBlock(script, null, addToHistory); } - internal ScriptBlock ParseScriptBlock(string script, string fileName, bool interactiveCommand) + internal ScriptBlock ParseScriptBlock(string script, string fileName, bool addToHistory) { ParseError[] errors; var ast = EngineParser.Parse(fileName, script, null, out errors, ParseMode.Default); - if (interactiveCommand) + if (addToHistory) { - if (ast.ScriptRequirements != null) - { - errors = new [] - { - new ParseError(ast.Extent, nameof(ParserStrings.RequiresCannotBeUsedInInteractiveConsole), - ParserStrings.RequiresCannotBeUsedInInteractiveConsole) - }; - } EngineParser.SetPreviousFirstLastToken(Context); } diff --git a/src/System.Management.Automation/resources/ParserStrings.resx b/src/System.Management.Automation/resources/ParserStrings.resx index cc0fe63372b..5f9b75345fe 100644 --- a/src/System.Management.Automation/resources/ParserStrings.resx +++ b/src/System.Management.Automation/resources/ParserStrings.resx @@ -1458,7 +1458,4 @@ ModuleVersion : Version of module to import. If used, ModuleName must represent Conflict in using PsDscRunAsCredential for Resource {0} because it already specifies PsDscRunAsCredential value. We can only use one PsDscRunAsCredential for the composite resource. - - A #requires statement cannot be used in an interactive console - From 163f93dd62637fa941bd2725e0d7673f0b176356 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 26 Mar 2018 10:31:19 -0700 Subject: [PATCH 3/9] Add interactive requires test --- .../powershell/Language/Scripting/Requires.Tests.ps1 | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index d4a25d04a70..f3df0d2e517 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -27,4 +27,14 @@ Describe "Requires tests" -Tags "CI" { } } -} \ No newline at end of file + Context "Interactive requires" { + It "Successfully does nothing when given '#requires' interactively" { + $settings = [System.Management.Automation.PSInvocationSettings]::new() + $settings.AddToHistory = $true + + $ps = [powershell]::Create() + $ps.AddScript("#requires`n10") + $ps.Invoke(@(), $settings) | Should -BeExactly 10 + } + } +} From cca907d29146985b96999be4178245a5f30cc5a0 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 26 Mar 2018 11:28:49 -0700 Subject: [PATCH 4/9] Add tests for #requires tokenizer state --- ex.ps1 | 4 ++++ test/powershell/Language/Parser/Parser.Tests.ps1 | 13 +++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 ex.ps1 diff --git a/ex.ps1 b/ex.ps1 new file mode 100644 index 00000000000..ba711b88d57 --- /dev/null +++ b/ex.ps1 @@ -0,0 +1,4 @@ +Write-Host "Hello" +#requires +7 +@($^, $$) diff --git a/test/powershell/Language/Parser/Parser.Tests.ps1 b/test/powershell/Language/Parser/Parser.Tests.ps1 index 0edc62fda66..536fe10627e 100644 --- a/test/powershell/Language/Parser/Parser.Tests.ps1 +++ b/test/powershell/Language/Parser/Parser.Tests.ps1 @@ -915,4 +915,17 @@ foo``u{2195}abc # Issue #2780 { ExecuteCommand "`$herestr=@`"`n'`"'`n`"@" } | Should Not Throw } + + Context "#requires nested scan tokenizer tests" { + $testCases = @( + @{ script = "#requires"; firstToken = $null; lastToken = $null }, + @{ script = "#requires -Version 5.0`n10"; firstToken = "10"; lastToken = "10" }, + @{ script = "Write-Host 'Hello'`n#requires -Version 5.0`n7"; firstToken = "Write-Host"; lastToken = "7" }, + @{ script = "Write-Host 'Hello'`n#requires -Version 5.0"; firstToken = "Write-Host"; lastToken = "'Hello'"} + ) + + It "Correctly resets the first and last tokens in the tokenizer after nested scan in script" -TestCases $testCases { + param($script, $firstToken, $lastToken) + } + } } From 9aa89814471d121f8d6d29942857b7c0651786a9 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 26 Mar 2018 11:59:05 -0700 Subject: [PATCH 5/9] Add tests for token state resetting for #requires scans --- .../Language/Parser/Parser.Tests.ps1 | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/test/powershell/Language/Parser/Parser.Tests.ps1 b/test/powershell/Language/Parser/Parser.Tests.ps1 index 536fe10627e..941fe23022e 100644 --- a/test/powershell/Language/Parser/Parser.Tests.ps1 +++ b/test/powershell/Language/Parser/Parser.Tests.ps1 @@ -917,15 +917,33 @@ foo``u{2195}abc } Context "#requires nested scan tokenizer tests" { + BeforeAll { + $settings = [System.Management.Automation.PSInvocationSettings]::new() + $settings.AddToHistory = $true + + $ps = [powershell]::Create() + } + + AfterEach { + $ps.Commands.Clear() + } + $testCases = @( @{ script = "#requires"; firstToken = $null; lastToken = $null }, @{ script = "#requires -Version 5.0`n10"; firstToken = "10"; lastToken = "10" }, @{ script = "Write-Host 'Hello'`n#requires -Version 5.0`n7"; firstToken = "Write-Host"; lastToken = "7" }, - @{ script = "Write-Host 'Hello'`n#requires -Version 5.0"; firstToken = "Write-Host"; lastToken = "'Hello'"} + @{ script = "Write-Host 'Hello'`n#requires -Version 5.0"; firstToken = "Write-Host"; lastToken = "Hello"} ) It "Correctly resets the first and last tokens in the tokenizer after nested scan in script" -TestCases $testCases { param($script, $firstToken, $lastToken) + + $ps.AddScript($script) + $ps.AddScript("(`$^,`$`$)") + $tokens = $ps.Invoke(@(), $settings) + + $tokens[0] | Should -BeExactly $firstToken + $tokens[1] | Should -BeExactly $lastToken } } } From b3c4324294930df9bfd0055839130bd0078d3da2 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 26 Mar 2018 13:11:35 -0700 Subject: [PATCH 6/9] Removed extraneous file --- ex.ps1 | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 ex.ps1 diff --git a/ex.ps1 b/ex.ps1 deleted file mode 100644 index ba711b88d57..00000000000 --- a/ex.ps1 +++ /dev/null @@ -1,4 +0,0 @@ -Write-Host "Hello" -#requires -7 -@($^, $$) From 214e854a19b4e6c8acf02a358be2ce8d074007ab Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 24 Apr 2018 16:24:42 -0700 Subject: [PATCH 7/9] [feature]Address PR feedback from @daxian-dbw --- test/powershell/Language/Parser/Parser.Tests.ps1 | 4 ++++ test/powershell/Language/Scripting/Requires.Tests.ps1 | 9 ++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/powershell/Language/Parser/Parser.Tests.ps1 b/test/powershell/Language/Parser/Parser.Tests.ps1 index 941fe23022e..6307fa5c99a 100644 --- a/test/powershell/Language/Parser/Parser.Tests.ps1 +++ b/test/powershell/Language/Parser/Parser.Tests.ps1 @@ -924,6 +924,10 @@ foo``u{2195}abc $ps = [powershell]::Create() } + AfterAll { + $ps.Dispose() + } + AfterEach { $ps.Commands.Clear() } diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index f3df0d2e517..fb729e31cac 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -32,9 +32,12 @@ Describe "Requires tests" -Tags "CI" { $settings = [System.Management.Automation.PSInvocationSettings]::new() $settings.AddToHistory = $true - $ps = [powershell]::Create() - $ps.AddScript("#requires`n10") - $ps.Invoke(@(), $settings) | Should -BeExactly 10 + { + $ps = [powershell]::Create() + $ps.AddScript("#requires") + $ps.Invoke(@(), $settings) + $ps.Dispose() + } | Should -Not -Throw } } } From c8617dc1d8a931e0d6c53acc988964e41e33ffba Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 24 Apr 2018 16:31:06 -0700 Subject: [PATCH 8/9] [feature]Refactor test to dispose of powershell instance --- .../powershell/Language/Scripting/Requires.Tests.ps1 | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index fb729e31cac..37720195ad5 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -28,15 +28,21 @@ Describe "Requires tests" -Tags "CI" { } Context "Interactive requires" { + + BeforeAll { + $ps = [powershell]::Create() + } + + AfterAll { + $ps.Dispose() + } + It "Successfully does nothing when given '#requires' interactively" { $settings = [System.Management.Automation.PSInvocationSettings]::new() $settings.AddToHistory = $true - { - $ps = [powershell]::Create() $ps.AddScript("#requires") $ps.Invoke(@(), $settings) - $ps.Dispose() } | Should -Not -Throw } } From f615591866f951ef0bd01baec2ed4f8e199fd115 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 24 Apr 2018 16:40:29 -0700 Subject: [PATCH 9/9] Minor update to the test in 'Requires.Tests.ps1' --- test/powershell/Language/Scripting/Requires.Tests.ps1 | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index 37720195ad5..4354dba3cc6 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -40,10 +40,8 @@ Describe "Requires tests" -Tags "CI" { It "Successfully does nothing when given '#requires' interactively" { $settings = [System.Management.Automation.PSInvocationSettings]::new() $settings.AddToHistory = $true - { - $ps.AddScript("#requires") - $ps.Invoke(@(), $settings) - } | Should -Not -Throw + + { $ps.AddScript("#requires").Invoke(@(), $settings) } | Should -Not -Throw } } }