Skip to content

Commit cc41f6d

Browse files
author
Kapil Borle
authored
Merge pull request #692 from PowerShell/kapilmb/FixPerformance
Fix performance related issues
2 parents cab1dcb + 8726c10 commit cc41f6d

File tree

3 files changed

+76
-66
lines changed

3 files changed

+76
-66
lines changed

Engine/Helper.cs

+70-35
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public class Helper
3838
private readonly static Version minSupportedPSVersion = new Version(3, 0);
3939
private Dictionary<string, Dictionary<string, object>> ruleArguments;
4040
private PSVersionTable psVersionTable;
41+
private Dictionary<string, CommandInfo> commandInfoCache;
4142

4243
#endregion
4344

@@ -146,6 +147,7 @@ public void Initialize()
146147
KeywordBlockDictionary = new Dictionary<String, List<Tuple<int, int>>>(StringComparer.OrdinalIgnoreCase);
147148
VariableAnalysisDictionary = new Dictionary<Ast, VariableAnalysis>();
148149
ruleArguments = new Dictionary<string, Dictionary<string, object>>(StringComparer.OrdinalIgnoreCase);
150+
commandInfoCache = new Dictionary<string, CommandInfo>(StringComparer.OrdinalIgnoreCase);
149151

150152
IEnumerable<CommandInfo> aliases = this.invokeCommand.GetCommands("*", CommandTypes.Alias, true);
151153

@@ -594,12 +596,13 @@ public string VariableNameWithoutScope(VariablePath variablePath)
594596
/// <returns></returns>
595597
public bool HasSplattedVariable(CommandAst cmdAst)
596598
{
597-
if (cmdAst == null || cmdAst.CommandElements == null)
598-
{
599-
return false;
600-
}
601-
602-
return cmdAst.CommandElements.Any(cmdElem => cmdElem is VariableExpressionAst && (cmdElem as VariableExpressionAst).Splatted);
599+
return cmdAst != null
600+
&& cmdAst.CommandElements != null
601+
&& cmdAst.CommandElements.Any(cmdElem =>
602+
{
603+
var varExprAst = cmdElem as VariableExpressionAst;
604+
return varExprAst != null && varExprAst.Splatted;
605+
});
603606
}
604607

605608
/// <summary>
@@ -610,12 +613,16 @@ public bool HasSplattedVariable(CommandAst cmdAst)
610613
/// <returns></returns>
611614
public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositional = false)
612615
{
613-
if (cmdAst == null || cmdAst.GetCommandName() == null)
616+
if (cmdAst == null)
614617
{
615618
return false;
616619
}
617620

618-
CommandInfo commandInfo = GetCommandInfo(GetCmdletNameFromAlias(cmdAst.GetCommandName())) ?? GetCommandInfo(cmdAst.GetCommandName());
621+
var commandInfo = GetCommandInfo(cmdAst.GetCommandName());
622+
if (commandInfo == null || (commandInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
623+
{
624+
return false;
625+
}
619626

620627
IEnumerable<ParameterMetadata> switchParams = null;
621628

@@ -642,29 +649,31 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
642649

643650
foreach (CommandElementAst ceAst in cmdAst.CommandElements)
644651
{
645-
if (ceAst is CommandParameterAst)
652+
var cmdParamAst = ceAst as CommandParameterAst;
653+
if (cmdParamAst != null)
654+
{
655+
// Skip if it's a switch parameter
656+
if (switchParams != null &&
657+
switchParams.Any(
658+
pm => String.Equals(
659+
pm.Name,
660+
cmdParamAst.ParameterName, StringComparison.OrdinalIgnoreCase)))
646661
{
647-
// Skip if it's a switch parameter
648-
if (switchParams != null &&
649-
switchParams.Any(pm => String.Equals(pm.Name, (ceAst as CommandParameterAst).ParameterName, StringComparison.OrdinalIgnoreCase)))
650-
{
651-
continue;
652-
}
653-
662+
continue;
663+
}
654664

655-
parameters += 1;
665+
parameters += 1;
656666

657-
if ((ceAst as CommandParameterAst).Argument != null)
658-
{
659-
arguments += 1;
660-
}
661-
662-
}
663-
else
667+
if (cmdParamAst.Argument != null)
664668
{
665669
arguments += 1;
666670
}
667671

672+
}
673+
else
674+
{
675+
arguments += 1;
676+
}
668677
}
669678

670679
// if not the first element in a pipeline, increase the number of arguments by 1
@@ -685,6 +694,23 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
685694
}
686695

687696

697+
/// <summary>
698+
/// Get a CommandInfo object of the given command name
699+
/// </summary>
700+
/// <returns>Returns null if command does not exists</returns>
701+
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType = null)
702+
{
703+
using (var ps = System.Management.Automation.PowerShell.Create())
704+
{
705+
var cmdInfo = ps.AddCommand("Get-Command")
706+
.AddArgument(cmdName)
707+
.AddParameter("ErrorAction", "SilentlyContinue")
708+
.Invoke<CommandInfo>()
709+
.FirstOrDefault();
710+
return cmdInfo;
711+
}
712+
}
713+
688714
/// <summary>
689715
/// Given a command's name, checks whether it exists
690716
/// </summary>
@@ -693,19 +719,28 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
693719
/// <returns></returns>
694720
public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null)
695721
{
696-
CommandTypes defaultCmdType = CommandTypes.Alias
697-
| CommandTypes.Cmdlet
698-
| CommandTypes.ExternalScript
699-
| CommandTypes.Filter
700-
| CommandTypes.Function
701-
| CommandTypes.Script
702-
| CommandTypes.Workflow;
703-
#if !PSV3
704-
defaultCmdType |= CommandTypes.Configuration;
705-
#endif
722+
if (string.IsNullOrWhiteSpace(name))
723+
{
724+
return null;
725+
}
726+
727+
// check if it is an alias
728+
string cmdletName = Helper.Instance.GetCmdletNameFromAlias(name);
729+
if (string.IsNullOrWhiteSpace(cmdletName))
730+
{
731+
cmdletName = name;
732+
}
733+
706734
lock (getCommandLock)
707735
{
708-
return this.invokeCommand.GetCommand(name, commandType ?? defaultCmdType);
736+
if (commandInfoCache.ContainsKey(cmdletName))
737+
{
738+
return commandInfoCache[cmdletName];
739+
}
740+
741+
var commandInfo = GetCommandInfoInternal(cmdletName, commandType);
742+
commandInfoCache.Add(cmdletName, commandInfo);
743+
return commandInfo;
709744
}
710745
}
711746

Rules/UseCmdletCorrectly.cs

+5-7
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
5555
if (cmdAst.GetCommandName() == null) continue;
5656

5757
// Checks mandatory parameters.
58-
if (!IsMandatoryParameterExisted(cmdAst))
58+
if (!MandatoryParameterExists(cmdAst))
5959
{
6060
yield return new DiagnosticRecord(String.Format(CultureInfo.CurrentCulture, Strings.UseCmdletCorrectlyError, cmdAst.GetCommandName()),
6161
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
@@ -68,7 +68,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
6868
/// </summary>
6969
/// <param name="cmdAst"></param>
7070
/// <returns></returns>
71-
private bool IsMandatoryParameterExisted(CommandAst cmdAst)
71+
private bool MandatoryParameterExists(CommandAst cmdAst)
7272
{
7373
CommandInfo cmdInfo = null;
7474
List<ParameterMetadata> mandParams = new List<ParameterMetadata>();
@@ -88,9 +88,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)
8888

8989
#region Compares parameter list and mandatory parameter list.
9090

91-
cmdInfo = Helper.Instance.GetCommandInfo(Helper.Instance.GetCmdletNameFromAlias(cmdAst.GetCommandName()))
92-
?? Helper.Instance.GetCommandInfo(cmdAst.GetCommandName());
93-
91+
cmdInfo = Helper.Instance.GetCommandInfo(cmdAst.GetCommandName());
9492
if (cmdInfo == null || (cmdInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
9593
{
9694
return true;
@@ -109,7 +107,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)
109107
// If cannot find any mandatory parameter, it's not necessary to do a further check for current cmdlet.
110108
try
111109
{
112-
int noOfParamSets = cmdInfo.ParameterSets.Count;
110+
int noOfParamSets = cmdInfo.ParameterSets.Count;
113111
foreach (ParameterMetadata pm in cmdInfo.Parameters.Values)
114112
{
115113
int count = 0;
@@ -163,7 +161,7 @@ private bool IsMandatoryParameterExisted(CommandAst cmdAst)
163161

164162
return returnValue;
165163
}
166-
164+
167165
/// <summary>
168166
/// GetName: Retrieves the name of this rule.
169167
/// </summary>

Rules/UseShouldProcessCorrectly.cs

+1-24
Original file line numberDiff line numberDiff line change
@@ -297,29 +297,6 @@ private bool DeclaresSupportsShouldProcess(FunctionDefinitionAst ast)
297297
return Helper.Instance.GetNamedArgumentAttributeValue(shouldProcessAttribute);
298298
}
299299

300-
/// <summary>
301-
/// Get a CommandInfo object of the given command name
302-
/// </summary>
303-
/// <returns>Returns null if command does not exists</returns>
304-
private CommandInfo GetCommandInfo(string cmdName)
305-
{
306-
try
307-
{
308-
using (var ps = System.Management.Automation.PowerShell.Create())
309-
{
310-
var cmdInfo = ps.AddCommand("Get-Command")
311-
.AddArgument(cmdName)
312-
.Invoke<CommandInfo>()
313-
.FirstOrDefault();
314-
return cmdInfo;
315-
}
316-
}
317-
catch (System.Management.Automation.CommandNotFoundException)
318-
{
319-
return null;
320-
}
321-
}
322-
323300
/// <summary>
324301
/// Checks if the given command supports ShouldProcess
325302
/// </summary>
@@ -331,7 +308,7 @@ private bool SupportsShouldProcess(string cmdName)
331308
return false;
332309
}
333310

334-
var cmdInfo = GetCommandInfo(cmdName);
311+
var cmdInfo = Helper.Instance.GetCommandInfo(cmdName);
335312
if (cmdInfo == null)
336313
{
337314
return false;

0 commit comments

Comments
 (0)