From 896748ecbeaa8cff2da724718e9fe1389188d68c Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sat, 10 Dec 2022 19:21:34 +0100 Subject: [PATCH 1/3] Fix TabExpansion2 variable leak when completing variables --- .../CommandCompletion/CommandCompletion.cs | 37 ++++--------------- .../TabCompletion/TabCompletion.Tests.ps1 | 5 +++ 2 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs index d0d248fa50a..5a6bf101fd3 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs @@ -537,43 +537,23 @@ private static CommandCompletion CompleteInputImpl(Ast ast, Token[] tokens, IScr if (results == null || results.Count == 0) { - /* BROKEN code commented out, fix sometime // If we were invoked from TabExpansion2, we want to "remove" TabExpansion2 and anything it calls // from our results. We do this by faking out the session so that TabExpansion2 isn't anywhere to be found. - MutableTuple tupleForFrameToSkipPast = null; - foreach (var stackEntry in context.Debugger.GetCallStack()) + SessionStateScope scopeToRestore; + if (context.CurrentCommandProcessor.Command.CommandInfo.Name.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase)) { - dynamic stackEntryAsPSObj = PSObject.AsPSObject(stackEntry); - if (stackEntryAsPSObj.Command.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase)) - { - tupleForFrameToSkipPast = stackEntry.FunctionContext._localsTuple; - break; - } + scopeToRestore = context.EngineSessionState.CurrentScope; + context.EngineSessionState.CurrentScope = scopeToRestore.Parent; } - - SessionStateScope scopeToRestore = null; - if (tupleForFrameToSkipPast != null) + else { - // Find this tuple in the scope stack. - scopeToRestore = context.EngineSessionState.CurrentScope; - var scope = context.EngineSessionState.CurrentScope; - while (scope != null && scope.LocalsTuple != tupleForFrameToSkipPast) - { - scope = scope.Parent; - } - - if (scope != null) - { - context.EngineSessionState.CurrentScope = scope.Parent; - } + scopeToRestore = null; } try { - */ - var completionAnalysis = new CompletionAnalysis(ast, tokens, positionOfCursor, options); - results = completionAnalysis.GetResults(powershell, out replacementIndex, out replacementLength); - /* + var completionAnalysis = new CompletionAnalysis(ast, tokens, positionOfCursor, options); + results = completionAnalysis.GetResults(powershell, out replacementIndex, out replacementLength); } finally { @@ -582,7 +562,6 @@ private static CommandCompletion CompleteInputImpl(Ast ast, Token[] tokens, IScr context.EngineSessionState.CurrentScope = scopeToRestore; } } - */ } var completionResults = results ?? EmptyCompletionResult; diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 528b81c23ac..02b4bba1cd8 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -823,6 +823,11 @@ Verb-Noun -Param1 Hello ^ $res.CompletionMatches[0].CompletionText | Should -Be "Get-ChildItem" } + it 'Should not complete TabExpansion2 variables' { + $res = TabExpansion2 -inputScript '$' -cursorColumn 1 + $res.CompletionMatches.CompletionText | Should -Not -Contain '$positionOfCursor' + } + Context "Script name completion" { BeforeAll { Setup -f 'install-powershell.ps1' -Content "" From f1f2fab89e09825cdafa5eb09dde2a7ef8a2dbdd Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Tue, 13 Dec 2022 21:35:18 +0100 Subject: [PATCH 2/3] Add null check for parent scope --- .../engine/CommandCompletion/CommandCompletion.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs index 5a6bf101fd3..5924fc02789 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs @@ -540,7 +540,8 @@ private static CommandCompletion CompleteInputImpl(Ast ast, Token[] tokens, IScr // If we were invoked from TabExpansion2, we want to "remove" TabExpansion2 and anything it calls // from our results. We do this by faking out the session so that TabExpansion2 isn't anywhere to be found. SessionStateScope scopeToRestore; - if (context.CurrentCommandProcessor.Command.CommandInfo.Name.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase)) + if (context.CurrentCommandProcessor.Command.CommandInfo.Name.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase) + && context.EngineSessionState.CurrentScope.Parent is not null) { scopeToRestore = context.EngineSessionState.CurrentScope; context.EngineSessionState.CurrentScope = scopeToRestore.Parent; From 8581b5474ec6b9c14cb5193264c307bac6a63a91 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Tue, 13 Dec 2022 23:51:42 +0100 Subject: [PATCH 3/3] Add check for dot sourced TabExpansion2 call. --- .../engine/CommandCompletion/CommandCompletion.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs index 5924fc02789..b0e9239fb3e 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CommandCompletion.cs @@ -541,6 +541,7 @@ private static CommandCompletion CompleteInputImpl(Ast ast, Token[] tokens, IScr // from our results. We do this by faking out the session so that TabExpansion2 isn't anywhere to be found. SessionStateScope scopeToRestore; if (context.CurrentCommandProcessor.Command.CommandInfo.Name.Equals("TabExpansion2", StringComparison.OrdinalIgnoreCase) + && context.CurrentCommandProcessor.UseLocalScope && context.EngineSessionState.CurrentScope.Parent is not null) { scopeToRestore = context.EngineSessionState.CurrentScope;