diff --git a/Rules/AvoidPositionalParameters.cs b/Rules/AvoidPositionalParameters.cs index d1c552957..3c6ec9626 100644 --- a/Rules/AvoidPositionalParameters.cs +++ b/Rules/AvoidPositionalParameters.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Management.Automation.Language; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System.Linq; #if !CORECLR using System.ComponentModel.Composition; #endif @@ -18,12 +19,20 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #if !CORECLR [Export(typeof(IScriptRule))] #endif - public class AvoidPositionalParameters : IScriptRule + public class AvoidPositionalParameters : ConfigurableRule { + [ConfigurableRuleProperty(defaultValue: new string[] { "az" })] + public string[] CommandAllowList { get; set; } + + public AvoidPositionalParameters() + { + Enable = true; // keep it enabled by default, user can still override this with settings + } + /// /// AnalyzeScript: Analyze the ast to check that positional parameters are not used. /// - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public override IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); @@ -57,20 +66,24 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { PipelineAst parent = cmdAst.Parent as PipelineAst; + string commandName = cmdAst.GetCommandName(); if (parent != null && parent.PipelineElements.Count > 1) { // raise if it's the first element in pipeline. otherwise no. - if (parent.PipelineElements[0] == cmdAst) + if (parent.PipelineElements[0] == cmdAst && !CommandAllowList.Contains(commandName, StringComparer.OrdinalIgnoreCase)) { - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName()); + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, commandName), + cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, commandName); } } // not in pipeline so just raise it normally else { - yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, cmdAst.GetCommandName()), - cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, cmdAst.GetCommandName()); + if (!CommandAllowList.Contains(commandName, StringComparer.OrdinalIgnoreCase)) + { + yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersError, commandName), + cmdAst.Extent, GetName(), DiagnosticSeverity.Information, fileName, commandName); + } } } } @@ -80,7 +93,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) /// GetName: Retrieves the name of this rule. /// /// The name of this rule - public string GetName() + public override string GetName() { return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingPositionalParametersName); } @@ -89,7 +102,7 @@ public string GetName() /// GetCommonName: Retrieves the common name of this rule. /// /// The common name of this rule - public string GetCommonName() + public override string GetCommonName() { return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersCommonName); } @@ -98,7 +111,7 @@ public string GetCommonName() /// GetDescription: Retrieves the description of this rule. /// /// The description of this rule - public string GetDescription() + public override string GetDescription() { return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingPositionalParametersDescription); } @@ -106,7 +119,7 @@ public string GetDescription() /// /// Method: Retrieves the type of the rule: builtin, managed or module. /// - public SourceType GetSourceType() + public override SourceType GetSourceType() { return SourceType.Builtin; } @@ -115,7 +128,7 @@ public SourceType GetSourceType() /// GetSeverity: Retrieves the severity of the rule: error, warning of information. /// /// - public RuleSeverity GetSeverity() + public override RuleSeverity GetSeverity() { return RuleSeverity.Information; } @@ -123,7 +136,7 @@ public RuleSeverity GetSeverity() /// /// Method: Retrieves the module/assembly name the rule is from. /// - public string GetSourceName() + public override string GetSourceName() { return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); } diff --git a/Tests/Rules/AvoidPositionalParameters.tests.ps1 b/Tests/Rules/AvoidPositionalParameters.tests.ps1 index 73fa5d563..ec5a91e24 100644 --- a/Tests/Rules/AvoidPositionalParameters.tests.ps1 +++ b/Tests/Rules/AvoidPositionalParameters.tests.ps1 @@ -26,6 +26,14 @@ Describe "AvoidPositionalParameters" { $violations.RuleName | Should -Contain 'PSAvoidUsingCmdletAliases' } + It "returns violations for command that is not in allow list of settings" { + $violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Join-Path a b c d' -Settings @{ + IncludeRules = @('PSAvoidUsingPositionalParameters') + Rules = @{ PSAvoidUsingPositionalParameters = @{ CommandAllowList = 'Test-Path' } } + } + $violations.Count | Should -Be 1 + $violations.RuleName | Should -Be 'PSAvoidUsingPositionalParameters' + } } Context "When there are no violations" { @@ -36,6 +44,17 @@ Describe "AvoidPositionalParameters" { It "returns no violations for DSC configuration" { $noViolationsDSC.Count | Should -Be 0 } + + It "returns no violations for AZ CLI by default" { + Invoke-ScriptAnalyzer -ScriptDefinition 'az group deployment list' | Should -BeNullOrEmpty + } + + It "returns no violations for command from allow list defined in settings and is case invariant" { + Invoke-ScriptAnalyzer -ScriptDefinition 'join-patH a b c' -Settings @{ + IncludeRules = @('PSAvoidUsingPositionalParameters') + Rules = @{ PSAvoidUsingPositionalParameters = @{ CommandAllowList = 'az', 'Join-Path' } } + } | Should -BeNullOrEmpty + } } Context "Function defined and called in script, which has 3 or more positional parameters triggers rule." { diff --git a/docs/Rules/AvoidUsingPositionalParameters.md b/docs/Rules/AvoidUsingPositionalParameters.md index 2c92b6e8f..4cecb2965 100644 --- a/docs/Rules/AvoidUsingPositionalParameters.md +++ b/docs/Rules/AvoidUsingPositionalParameters.md @@ -20,6 +20,27 @@ rule from being too noisy, this rule gets only triggered when there are 3 or mor supplied. A simple example where the risk of using positional parameters is negligible, is `Test-Path $Path`. +## Configuration + +```powershell +Rules = @{ + AvoidUsingPositionalParameters = @{ + CommandAllowList = 'az', 'Join-Path' + Enable = $true + } +} +``` + +### Parameters + +#### AvoidUsingPositionalParameters: string[] (Default value is 'az') + +Commands to be excluded from this rule. `az` is excluded by default because starting with version 2.40.0 the entrypoint of the AZ CLI became an `az.ps1` script but this script does not have any named parameters and just passes them on using `$args` as is to the Python process that it starts, therefore it is still a CLI and not a PowerShell command. + +#### Enable: bool (Default value is `$true`) + +Enable or disable the rule during ScriptAnalyzer invocation. + ## How Use full parameter names when calling commands.