Skip to content
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

Do not use alias name as key for command info cache to fix the problem where UseCorrectCasing corrects aliases #1216

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Do not use alias name as key for command info cache to fix the problem where UseCorrectCasing corrects aliases #1216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0bb71d
Do not use alias name as key for command info cache to fix the proble…
bergmeister Apr 13, 2019
68255cc
Remove unused aliasName parameter and left-over GetCommandInfoInterna…
bergmeister Apr 15, 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
9 changes: 3 additions & 6 deletions Engine/CommandInfoCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,16 @@ public CommandInfoCache(Helper pssaHelperInstance)
/// Retrieve a command info object about a command.
/// </summary>
/// <param name="commandName">Name of the command to get a commandinfo object for.</param>
/// <param name="aliasName">The alias of the command to be used in the cache key. If not given, uses the command name.</param>
/// <param name="commandTypes">What types of command are needed. If omitted, all types are retrieved.</param>
/// <returns></returns>
public CommandInfo GetCommandInfo(string commandName, string aliasName = null, CommandTypes? commandTypes = null)
public CommandInfo GetCommandInfo(string commandName, CommandTypes? commandTypes = null)
{
if (string.IsNullOrWhiteSpace(commandName))
{
return null;
}

// If alias name is given, we store the entry under that, but search with the command name
var key = new CommandLookupKey(aliasName ?? commandName, commandTypes);

var key = new CommandLookupKey(commandName, commandTypes);
Copy link
Contributor

Choose a reason for hiding this comment

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

If aliasName is unused, maybe time to remove it from the parameters?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, done, I also removed the left-over GetCommandInfoInternal function that we moved into the CommandInfoCache a few weeks ago but forgot to delete.
I plan to do another PR with more general tidy up later btw.

// Atomically either use PowerShell to query a command info object, or fetch it from the cache
return _commandInfoCache.GetOrAdd(key, new Lazy<CommandInfo>(() => GetCommandInfoInternal(commandName, commandTypes))).Value;
}
Expand All @@ -60,7 +57,7 @@ public CommandInfo GetCommandInfoLegacy(string commandOrAliasName, CommandTypes?

return string.IsNullOrEmpty(commandName)
? GetCommandInfo(commandOrAliasName, commandTypes: commandTypes)
: GetCommandInfo(commandName, aliasName: commandOrAliasName, commandTypes: commandTypes);
: GetCommandInfo(commandName, commandTypes: commandTypes);
}

/// <summary>
Expand Down
27 changes: 1 addition & 26 deletions Engine/Helper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,7 @@ public HashSet<string> GetExportedFunction(Ast ast)
IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);

CommandInfo exportMM = Helper.Instance.GetCommandInfoLegacy("export-modulemember", CommandTypes.Cmdlet);
CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet);

// switch parameters
IEnumerable<ParameterMetadata> switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter) : Enumerable.Empty<ParameterMetadata>();
Expand Down Expand Up @@ -661,31 +661,6 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanTwoPositiona
return moreThanTwoPositional ? argumentsWithoutProcedingParameters > 2 : argumentsWithoutProcedingParameters > 0;
}


/// <summary>
/// Get a CommandInfo object of the given command name
/// </summary>
/// <returns>Returns null if command does not exists</returns>
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
{
using (var ps = System.Management.Automation.PowerShell.Create())
{
var psCommand = ps.AddCommand("Get-Command")
.AddParameter("Name", cmdName)
.AddParameter("ErrorAction", "SilentlyContinue");

if(commandType!=null)
{
psCommand.AddParameter("CommandType", commandType);
}

var commandInfo = psCommand.Invoke<CommandInfo>()
.FirstOrDefault();

return commandInfo;
}
}

/// <summary>
/// Legacy method, new callers should use <see cref="GetCommandInfo"/> instead.
/// Given a command's name, checks whether it exists. It does not use the passed in CommandTypes parameter, which is a bug.
Expand Down