Skip to content

Commit 81bdc15

Browse files
committed
Merge branch 'development' of https://github.com/PowerShell/PSScriptAnalyzer into Parser
2 parents 7403578 + 6a8e828 commit 81bdc15

19 files changed

+190
-65
lines changed

Engine/Helper.cs

Lines changed: 51 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,7 @@ public HashSet<string> GetExportedFunction(Ast ast)
399399
IEnumerable<Ast> cmdAsts = ast.FindAll(item => item is CommandAst
400400
&& exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true);
401401

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

404404
// switch parameters
405405
IEnumerable<ParameterMetadata> switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where<ParameterMetadata>(pm => pm.SwitchParameter) : Enumerable.Empty<ParameterMetadata>();
@@ -614,7 +614,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
614614
return false;
615615
}
616616

617-
var commandInfo = GetCommandInfo(cmdAst.GetCommandName());
617+
var commandInfo = GetCommandInfoLegacy(cmdAst.GetCommandName());
618618
if (commandInfo == null || (commandInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
619619
{
620620
return false;
@@ -694,26 +694,38 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio
694694
/// Get a CommandInfo object of the given command name
695695
/// </summary>
696696
/// <returns>Returns null if command does not exists</returns>
697-
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType = null)
697+
private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType)
698698
{
699699
using (var ps = System.Management.Automation.PowerShell.Create())
700700
{
701-
var cmdInfo = ps.AddCommand("Get-Command")
702-
.AddArgument(cmdName)
703-
.AddParameter("ErrorAction", "SilentlyContinue")
704-
.Invoke<CommandInfo>()
705-
.FirstOrDefault();
706-
return cmdInfo;
701+
var psCommand = ps.AddCommand("Get-Command")
702+
.AddParameter("Name", cmdName)
703+
.AddParameter("ErrorAction", "SilentlyContinue");
704+
705+
if(commandType!=null)
706+
{
707+
psCommand.AddParameter("CommandType", commandType);
708+
}
709+
710+
var commandInfo = psCommand.Invoke<CommandInfo>()
711+
.FirstOrDefault();
712+
713+
return commandInfo;
707714
}
708715
}
709716

710717
/// <summary>
711-
/// Given a command's name, checks whether it exists
718+
719+
/// Legacy method, new callers should use <see cref="GetCommandInfo"/> instead.
720+
/// Given a command's name, checks whether it exists. It does not use the passed in CommandTypes parameter, which is a bug.
721+
/// But existing method callers are already depending on this behaviour and therefore this could not be simply fixed.
722+
/// It also populates the commandInfoCache which can have side effects in some cases.
712723
/// </summary>
713-
/// <param name="name"></param>
714-
/// <param name="commandType"></param>
724+
/// <param name="name">Command Name.</param>
725+
/// <param name="commandType">Not being used.</param>
715726
/// <returns></returns>
716-
public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null)
727+
[Obsolete]
728+
public CommandInfo GetCommandInfoLegacy(string name, CommandTypes? commandType = null)
717729
{
718730
if (string.IsNullOrWhiteSpace(name))
719731
{
@@ -740,6 +752,32 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null)
740752
}
741753
}
742754

755+
/// <summary>
756+
/// Given a command's name, checks whether it exists.
757+
/// </summary>
758+
/// <param name="name"></param>
759+
/// <param name="commandType"></param>
760+
/// <returns></returns>
761+
public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null)
762+
{
763+
if (string.IsNullOrWhiteSpace(name))
764+
{
765+
return null;
766+
}
767+
768+
lock (getCommandLock)
769+
{
770+
if (commandInfoCache.ContainsKey(name))
771+
{
772+
return commandInfoCache[name];
773+
}
774+
775+
var commandInfo = GetCommandInfoInternal(name, commandType);
776+
777+
return commandInfo;
778+
}
779+
}
780+
743781
/// <summary>
744782
/// Returns the get, set and test targetresource dsc function
745783
/// </summary>

Engine/Settings.cs

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -690,40 +690,42 @@ internal static SettingsMode FindSettingsMode(object settings, string path, out
690690
}
691691
else
692692
{
693-
var settingsFilePath = settingsFound as String;
694-
if (settingsFilePath != null)
695-
{
696-
if (IsBuiltinSettingPreset(settingsFilePath))
697-
{
698-
settingsMode = SettingsMode.Preset;
699-
settingsFound = GetSettingPresetFilePath(settingsFilePath);
700-
}
701-
else
702-
{
703-
settingsMode = SettingsMode.File;
704-
settingsFound = settingsFilePath;
705-
}
706-
}
707-
else
693+
if (!TryResolveSettingForStringType(settingsFound, ref settingsMode, ref settingsFound))
708694
{
709695
if (settingsFound is Hashtable)
710696
{
711697
settingsMode = SettingsMode.Hashtable;
712698
}
713-
else // if the provided argument is wrapped in an expressions then PowerShell resolves it but it will be of type PSObject and we have to operate then on the BaseObject
699+
// if the provided argument is wrapped in an expressions then PowerShell resolves it but it will be of type PSObject and we have to operate then on the BaseObject
700+
else if (settingsFound is PSObject settingsFoundPSObject)
714701
{
715-
if (settingsFound is PSObject settingsFoundPSObject)
716-
{
717-
if (settingsFoundPSObject.BaseObject is String)
718-
{
719-
settingsMode = SettingsMode.File;
720-
}
721-
}
702+
TryResolveSettingForStringType(settingsFoundPSObject.BaseObject, ref settingsMode, ref settingsFound);
722703
}
723704
}
724705
}
725706

726707
return settingsMode;
727708
}
709+
710+
// If the settings object is a string determine wheter it is one of the settings preset or a file path and resolve the setting in the former case.
711+
private static bool TryResolveSettingForStringType(object settingsObject, ref SettingsMode settingsMode, ref object resolvedSettingValue)
712+
{
713+
if (settingsObject is string settingsString)
714+
{
715+
if (IsBuiltinSettingPreset(settingsString))
716+
{
717+
settingsMode = SettingsMode.Preset;
718+
resolvedSettingValue = GetSettingPresetFilePath(settingsString);
719+
}
720+
else
721+
{
722+
settingsMode = SettingsMode.File;
723+
resolvedSettingValue = settingsString;
724+
}
725+
return true;
726+
}
727+
728+
return false;
729+
}
728730
}
729731
}

RuleDocumentation/AvoidUsingCmdletAliases.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
## Description
66

77
An alias is an alternate name or nickname for a CMDLet or for a command element, such as a function, script, file, or executable file.
8-
You can use the alias instead of the command name in any Windows PowerShell commands.
8+
You can use the alias instead of the command name in any Windows PowerShell commands. There are also implicit aliases: When PowerShell cannot find the cmdlet name, it will try to append `Get-` to the command as a last resort before, therefore e.g. `verb` will excute `Get-Verb`.
99

1010
Every PowerShell author learns the actual command names, but different authors learn and use different aliases. Aliases can make code difficult to read, understand and
1111
impact availability.

RuleDocumentation/UseConsistentWhitespace.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ Checks if there is space between a keyword and its corresponding open parenthesi
3838

3939
#### CheckOperator: bool (Default value is `$true`)
4040

41-
Checks if a binary operator is surrounded on both sides by a space. E.g. `$x = 1`.
41+
Checks if a binary or unary operator is surrounded on both sides by a space. E.g. `$x = 1`.
4242

4343
#### CheckSeparator: bool (Default value is `$true`)
4444

Rules/AvoidAlias.cs

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#endif
1111
using System.Globalization;
1212
using System.Linq;
13+
using System.Management.Automation;
1314

1415
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
1516
{
@@ -95,38 +96,54 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
9596
IEnumerable<Ast> foundAsts = ast.FindAll(testAst => testAst is CommandAst, true);
9697

9798
// Iterates all CommandAsts and check the command name.
98-
foreach (Ast foundAst in foundAsts)
99+
foreach (CommandAst cmdAst in foundAsts)
99100
{
100-
CommandAst cmdAst = (CommandAst)foundAst;
101-
102101
// Check if the command ast should be ignored
103102
if (IgnoreCommandast(cmdAst))
104103
{
105104
continue;
106105
}
107106

108-
string aliasName = cmdAst.GetCommandName();
107+
string commandName = cmdAst.GetCommandName();
109108

110109
// Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}.
111110
// You can also review the remark section in following document,
112111
// MSDN: CommandAst.GetCommandName Method
113-
if (aliasName == null
114-
|| whiteList.Contains<string>(aliasName, StringComparer.OrdinalIgnoreCase))
112+
if (commandName == null
113+
|| whiteList.Contains<string>(commandName, StringComparer.OrdinalIgnoreCase))
115114
{
116115
continue;
117116
}
118117

119-
string cmdletName = Helper.Instance.GetCmdletNameFromAlias(aliasName);
120-
if (!String.IsNullOrEmpty(cmdletName))
118+
string cmdletNameIfCommandNameWasAlias = Helper.Instance.GetCmdletNameFromAlias(commandName);
119+
if (!String.IsNullOrEmpty(cmdletNameIfCommandNameWasAlias))
121120
{
122121
yield return new DiagnosticRecord(
123-
string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, aliasName, cmdletName),
122+
string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, commandName, cmdletNameIfCommandNameWasAlias),
124123
GetCommandExtent(cmdAst),
125124
GetName(),
126125
DiagnosticSeverity.Warning,
127126
fileName,
128-
aliasName,
129-
suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletName));
127+
commandName,
128+
suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletNameIfCommandNameWasAlias));
129+
}
130+
131+
var isNativeCommand = Helper.Instance.GetCommandInfo(commandName, CommandTypes.Application | CommandTypes.ExternalScript) != null;
132+
if (!isNativeCommand)
133+
{
134+
var commdNameWithGetPrefix = $"Get-{commandName}";
135+
var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo($"Get-{commandName}");
136+
if (cmdletNameIfCommandWasMissingGetPrefix != null)
137+
{
138+
yield return new DiagnosticRecord(
139+
string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesMissingGetPrefixError, commandName, commdNameWithGetPrefix),
140+
GetCommandExtent(cmdAst),
141+
GetName(),
142+
DiagnosticSeverity.Warning,
143+
fileName,
144+
commandName,
145+
suggestedCorrections: GetCorrectionExtent(cmdAst, commdNameWithGetPrefix));
146+
}
130147
}
131148
}
132149
}

Rules/AvoidPositionalParameters.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3939
// MSDN: CommandAst.GetCommandName Method
4040
if (cmdAst.GetCommandName() == null) continue;
4141

42-
if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null
42+
if (Helper.Instance.GetCommandInfoLegacy(cmdAst.GetCommandName()) != null
4343
&& Helper.Instance.PositionalParameterUsed(cmdAst, true))
4444
{
4545
PipelineAst parent = cmdAst.Parent as PipelineAst;

Rules/Strings.Designer.cs

Lines changed: 11 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Rules/Strings.resx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,10 +118,10 @@
118118
<value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
119119
</resheader>
120120
<data name="AvoidUsingCmdletAliasesDescription" xml:space="preserve">
121-
<value>An alias is an alternate name or nickname for a cmdlet or for a command element, such as a function, script, file, or executable file. But when writing scripts that will potentially need to be maintained over time, either by the original author or another Windows PowerShell scripter, please consider using full cmdlet name instead of alias. Aliases can introduce these problems, readability, understandability and availability.</value>
121+
<value>An alias is an alternate name or nickname for a cmdlet or for a command element, such as a function, script, file, or executable file. An implicit alias is also the omission of the 'Get-' prefix for commands with this prefix. But when writing scripts that will potentially need to be maintained over time, either by the original author or another Windows PowerShell scripter, please consider using full cmdlet name instead of alias. Aliases can introduce these problems, readability, understandability and availability.</value>
122122
</data>
123123
<data name="AvoidUsingCmdletAliasesCommonName" xml:space="preserve">
124-
<value>Avoid Using Cmdlet Aliases</value>
124+
<value>Avoid Using Cmdlet Aliases or omitting the 'Get-' prefix.</value>
125125
</data>
126126
<data name="AvoidUsingEmptyCatchBlockDescription" xml:space="preserve">
127127
<value>Empty catch blocks are considered poor design decisions because if an error occurs in the try block, this error is simply swallowed and not acted upon. While this does not inherently lead to bad things. It can and this should be avoided if possible. To fix a violation of this rule, using Write-Error or throw statements in catch blocks.</value>
@@ -1005,4 +1005,7 @@
10051005
<data name="AvoidAssignmentToAutomaticVariableName" xml:space="preserve">
10061006
<value>AvoidAssignmentToAutomaticVariable</value>
10071007
</data>
1008+
<data name="AvoidUsingCmdletAliasesMissingGetPrefixError" xml:space="preserve">
1009+
<value>'{0}' is implicitly aliasing '{1}' because it is missing the 'Get-' prefix. This can introduce possible problems and make scripts hard to maintain. Please consider changing command to its full name.</value>
1010+
</data>
10081011
</root>

Rules/UseCmdletCorrectly.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ private bool MandatoryParameterExists(CommandAst cmdAst)
7979

8080
#region Compares parameter list and mandatory parameter list.
8181

82-
cmdInfo = Helper.Instance.GetCommandInfo(cmdAst.GetCommandName());
82+
cmdInfo = Helper.Instance.GetCommandInfoLegacy(cmdAst.GetCommandName());
8383
if (cmdInfo == null || (cmdInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet))
8484
{
8585
return true;

Rules/UseConsistentWhitespace.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,14 @@ private IEnumerable<DiagnosticRecord> FindOperatorViolations(TokenOperations tok
307307
continue;
308308
}
309309

310+
// exclude unary operator for cases like $foo.bar(-$Var)
311+
if (TokenTraits.HasTrait(tokenNode.Value.Kind, TokenFlags.UnaryOperator) &&
312+
tokenNode.Previous.Value.Kind == TokenKind.LParen &&
313+
tokenNode.Next.Value.Kind == TokenKind.Variable)
314+
{
315+
continue;
316+
}
317+
310318
var hasWhitespaceBefore = IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode);
311319
var hasWhitespaceAfter = tokenNode.Next.Value.Kind == TokenKind.NewLine
312320
|| IsPreviousTokenOnSameLineAndApartByWhitespace(tokenNode.Next);

0 commit comments

Comments
 (0)