From ea3920c7818088fa9fe7b5342b34eff03a99995c Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sat, 20 Jan 2024 15:52:30 +0100 Subject: [PATCH 1/9] Don't complete duplicate command names --- .../CommandCompletion/CompletionCompleters.cs | 4 ++- .../TabCompletion/TabCompletion.Tests.ps1 | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 36bbfc8a718..70428a726b0 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -359,6 +359,8 @@ internal static List MakeCommandsUnique(IEnumerable // but put these at the end as it's less likely any of the hidden // commands are desired. If we can't add anything to disambiguate, // then we'll skip adding a completion result. + var modulesWithCommand = new HashSet(StringComparer.OrdinalIgnoreCase); + _ = modulesWithCommand.Add(((CommandInfo)commandList[0]).ModuleName); for (int index = 1; index < commandList.Count; index++) { var commandInfo = commandList[index] as CommandInfo; @@ -368,7 +370,7 @@ internal static List MakeCommandsUnique(IEnumerable { endResults.Add(GetCommandNameCompletionResult(commandInfo.Definition, commandInfo, addAmpersandIfNecessary, quote)); } - else if (!string.IsNullOrEmpty(commandInfo.ModuleName)) + else if (!string.IsNullOrEmpty(commandInfo.ModuleName) && modulesWithCommand.Add(commandInfo.ModuleName)) { var name = commandInfo.ModuleName + "\\" + commandInfo.Name; endResults.Add(GetCommandNameCompletionResult(name, commandInfo, addAmpersandIfNecessary, quote)); diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index b92a436e441..2633ee6cfaf 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -28,6 +28,40 @@ Describe "TabCompletion" -Tags CI { $res.CompletionMatches[0].CompletionText | Should -BeExactly 'notepad.exe' } + It 'Should not include duplicate command results' { + $OldModulePath = $env:PSModulePath + $tempDir = Join-Path -Path $TestDrive -ChildPath "TempPsModuleDir" + try + { + $ModuleDirs = @( + Join-Path $tempDir "TestModule1\1.0" + Join-Path $tempDir "TestModule1\1.1" + Join-Path $tempDir "TestModule2\1.0" + ) + foreach ($Dir in $ModuleDirs) + { + $NewDir = New-Item -Path $Dir -ItemType Directory -Force + $ModuleName = $NewDir.Parent.Name + Set-Content -Value 'MyTestFunction{}' -LiteralPath "$($NewDir.FullName)\$ModuleName.psm1" + New-ModuleManifest -Path "$($NewDir.FullName)\$ModuleName.psd1" -RootModule "$ModuleName.psm1" -FunctionsToExport "MyTestFunction" -ModuleVersion $NewDir.Name + } + + $env:PSModulePath += ";D:\Temp\TempPsModuleDir" + $Res = TabExpansion2 -inputScript MyTestFunction + $Res.CompletionMatches.Count | Should -Be 2 + $Res.CompletionMatches[0].CompletionText | Should -Be "MyTestFunction" + $Res.CompletionMatches[1].CompletionText | Should -Be "TestModule2\MyTestFunction" + } + finally + { + $env:PSModulePath = $OldModulePath + if (Test-Path -LiteralPath $tempDir) + { + Remove-Item $tempDir -Recurse -Force + } + } + } + It 'Should complete dotnet method' { $res = TabExpansion2 -inputScript '(1).ToSt' -cursorColumn '(1).ToSt'.Length $res.CompletionMatches[0].CompletionText | Should -BeExactly 'ToString(' From 7e05a1b71855ea05a60372ce4fed8cba15016b0e Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sat, 20 Jan 2024 16:30:42 +0100 Subject: [PATCH 2/9] Fix wrong path and separator in test --- .../Host/TabCompletion/TabCompletion.Tests.ps1 | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 2633ee6cfaf..5ecf24d9aad 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -46,7 +46,15 @@ Describe "TabCompletion" -Tags CI { New-ModuleManifest -Path "$($NewDir.FullName)\$ModuleName.psd1" -RootModule "$ModuleName.psm1" -FunctionsToExport "MyTestFunction" -ModuleVersion $NewDir.Name } - $env:PSModulePath += ";D:\Temp\TempPsModuleDir" + if ($IsWindows) + { + $env:PSModulePath += ";$tempDir" + } + else + { + $env:PSModulePath += ":$tempDir" + } + $Res = TabExpansion2 -inputScript MyTestFunction $Res.CompletionMatches.Count | Should -Be 2 $Res.CompletionMatches[0].CompletionText | Should -Be "MyTestFunction" From a2c6d8b8ae150572412560c88c13e212bac31a84 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sat, 20 Jan 2024 21:33:29 +0100 Subject: [PATCH 3/9] Update logic so commands with the same name stay grouped together and to add the module prefix if there's any ambiguity about which command will run. --- .../CommandCompletion/CompletionCompleters.cs | 64 ++++++++++--------- .../TabCompletion/TabCompletion.Tests.ps1 | 2 +- 2 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 70428a726b0..bfbe33d79e5 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -331,49 +331,55 @@ internal static List MakeCommandsUnique(IEnumerable List endResults = null; foreach (var keyValuePair in commandTable) { - var commandList = keyValuePair.Value as List; - if (commandList != null) + if (keyValuePair.Value is List commandList) { - endResults ??= new List(); - - // The first command might be an un-prefixed commandInfo that we get by importing a module with the -Prefix parameter, - // in that case, we should add the module name qualification because if the module is not in the module path, calling - // 'Get-Foo' directly doesn't work - string completionName = keyValuePair.Key; - if (!includeModulePrefix) + var modulesWithCommand = new HashSet(StringComparer.OrdinalIgnoreCase); + var importedModules = new HashSet(StringComparer.OrdinalIgnoreCase); + var commandInfoArray = new CommandInfo[commandList.Count]; + for (int i = 0; i < commandInfoArray.Length; i++) { - var commandInfo = commandList[0] as CommandInfo; - if (commandInfo != null && !string.IsNullOrEmpty(commandInfo.Prefix)) + var commandInfo = (CommandInfo)commandList[i]; + commandInfoArray[i] = commandInfo; + if (commandInfo.CommandType == CommandTypes.Application) { - Diagnostics.Assert(!string.IsNullOrEmpty(commandInfo.ModuleName), "the module name should exist if commandInfo.Prefix is not an empty string"); - if (!ModuleCmdletBase.IsPrefixedCommand(commandInfo)) - { - completionName = commandInfo.ModuleName + "\\" + completionName; - } + continue; } - } - results.Add(GetCommandNameCompletionResult(completionName, commandList[0], addAmpersandIfNecessary, quote)); + _ = modulesWithCommand.Add(commandInfo.ModuleName); + if (commandInfo.CommandMetadata.CommandType is not null) + { + _ = importedModules.Add(commandInfo.ModuleName); + } + } - // For the other commands that are hidden, we need to disambiguate, - // but put these at the end as it's less likely any of the hidden - // commands are desired. If we can't add anything to disambiguate, - // then we'll skip adding a completion result. - var modulesWithCommand = new HashSet(StringComparer.OrdinalIgnoreCase); - _ = modulesWithCommand.Add(((CommandInfo)commandList[0]).ModuleName); - for (int index = 1; index < commandList.Count; index++) + int moduleCount = modulesWithCommand.Count; + modulesWithCommand.Clear(); + int index; + if (commandInfoArray[0].CommandType == CommandTypes.Application + || importedModules.Count == 1 + || moduleCount < 2) + { + // We can use the short name for this command because there's no ambiguity about which command it resolves to. + index = 1; + results.Add(GetCommandNameCompletionResult(keyValuePair.Key, commandInfoArray[0], addAmpersandIfNecessary, quote)); + _ = modulesWithCommand.Add(commandInfoArray[0].ModuleName); + } + else { - var commandInfo = commandList[index] as CommandInfo; - Diagnostics.Assert(commandInfo != null, "Elements should always be CommandInfo"); + index = 0; + } + for (; index < commandInfoArray.Length; index++) + { + CommandInfo commandInfo = commandInfoArray[index]; if (commandInfo.CommandType == CommandTypes.Application) { - endResults.Add(GetCommandNameCompletionResult(commandInfo.Definition, commandInfo, addAmpersandIfNecessary, quote)); + results.Add(GetCommandNameCompletionResult(commandInfo.Definition, commandInfo, addAmpersandIfNecessary, quote)); } else if (!string.IsNullOrEmpty(commandInfo.ModuleName) && modulesWithCommand.Add(commandInfo.ModuleName)) { var name = commandInfo.ModuleName + "\\" + commandInfo.Name; - endResults.Add(GetCommandNameCompletionResult(name, commandInfo, addAmpersandIfNecessary, quote)); + results.Add(GetCommandNameCompletionResult(name, commandInfo, addAmpersandIfNecessary, quote)); } } } diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 5ecf24d9aad..8b094e73924 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -57,7 +57,7 @@ Describe "TabCompletion" -Tags CI { $Res = TabExpansion2 -inputScript MyTestFunction $Res.CompletionMatches.Count | Should -Be 2 - $Res.CompletionMatches[0].CompletionText | Should -Be "MyTestFunction" + $Res.CompletionMatches[0].CompletionText | Should -Be "TestModule1\MyTestFunction" $Res.CompletionMatches[1].CompletionText | Should -Be "TestModule2\MyTestFunction" } finally From bf99aa9529f4ed17c26097a0b483640eea7c6157 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Thu, 14 Mar 2024 22:14:16 +0100 Subject: [PATCH 4/9] Fix test and crash when encountering aliases. --- .../engine/CommandCompletion/CompletionCompleters.cs | 3 ++- test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index bfbe33d79e5..96b8e1f909f 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -346,7 +346,8 @@ internal static List MakeCommandsUnique(IEnumerable } _ = modulesWithCommand.Add(commandInfo.ModuleName); - if (commandInfo.CommandMetadata.CommandType is not null) + if ((commandInfo.CommandType != CommandTypes.Alias && commandInfo.CommandMetadata.CommandType is not null) + || (commandInfo.CommandType == CommandTypes.Alias && commandInfo.Definition is not null)) { _ = importedModules.Add(commandInfo.ModuleName); } diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 8b094e73924..4de58526ecf 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -57,8 +57,9 @@ Describe "TabCompletion" -Tags CI { $Res = TabExpansion2 -inputScript MyTestFunction $Res.CompletionMatches.Count | Should -Be 2 - $Res.CompletionMatches[0].CompletionText | Should -Be "TestModule1\MyTestFunction" - $Res.CompletionMatches[1].CompletionText | Should -Be "TestModule2\MyTestFunction" + $SortedMatches = $Res.CompletionMatches.CompletionText | Sort-Object + $SortedMatches[0] | Should -Be "TestModule1\MyTestFunction" + $SortedMatches[1] | Should -Be "TestModule2\MyTestFunction" } finally { From b4a7091ffd607d5baface8047e7d3bcfb03ca321 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sat, 15 Jun 2024 20:13:32 +0200 Subject: [PATCH 5/9] Apply suggestion --- .../Host/TabCompletion/TabCompletion.Tests.ps1 | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 4de58526ecf..30d743a03cd 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -46,15 +46,7 @@ Describe "TabCompletion" -Tags CI { New-ModuleManifest -Path "$($NewDir.FullName)\$ModuleName.psd1" -RootModule "$ModuleName.psm1" -FunctionsToExport "MyTestFunction" -ModuleVersion $NewDir.Name } - if ($IsWindows) - { - $env:PSModulePath += ";$tempDir" - } - else - { - $env:PSModulePath += ":$tempDir" - } - + $env:PSModulePath += [System.IO.Path]::PathSeparator + $tempDir $Res = TabExpansion2 -inputScript MyTestFunction $Res.CompletionMatches.Count | Should -Be 2 $SortedMatches = $Res.CompletionMatches.CompletionText | Sort-Object From 597cb542637fbc9be84be62808d44971b178cd54 Mon Sep 17 00:00:00 2001 From: MartinGC94 <42123497+MartinGC94@users.noreply.github.com> Date: Tue, 18 Jun 2024 10:54:09 +0200 Subject: [PATCH 6/9] Apply suggestions from code review Co-authored-by: Dongbo Wang --- .../engine/CommandCompletion/CompletionCompleters.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index ac65fc08652..9aae5c9f079 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -336,7 +336,7 @@ internal static List MakeCommandsUnique(IEnumerable var modulesWithCommand = new HashSet(StringComparer.OrdinalIgnoreCase); var importedModules = new HashSet(StringComparer.OrdinalIgnoreCase); var commandInfoArray = new CommandInfo[commandList.Count]; - for (int i = 0; i < commandInfoArray.Length; i++) + for (int i = 0; i < commandList.Count; i++) { var commandInfo = (CommandInfo)commandList[i]; commandInfoArray[i] = commandInfo; @@ -345,7 +345,7 @@ internal static List MakeCommandsUnique(IEnumerable continue; } - _ = modulesWithCommand.Add(commandInfo.ModuleName); + modulesWithCommand.Add(commandInfo.ModuleName); if ((commandInfo.CommandType != CommandTypes.Alias && commandInfo.CommandMetadata.CommandType is not null) || (commandInfo.CommandType == CommandTypes.Alias && commandInfo.Definition is not null)) { From 81a35f61d7bb863758addaaa7a937b172c884618 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Tue, 18 Jun 2024 15:59:07 +0200 Subject: [PATCH 7/9] Update check. Add comments. Remove unused list. --- .../CommandCompletion/CompletionCompleters.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 9aae5c9f079..b5747120187 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -328,7 +328,6 @@ internal static List MakeCommandsUnique(IEnumerable } } - List endResults = null; foreach (var keyValuePair in commandTable) { if (keyValuePair.Value is List commandList) @@ -346,9 +345,11 @@ internal static List MakeCommandsUnique(IEnumerable } modulesWithCommand.Add(commandInfo.ModuleName); - if ((commandInfo.CommandType != CommandTypes.Alias && commandInfo.CommandMetadata.CommandType is not null) + if ((commandInfo.CommandType == CommandTypes.Cmdlet && commandInfo.CommandMetadata.CommandType is not null) + || (commandInfo.CommandType is CommandTypes.Function or CommandTypes.Filter && commandInfo.Definition != string.Empty) || (commandInfo.CommandType == CommandTypes.Alias && commandInfo.Definition is not null)) { + // Checks if the command or source module has been imported. _ = importedModules.Add(commandInfo.ModuleName); } } @@ -361,9 +362,12 @@ internal static List MakeCommandsUnique(IEnumerable || moduleCount < 2) { // We can use the short name for this command because there's no ambiguity about which command it resolves to. + // If the first element is an application then we know there's no conflicting commands/aliases (because of the command precedence). + // If there's just 1 module imported then the short name refers to that module (and it will be the first element in the list) + // If there's less than 2 unique modules exporting that command then we can use the short name because it can only refer to that module. index = 1; results.Add(GetCommandNameCompletionResult(keyValuePair.Key, commandInfoArray[0], addAmpersandIfNecessary, quote)); - _ = modulesWithCommand.Add(commandInfoArray[0].ModuleName); + modulesWithCommand.Add(commandInfoArray[0].ModuleName); } else { @@ -407,11 +411,6 @@ internal static List MakeCommandsUnique(IEnumerable } } - if (endResults != null && endResults.Count > 0) - { - results.AddRange(endResults); - } - return results; } From 24d8707a6c57e182a36e88de4a6ba24a8b0eb868 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Mon, 3 Mar 2025 18:42:49 +0100 Subject: [PATCH 8/9] Remove Remove-Item call --- test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 index 3f64bd5c3a9..fcc1a320da1 100644 --- a/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 +++ b/test/powershell/Host/TabCompletion/TabCompletion.Tests.ps1 @@ -61,10 +61,6 @@ Describe "TabCompletion" -Tags CI { finally { $env:PSModulePath = $OldModulePath - if (Test-Path -LiteralPath $tempDir) - { - Remove-Item $tempDir -Recurse -Force - } } } From 54374cd58160a31516ec3183e9327c8b0ab6a566 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Tue, 4 Mar 2025 12:15:09 +0100 Subject: [PATCH 9/9] Handle theoretical scenario where it's not a CommandInfo --- .../CommandCompletion/CompletionCompleters.cs | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs index 721ca0de847..bd43875c0da 100644 --- a/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs +++ b/src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs @@ -335,11 +335,15 @@ internal static List MakeCommandsUnique(IEnumerable { var modulesWithCommand = new HashSet(StringComparer.OrdinalIgnoreCase); var importedModules = new HashSet(StringComparer.OrdinalIgnoreCase); - var commandInfoArray = new CommandInfo[commandList.Count]; + var commandInfoList = new List(commandList.Count); for (int i = 0; i < commandList.Count; i++) { - var commandInfo = (CommandInfo)commandList[i]; - commandInfoArray[i] = commandInfo; + if (commandList[i] is not CommandInfo commandInfo) + { + continue; + } + + commandInfoList.Add(commandInfo); if (commandInfo.CommandType == CommandTypes.Application) { continue; @@ -355,10 +359,15 @@ internal static List MakeCommandsUnique(IEnumerable } } + if (commandInfoList.Count == 0) + { + continue; + } + int moduleCount = modulesWithCommand.Count; modulesWithCommand.Clear(); int index; - if (commandInfoArray[0].CommandType == CommandTypes.Application + if (commandInfoList[0].CommandType == CommandTypes.Application || importedModules.Count == 1 || moduleCount < 2) { @@ -367,17 +376,17 @@ internal static List MakeCommandsUnique(IEnumerable // If there's just 1 module imported then the short name refers to that module (and it will be the first element in the list) // If there's less than 2 unique modules exporting that command then we can use the short name because it can only refer to that module. index = 1; - results.Add(GetCommandNameCompletionResult(keyValuePair.Key, commandInfoArray[0], addAmpersandIfNecessary, quote)); - modulesWithCommand.Add(commandInfoArray[0].ModuleName); + results.Add(GetCommandNameCompletionResult(keyValuePair.Key, commandInfoList[0], addAmpersandIfNecessary, quote)); + modulesWithCommand.Add(commandInfoList[0].ModuleName); } else { index = 0; } - for (; index < commandInfoArray.Length; index++) + for (; index < commandInfoList.Count; index++) { - CommandInfo commandInfo = commandInfoArray[index]; + CommandInfo commandInfo = commandInfoList[index]; if (commandInfo.CommandType == CommandTypes.Application) { results.Add(GetCommandNameCompletionResult(commandInfo.Definition, commandInfo, addAmpersandIfNecessary, quote));