Skip to content

Get-Command: Specifying an alias and -Syntax returns the aliased commands syntax #10784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adityapatwardhan merged 4 commits into from
Apr 13, 2020
Merged

Get-Command: Specifying an alias and -Syntax returns the aliased commands syntax #10784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
06bf207
Add additional check for alias and syntax switch
ChrisLGardner Oct 13, 2019
29dab33
Add support for aliases to script files returning the correct syntax …
ChrisLGardner Oct 14, 2019
7fdb4d0
Refactor output creation to its own method to reduce complexity withi…
ChrisLGardner Oct 17, 2019
891092d
Add check for null resolved name and lookup actual command info objec…
ChrisLGardner Dec 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 102 additions & 25 deletions src/System.Management.Automation/engine/GetCommandCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,9 +531,7 @@ private void OutputResultsHelper(IEnumerable<CommandInfo> results)
{
if (!string.IsNullOrEmpty(result.Syntax))
{
PSObject syntax = PSObject.AsPSObject(result.Syntax);

syntax.IsHelpObject = true;
PSObject syntax = GetSyntaxObject(result);

WriteObject(syntax);
}
Expand Down Expand Up @@ -571,6 +569,81 @@ private void OutputResultsHelper(IEnumerable<CommandInfo> results)
#endif
}

/// <summary>
/// Creates the syntax output based on if the command is an alias, script, application or command.
/// </summary>
/// <param name="command">
/// CommandInfo object containing the syntax to be output.
/// </param>
/// <returns>
/// Syntax string cast as a PSObject for outputting.
/// </returns>
private PSObject GetSyntaxObject(CommandInfo command)
{
PSObject syntax = PSObject.AsPSObject(command.Syntax);

// This is checking if the command name that's been passed in is one that was specified by a user,
// if not then we have to assume they specified an alias or a wildcard and do some extra formatting for those,
// if it is then just go with the default formatting.
// So if a user runs Get-Command -Name del -Syntax the code will find del and the command it resolves to as Remove-Item
// and attempt to return that, but as the user specified del we want to fiddle with the output a bit to make it clear
// that's an alias but still give the Remove-Item syntax.
if (this.Name != null && !Array.Exists(this.Name, name => name.Equals(command.Name, StringComparison.InvariantCultureIgnoreCase)))
{
string aliasName = _nameContainsWildcard ? command.Name : this.Name[0];

IDictionary<string, AliasInfo> aliasTable = SessionState.Internal.GetAliasTable();
foreach (KeyValuePair<string, AliasInfo> tableEntry in aliasTable)
{
if ((Array.Exists(this.Name, name => name.Equals(tableEntry.Key, StringComparison.InvariantCultureIgnoreCase)) &&
tableEntry.Value.Definition == command.Name) ||
(_nameContainsWildcard && tableEntry.Value.Definition == command.Name))
{
aliasName = tableEntry.Key;
break;
}
}

string replacedSyntax = string.Empty;
switch (command)
{
case ExternalScriptInfo externalScript:
replacedSyntax = string.Format(
"{0} (alias) -> {1}{2}{3}",
aliasName,
string.Format("{0}{1}", externalScript.Path, Environment.NewLine),
Environment.NewLine,
command.Syntax.Replace(command.Name, aliasName));
break;
case ApplicationInfo app:
replacedSyntax = app.Path;
break;
default:
if (aliasName.Equals(command.Name))
{
replacedSyntax = command.Syntax;
}
else
{
replacedSyntax = string.Format(
"{0} (alias) -> {1}{2}{3}",
aliasName,
command.Name,
Environment.NewLine,
command.Syntax.Replace(command.Name, aliasName));
}

break;
}

syntax = PSObject.AsPSObject(replacedSyntax);
}

syntax.IsHelpObject = true;

return syntax;
}

/// <summary>
/// The comparer to sort CommandInfo objects in the result list.
/// </summary>
Expand Down Expand Up @@ -1229,33 +1302,37 @@ private bool IsCommandMatch(ref CommandInfo current, out bool isDuplicate)

if (isCommandMatch)
{
if (ArgumentList != null)
if (Syntax.IsPresent && current is AliasInfo ai)
{
AliasInfo ai = current as AliasInfo;
if (ai != null)
// If the matching command was an alias, then use the resolved command
// instead of the alias...
current = ai.ResolvedCommand ?? CommandDiscovery.LookupCommandInfo(
ai.UnresolvedCommandName,
this.MyInvocation.CommandOrigin,
this.Context);

// there are situations where both ResolvedCommand and UnresolvedCommandName
// are both null (often due to multiple versions of modules with aliases)
// therefore we need to exit early.
if (current == null)
Copy link
Member

@adityapatwardhan adityapatwardhan Dec 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this will be dead code now? Is that correct?

Copy link
Member

@adityapatwardhan adityapatwardhan Mar 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisLGardner ping..

Copy link
Contributor Author

@ChrisLGardner ChrisLGardner Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, doing some testing and it's there to resolve the command name if the module the alias is in hasn't been loaded yet and then that can be used to get the syntax. Without it you get the below behaviour:

PS> gcm del -syn
Remove-Item

with it you get this:

PS>  gcm del -syn
del (alias) -> Remove-Item

del [-Path] <string[]> [-Filter <string>] [-Include <string[]>] [-Exclude <string[]>] [-Recurse] [-Force] [-Credential <pscredential>] 

del -LiteralPath <string[]> [-Filter <string>] [-Include <string[]>] [-Exclude <string[]>] [-Recurse] [-Force] [-Credential <pscredential>] [-WhatIf] [-Confirm] [-Stream <string[]>] [<CommonParameters>]

Copy link
Collaborator

@vexx32 vexx32 Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ChrisLGardner I think @adityapatwardhan is referring to the line 1312 if (current == null) code path, which shouldn't be reached anymore since you're checking for null and accounting for that directly. If whatever's in that code path isn't reachable anymore, it should be removed. 🙂

Also, this condition with the null can be simplified:

current = ai.ResolvedCommand ?? CommandDiscovery.LookupCommandInfo(
    ai.UnresolvedCommandName,
    this.MyInvocation.CommandOrigin,
    this.Context);

Copy link
Contributor Author

@ChrisLGardner ChrisLGardner Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll get that fixed now.

Copy link
Contributor Author

@ChrisLGardner ChrisLGardner Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at the failure in the tests there's a reason that check is there. It can sometimes return null from LookupCommandInfo. In the case for the tests (and locally) it is because spvm has a null ResolvedCommand and UnresolvedCommandName, probably in my case because I've apparently got both 1.1 and 2.0.0.0 of Hyper-V installed and I'd guess the AzDOS agents are similar.

Copy link
Collaborator

@vexx32 vexx32 Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to add a comment explaining that so nobody tries to remove that later 🙂

{
// If the matching command was an alias, then use the resolved command
// instead of the alias...
current = ai.ResolvedCommand;
if (current == null)
{
return false;
}
}
else if (!(current is CmdletInfo || current is IScriptCommandInfo))
{
// If current is not a cmdlet or script, we need to throw a terminating error.
ThrowTerminatingError(
new ErrorRecord(
PSTraceSource.NewArgumentException(
"ArgumentList",
DiscoveryExceptions.CommandArgsOnlyForSingleCmdlet),
"CommandArgsOnlyForSingleCmdlet",
ErrorCategory.InvalidArgument,
current));
return false;
}
}

if (ArgumentList != null && !(current is CmdletInfo || current is IScriptCommandInfo))
{
// If current is not a cmdlet or script, we need to throw a terminating error.
ThrowTerminatingError(
new ErrorRecord(
PSTraceSource.NewArgumentException(
"ArgumentList",
DiscoveryExceptions.CommandArgsOnlyForSingleCmdlet),
"CommandArgsOnlyForSingleCmdlet",
ErrorCategory.InvalidArgument,
current));
}

// If the command implements dynamic parameters
// then we must make a copy of the CommandInfo which merges the
// dynamic parameter metadata with the statically defined parameter
Expand Down
77 changes: 77 additions & 0 deletions test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Command.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,80 @@ Describe "Get-Command Feature tests" -Tag Feature {
}
}
}

Describe "Get-Command" -Tag CI {
BeforeAll {
Import-Module Microsoft.PowerShell.Management
}
Context "-Syntax tests" {
Copy link
Collaborator

@iSazonov iSazonov Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the tests reliable we need to load Microsoft.PowerShell.Management module.
Please add this in BeforeAll block

Copy link
Contributor Author

@ChrisLGardner ChrisLGardner Oct 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've added it in the right place, I couldn't see any other examples of how to do it so figured a simple Import-Module Microsoft.PowerShell.Management would be enough.

It "Should return a string object when -Name is an alias and -Syntax is specified" {
$Result = Get-Command -Name del -Syntax

$Result | Should -BeOfType [String]
$Result | Should -Match 'del \[-Path\]'
}

It "Should replace commands with aliases in matching commands when using a wildcard search" {
$Result = Get-Command -Name sp* -Syntax

$Result | Should -BeOfType [String]
$Result -join '' | Should -Match 'sp \(alias\) -> Set-ItemProperty'
$Result -join '' | Should -Match 'sp \[-Path\]'
}

It "Should not add the alias (alias) -> command decorator for non-alias commands" {
$Result = Get-Command -Name sp* -Syntax

$Result -join '' | Should -Not -Match 'Split-Path \(alias\) -> Split-Path'
$Result -join '' | Should -Match 'Split-Path \[-Path\]'
}

It "Should only replace aliases when given multiple entries including a command and an alias" {
$Result = Get-Command -Name get-help, del -Syntax

$Result -join '' | Should -Match 'del \(alias\) -> Remove-Item'
$Result -join '' | Should -Match 'del \[-Path\]'
$Result -join '' | Should -Match 'Get-Help \[\[-Name\]'
$Result -join '' | Should -Not -Match 'del \(alias\) -> Get-Help'
}

It "Should return the path to an aliased script when -Syntax is specified" {
# First, create a script file
$TestGcmSyntax = @'
[CmdletBinding()]
param(
[Parameter(Position=0, Mandatory, ValueFromPipeline, ValueFromPipelineByPropertyName)]
[ValidateNotNullOrEmpty()]
[string[]]
$Name
)
process {
"Processing ${Name}"
}
'@
Set-Content -Path TestDrive:\Test-GcmSyntax.ps1 -Value $TestGcmSyntax

# Now set up an alias for that file
New-Alias -Name tgs -Value TestDrive:\Test-GcmSyntax.ps1

$Result = Get-Command -Name tgs -Syntax

$Result | Should -Match "tgs \(alias\) -> $([Regex]::Escape((Get-Item TestDrive:\\Test-GcmSyntax.ps1).FullName))"
}
}

Context "-Name tests" {
It "Should return a AliasInfo object when -Name is an alias" {
$Result = Get-Command -Name del

$Result | Should -BeOfType [System.Management.Automation.AliasInfo]
$Result.DisplayName | Should -Be 'del -> Remove-Item'
}

It "Should return a CommandInfo object when -Name is a command" {
$Result = Get-Command -Name Remove-Item

$Result | Should -BeOfType [System.Management.Automation.CommandInfo]
}
}
}