diff --git a/Engine/Helper.cs b/Engine/Helper.cs index 9e74d2b4b..4adf407ad 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -399,7 +399,7 @@ public HashSet GetExportedFunction(Ast ast) IEnumerable cmdAsts = ast.FindAll(item => item is CommandAst && exportFunctionsCmdlet.Contains((item as CommandAst).GetCommandName(), StringComparer.OrdinalIgnoreCase), true); - CommandInfo exportMM = Helper.Instance.GetCommandInfo("export-modulemember", CommandTypes.Cmdlet); + CommandInfo exportMM = Helper.Instance.GetCommandInfoLegacy("export-modulemember", CommandTypes.Cmdlet); // switch parameters IEnumerable switchParams = (exportMM != null) ? exportMM.Parameters.Values.Where(pm => pm.SwitchParameter) : Enumerable.Empty(); @@ -614,7 +614,7 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio return false; } - var commandInfo = GetCommandInfo(cmdAst.GetCommandName()); + var commandInfo = GetCommandInfoLegacy(cmdAst.GetCommandName()); if (commandInfo == null || (commandInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet)) { return false; @@ -694,26 +694,38 @@ public bool PositionalParameterUsed(CommandAst cmdAst, bool moreThanThreePositio /// Get a CommandInfo object of the given command name /// /// Returns null if command does not exists - private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType = null) + private CommandInfo GetCommandInfoInternal(string cmdName, CommandTypes? commandType) { using (var ps = System.Management.Automation.PowerShell.Create()) { - var cmdInfo = ps.AddCommand("Get-Command") - .AddArgument(cmdName) - .AddParameter("ErrorAction", "SilentlyContinue") - .Invoke() - .FirstOrDefault(); - return cmdInfo; + var psCommand = ps.AddCommand("Get-Command") + .AddParameter("Name", cmdName) + .AddParameter("ErrorAction", "SilentlyContinue"); + + if(commandType!=null) + { + psCommand.AddParameter("CommandType", commandType); + } + + var commandInfo = psCommand.Invoke() + .FirstOrDefault(); + + return commandInfo; } } /// - /// Given a command's name, checks whether it exists + + /// Legacy method, new callers should use instead. + /// Given a command's name, checks whether it exists. It does not use the passed in CommandTypes parameter, which is a bug. + /// But existing method callers are already depending on this behaviour and therefore this could not be simply fixed. + /// It also populates the commandInfoCache which can have side effects in some cases. /// - /// - /// + /// Command Name. + /// Not being used. /// - public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null) + [Obsolete] + public CommandInfo GetCommandInfoLegacy(string name, CommandTypes? commandType = null) { if (string.IsNullOrWhiteSpace(name)) { @@ -740,6 +752,32 @@ public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null) } } + /// + /// Given a command's name, checks whether it exists. + /// + /// + /// + /// + public CommandInfo GetCommandInfo(string name, CommandTypes? commandType = null) + { + if (string.IsNullOrWhiteSpace(name)) + { + return null; + } + + lock (getCommandLock) + { + if (commandInfoCache.ContainsKey(name)) + { + return commandInfoCache[name]; + } + + var commandInfo = GetCommandInfoInternal(name, commandType); + + return commandInfo; + } + } + /// /// Returns the get, set and test targetresource dsc function /// diff --git a/RuleDocumentation/AvoidUsingCmdletAliases.md b/RuleDocumentation/AvoidUsingCmdletAliases.md index 0165b6d90..39d7437d0 100644 --- a/RuleDocumentation/AvoidUsingCmdletAliases.md +++ b/RuleDocumentation/AvoidUsingCmdletAliases.md @@ -5,7 +5,7 @@ ## Description 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. -You can use the alias instead of the command name in any Windows PowerShell commands. +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`. 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 impact availability. diff --git a/Rules/AvoidAlias.cs b/Rules/AvoidAlias.cs index aa3aff05c..ad35ec4fe 100644 --- a/Rules/AvoidAlias.cs +++ b/Rules/AvoidAlias.cs @@ -10,6 +10,7 @@ #endif using System.Globalization; using System.Linq; +using System.Management.Automation; namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { @@ -95,38 +96,54 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) IEnumerable foundAsts = ast.FindAll(testAst => testAst is CommandAst, true); // Iterates all CommandAsts and check the command name. - foreach (Ast foundAst in foundAsts) + foreach (CommandAst cmdAst in foundAsts) { - CommandAst cmdAst = (CommandAst)foundAst; - // Check if the command ast should be ignored if (IgnoreCommandast(cmdAst)) { continue; } - string aliasName = cmdAst.GetCommandName(); + string commandName = cmdAst.GetCommandName(); // Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}. // You can also review the remark section in following document, // MSDN: CommandAst.GetCommandName Method - if (aliasName == null - || whiteList.Contains(aliasName, StringComparer.OrdinalIgnoreCase)) + if (commandName == null + || whiteList.Contains(commandName, StringComparer.OrdinalIgnoreCase)) { continue; } - string cmdletName = Helper.Instance.GetCmdletNameFromAlias(aliasName); - if (!String.IsNullOrEmpty(cmdletName)) + string cmdletNameIfCommandNameWasAlias = Helper.Instance.GetCmdletNameFromAlias(commandName); + if (!String.IsNullOrEmpty(cmdletNameIfCommandNameWasAlias)) { yield return new DiagnosticRecord( - string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, aliasName, cmdletName), + string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, commandName, cmdletNameIfCommandNameWasAlias), GetCommandExtent(cmdAst), GetName(), DiagnosticSeverity.Warning, fileName, - aliasName, - suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletName)); + commandName, + suggestedCorrections: GetCorrectionExtent(cmdAst, cmdletNameIfCommandNameWasAlias)); + } + + var isNativeCommand = Helper.Instance.GetCommandInfo(commandName, CommandTypes.Application | CommandTypes.ExternalScript) != null; + if (!isNativeCommand) + { + var commdNameWithGetPrefix = $"Get-{commandName}"; + var cmdletNameIfCommandWasMissingGetPrefix = Helper.Instance.GetCommandInfo($"Get-{commandName}"); + if (cmdletNameIfCommandWasMissingGetPrefix != null) + { + yield return new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesMissingGetPrefixError, commandName, commdNameWithGetPrefix), + GetCommandExtent(cmdAst), + GetName(), + DiagnosticSeverity.Warning, + fileName, + commandName, + suggestedCorrections: GetCorrectionExtent(cmdAst, commdNameWithGetPrefix)); + } } } } diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index 60f5cea8f..de530ab78 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -39,7 +39,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) // MSDN: CommandAst.GetCommandName Method if (cmdAst.GetCommandName() == null) continue; - if (Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()) != null + if (Helper.Instance.GetCommandInfoLegacy(cmdAst.GetCommandName()) != null && Helper.Instance.PositionalParameterUsed(cmdAst, true)) { PipelineAst parent = cmdAst.Parent as PipelineAst; diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs index 565e67629..7c0e53ec3 100644 --- a/Rules/Strings.Designer.cs +++ b/Rules/Strings.Designer.cs @@ -710,7 +710,7 @@ internal static string AvoidUsingClearHostName { } /// - /// Looks up a localized string similar to Avoid Using Cmdlet Aliases. + /// Looks up a localized string similar to Avoid Using Cmdlet Aliases or omitting the 'Get-' prefix.. /// internal static string AvoidUsingCmdletAliasesCommonName { get { @@ -728,7 +728,7 @@ internal static string AvoidUsingCmdletAliasesCorrectionDescription { } /// - /// Looks up a localized string similar to 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.. + /// Looks up a localized string similar to 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 availa [rest of string was truncated]";. /// internal static string AvoidUsingCmdletAliasesDescription { get { @@ -745,6 +745,15 @@ internal static string AvoidUsingCmdletAliasesError { } } + /// + /// Looks up a localized string similar to '{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.. + /// + internal static string AvoidUsingCmdletAliasesMissingGetPrefixError { + get { + return ResourceManager.GetString("AvoidUsingCmdletAliasesMissingGetPrefixError", resourceCulture); + } + } + /// /// Looks up a localized string similar to AvoidUsingCmdletAliases. /// diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 5e0e1314e..352d5ee3f 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -118,10 +118,10 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - 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. + 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. - Avoid Using Cmdlet Aliases + Avoid Using Cmdlet Aliases or omitting the 'Get-' prefix. 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. @@ -1005,4 +1005,7 @@ AvoidAssignmentToAutomaticVariable + + '{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. + \ No newline at end of file diff --git a/Rules/UseCmdletCorrectly.cs b/Rules/UseCmdletCorrectly.cs index 37cfa3cdb..fae21cfad 100644 --- a/Rules/UseCmdletCorrectly.cs +++ b/Rules/UseCmdletCorrectly.cs @@ -79,7 +79,7 @@ private bool MandatoryParameterExists(CommandAst cmdAst) #region Compares parameter list and mandatory parameter list. - cmdInfo = Helper.Instance.GetCommandInfo(cmdAst.GetCommandName()); + cmdInfo = Helper.Instance.GetCommandInfoLegacy(cmdAst.GetCommandName()); if (cmdInfo == null || (cmdInfo.CommandType != System.Management.Automation.CommandTypes.Cmdlet)) { return true; diff --git a/Rules/UseShouldProcessCorrectly.cs b/Rules/UseShouldProcessCorrectly.cs index 8f619067d..6be9db87e 100644 --- a/Rules/UseShouldProcessCorrectly.cs +++ b/Rules/UseShouldProcessCorrectly.cs @@ -299,7 +299,7 @@ private bool SupportsShouldProcess(string cmdName) return false; } - var cmdInfo = Helper.Instance.GetCommandInfo(cmdName); + var cmdInfo = Helper.Instance.GetCommandInfoLegacy(cmdName); if (cmdInfo == null) { return false; diff --git a/Tests/Rules/AvoidUsingAlias.tests.ps1 b/Tests/Rules/AvoidUsingAlias.tests.ps1 index 415d20fd9..e99d8aa47 100644 --- a/Tests/Rules/AvoidUsingAlias.tests.ps1 +++ b/Tests/Rules/AvoidUsingAlias.tests.ps1 @@ -26,6 +26,11 @@ Describe "AvoidUsingAlias" { Test-CorrectionExtent $violationFilepath $violations[1] 1 'cls' 'Clear-Host' $violations[1].SuggestedCorrections[0].Description | Should -Be 'Replace cls with Clear-Host' } + + It "warns when 'Get-' prefix was omitted" { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'verb' | Where-Object { $_.RuleName -eq $violationName } + $violations.Count | Should -Be 1 + } } Context "Violation Extent" { @@ -95,5 +100,10 @@ Configuration MyDscConfiguration { $violations = Invoke-ScriptAnalyzer -ScriptDefinition "CD" -Settings $settings -IncludeRule $violationName $violations.Count | Should -Be 0 } + + It "do not warn when about Get-* completed cmdlets when the command exists natively on Unix platforms" -skip:(-not ($IsLinux -or $IsMacOS)) { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'date' | Where-Object { $_.RuleName -eq $violationName } + $violations.Count | Should -Be 0 + } } }