diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 260214967..3059ceaf2 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1224,4 +1224,31 @@ AvoidUsingAllowUnencryptedAuthentication + + Use Consistent Parameter Set Name + + + Parameter set names are case-sensitive in PowerShell. This rule checks for case mismatches between DefaultParameterSetName and ParameterSetName values, case mismatches between different ParameterSetName values, and missing DefaultParameterSetName when parameter sets are used. + + + Param block uses parameter sets but does not specify a DefaultParameterSetName. Consider adding DefaultParameterSetName to the CmdletBinding attribute. + + + DefaultParameterSetName '{0}' does not match the case of ParameterSetName '{1}'. Parameter set names are case-sensitive. + + + ParameterSetName '{0}' does not match the case of '{1}'. Parameter set names are case-sensitive and should use consistent casing. + + + Parameter '{0}' is declared in parameter-set '{1}' multiple times. + + + Parameter set names should not contain new lines. + + + Rename ParameterSet '{0}' to '{1}'. + + + UseConsistentParameterSetName + \ No newline at end of file diff --git a/Rules/UseConsistentParameterSetName.cs b/Rules/UseConsistentParameterSetName.cs new file mode 100644 index 000000000..10723d550 --- /dev/null +++ b/Rules/UseConsistentParameterSetName.cs @@ -0,0 +1,467 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Management.Automation.Language; +#if !CORECLR +using System.ComponentModel.Composition; +#endif +using System.Globalization; +using System.Linq; +using System.Management.Automation; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// UseConsistentParameterSetName: Check for case-sensitive parameter set + /// name mismatches, missing default parameter set names, and parameter set + /// names containing new lines. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class UseConsistentParameterSetName : IScriptRule + { + + private const string AllParameterSetsName = "__AllParameterSets"; + + /// + /// AnalyzeScript: Check for parameter set name issues. + /// + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + throw new ArgumentNullException(Strings.NullAstErrorMessage); + } + + var allParameterBlocks = ast + .FindAll(testAst => testAst is ParamBlockAst, true) + .Cast() + .Where(pb => pb.Parameters?.Count > 0); + + foreach (var paramBlock in allParameterBlocks) + { + // If the paramblock has no parameters, skip it + if (paramBlock.Parameters.Count == 0) + { + continue; + } + + // Get the CmdletBinding attribute and default parameter set name + // Or null if not present + var cmdletBindingAttr = Helper.Instance.GetCmdletBindingAttributeAst(paramBlock.Attributes); + var defaultParamSetName = GetNamedArgumentValue(cmdletBindingAttr, "DefaultParameterSetName"); + + // For each parameter block, build up a list of all the parameters + // and the parameter sets in which they appear. + List paramBlockInfo = new List(); + + foreach (var parameter in paramBlock.Parameters) + { + // If the parameter has no attributes, it is part of all + // parameter sets. We can ignore it for these checks. + if (parameter.Attributes.Count == 0) + { + continue; + } + + // For each parameter attribute a parameter has, extract + // the parameter set and add it to our knowledge of the + // param block. + foreach (var attribute in parameter.Attributes.Where(attr => attr is AttributeAst).Cast()) + { + if (string.Equals(attribute.TypeName?.Name, "Parameter", StringComparison.OrdinalIgnoreCase)) + { + var parameterSetName = GetNamedArgumentValue(attribute, "ParameterSetName", AllParameterSetsName); + paramBlockInfo.Add(new ParameterSetInfo(parameter.Name.VariablePath.UserPath, parameterSetName, attribute)); + } + } + } + + // We now have a picture of the parameters and parameterset + // usage of this paramblock. We can make each check. + + // Check 1: Default parameter set name + // ------------------------------------------------------------- + // If we have parameter sets in use and the CmdletBinding + // attribute, but no default specified, warn about this. + if (string.IsNullOrEmpty(defaultParamSetName) && + cmdletBindingAttr != null && + paramBlockInfo.Any(p => p.ParameterSetName != AllParameterSetsName) + ) + { + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameMissingDefaultError), + cmdletBindingAttr?.Extent ?? paramBlock.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + + // Check 2: Parameter Declared Multiple Times in Same Set + // ------------------------------------------------------------- + // If any parameter has more than one parameter attribute for + // the same parameterset, warn about each instance. + // Parameters cannot be declared multiple times in the same set. + // Calling a function that has a parameter declared multiple + // times in the same parameterset is a runtime exception - + // specifically a [System.Management.Automation.MetadataException] + // It'd be better to know before runtime. + // We use the same message text as the MetadataException for + // consistency + var duplicateAttributes = paramBlockInfo + .GroupBy(p => new { p.ParameterName, p.ParameterSetName }) + .Where(g => g.Count() > 1) + .SelectMany(g => g); + + foreach (var duplicate in duplicateAttributes) + { + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameMultipleDeclarationsError, + duplicate.ParameterName, + duplicate.ParameterSetName), + duplicate.ParameterAttributeAst.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + + // Check 3: Validate Default Parameter Set + // ------------------------------------------------------------- + // If a default parameter set is specified and matches one of + // the used parameter set names ignoring case, but not otherwise + // then we should warn about this + if (!string.IsNullOrEmpty(defaultParamSetName)) + { + // Look for an exact (case-sensitive) match + var exactMatch = paramBlockInfo + .FirstOrDefault(p => + string.Equals( + p.ParameterSetName, + defaultParamSetName, + StringComparison.Ordinal + ) + ); + + if (exactMatch == null) + { + // No exact match, look for a case-insensitive match + var caseInsensitiveMatch = paramBlockInfo + .FirstOrDefault(p => + string.Equals( + p.ParameterSetName, + defaultParamSetName, + StringComparison.OrdinalIgnoreCase + ) + ); + + if (caseInsensitiveMatch != null) + { + var defaultParameterSetNameExtents = GetDefaultParameterSetNameValueExtent(cmdletBindingAttr); + + // Emit a diagnostic for the first case-insensitive match + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameCaseMismatchDefaultError, + defaultParamSetName, + caseInsensitiveMatch.ParameterSetName), + defaultParameterSetNameExtents ?? cmdletBindingAttr?.Extent ?? paramBlock.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + } + } + + // Check 4: Parameter Set Name Consistency + // ------------------------------------------------------------- + // If a parameter set name is used in multiple places, it must + // be consistently used across all usages. This means the casing + // must match exactly. We should warn about any inconsistencies + // found. + var paramSetGroups = paramBlockInfo + .GroupBy(p => p.ParameterSetName, StringComparer.OrdinalIgnoreCase) + .Where(g => + g.Select(p => p.ParameterSetName) + .Distinct(StringComparer.Ordinal) + .Count() > 1 + ); + + foreach (var group in paramSetGroups) + { + // Take the first instance as the canonical casing + var canonical = group.First(); + foreach (var entry in group.Skip(1)) + { + if (!string.Equals( + entry.ParameterSetName, + canonical.ParameterSetName, + StringComparison.Ordinal + ) + ) + { + var parameterSetNameExtents = GetParameterSetNameValueExtent(entry.ParameterAttributeAst); + + if (parameterSetNameExtents != null) + { + var correction = new CorrectionExtent( + parameterSetNameExtents.StartLineNumber, + parameterSetNameExtents.EndLineNumber, + parameterSetNameExtents.StartColumnNumber, + parameterSetNameExtents.EndColumnNumber, + $"'{canonical.ParameterSetName}'", + fileName, + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameCaseMismatchSuggestedCorrectionDescription, + entry.ParameterSetName, + canonical.ParameterSetName + ) + ); + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameCaseMismatchParameterError, + entry.ParameterSetName, + canonical.ParameterSetName), + parameterSetNameExtents, + GetName(), + DiagnosticSeverity.Warning, + fileName, + null, + new List { correction }); + } + else + { + // If we couldn't find the parameter set name extents, we can't create a correction + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameCaseMismatchParameterError, + entry.ParameterSetName, + canonical.ParameterSetName), + entry.ParameterAttributeAst.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + } + } + } + + // Check 5: Parameter Set Names should not contain New Lines + // ------------------------------------------------------------- + // There is no practical purpose for parameterset names to + // contain a newline + foreach (var entry in paramBlockInfo) + { + if (entry.ParameterSetName.Contains('\n') || entry.ParameterSetName.Contains('\r')) + { + var parameterSetNameExtents = GetParameterSetNameValueExtent(entry.ParameterAttributeAst); + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameNewLineError), + parameterSetNameExtents ?? entry.ParameterAttributeAst.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + } + if (defaultParamSetName != null && + (defaultParamSetName.Contains('\n') || defaultParamSetName.Contains('\r'))) + { + // If the default parameter set name contains new lines, warn about it + var defaultParameterSetNameExtents = GetDefaultParameterSetNameValueExtent(cmdletBindingAttr); + yield return new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameNewLineError, + defaultParamSetName), + defaultParameterSetNameExtents ?? cmdletBindingAttr?.Extent ?? paramBlock.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName); + } + + } + } + + /// + /// Retrieves the value of a named argument from an AttributeAst's NamedArguments collection. + /// If the named argument is not found, returns the provided default value. + /// If the argument value is a constant, returns its string representation; otherwise, returns the argument's text. + /// + /// The AttributeAst to search for the named argument. + /// The name of the argument to look for (case-insensitive). + /// The value to return if the named argument is not found. Defaults to null. + /// + /// The value of the named argument as a string if found; otherwise, the default value. + /// + private static string GetNamedArgumentValue(AttributeAst attributeAst, string argumentName, string defaultValue = null) + { + if (attributeAst == null || attributeAst.NamedArguments == null) + { + return defaultValue; + } + + foreach (var namedArg in attributeAst.NamedArguments) + { + if (namedArg?.ArgumentName == null) continue; + + if (string.Equals(namedArg.ArgumentName, argumentName, StringComparison.OrdinalIgnoreCase)) + { + // Try to evaluate the argument value as a constant string + if (namedArg.Argument is ConstantExpressionAst constAst) + { + return constAst.Value?.ToString(); + } + // If not a constant, try to get the string representation + return namedArg.Argument.Extent.Text; + } + } + return defaultValue; + } + + /// + /// Finds the IScriptExtent of the value assigned to the ParameterSetName argument + /// in the given AttributeAst (if it is a [Parameter()] attribute). + /// Returns null if not found. + /// + /// The AttributeAst to search. + /// The IScriptExtent of the ParameterSetName value, or null if not found. + private static IScriptExtent GetParameterSetNameValueExtent(AttributeAst attributeAst) + { + return GetAttributeNamedArgumentValueExtent(attributeAst, "ParameterSetName", "Parameter"); + // if (attributeAst == null || attributeAst.NamedArguments == null) + // return null; + + // if (!string.Equals(attributeAst.TypeName?.Name, "Parameter", StringComparison.OrdinalIgnoreCase)) + // return null; + + // foreach (var namedArg in attributeAst.NamedArguments) + // { + // if (string.Equals(namedArg.ArgumentName, "ParameterSetName", StringComparison.OrdinalIgnoreCase)) + // { + // return namedArg.Argument?.Extent; + // } + // } + // return null; + } + + /// + /// Finds the IScriptExtent of the value assigned to the DefaultParameterSetName argument + /// in the given AttributeAst (if it is a [CmdletBinding()] attribute). + /// Returns null if not found. + /// + /// The AttributeAst to search. + /// The IScriptExtent of the DefaultParameterSetName value, or null if not found. + private static IScriptExtent GetDefaultParameterSetNameValueExtent(AttributeAst attributeAst) + { + return GetAttributeNamedArgumentValueExtent(attributeAst, "DefaultParameterSetName", "CmdletBinding"); + } + + /// + /// Finds the IScriptExtent of the value of a named argument in the given AttributeAst. + /// Returns null if not found. + /// + /// The AttributeAst to search. + /// The name of the argument to find. + /// The expected type name of the attribute. i.e. Parameter (optional). + /// The IScriptExtent of the named argument value, or null if not found. + private static IScriptExtent GetAttributeNamedArgumentValueExtent(AttributeAst attributeAst, string argumentName, string expectedAttributeName = null) + { + if (attributeAst == null || attributeAst.NamedArguments == null) + return null; + + if (!string.IsNullOrEmpty(expectedAttributeName) && + !string.Equals( + attributeAst.TypeName?.Name, + expectedAttributeName, + StringComparison.OrdinalIgnoreCase) + ) + return null; + + foreach (var namedArg in attributeAst.NamedArguments) + { + if (string.Equals(namedArg.ArgumentName, argumentName, StringComparison.OrdinalIgnoreCase)) + { + return namedArg.Argument?.Extent; + } + } + return null; + } + + /// + /// Represents information about a parameter and its parameter set. + /// + private class ParameterSetInfo + { + public string ParameterName { get; } + public string ParameterSetName { get; } + public AttributeAst ParameterAttributeAst { get; } + + public ParameterSetInfo(string parameterName, string parameterSetName, AttributeAst parameterAttributeAst) + { + ParameterName = parameterName; + ParameterSetName = parameterSetName; + ParameterAttributeAst = parameterAttributeAst; + } + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.UseConsistentParameterSetNameName + ); + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() => string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameCommonName + ); + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() => string.Format( + CultureInfo.CurrentCulture, + Strings.UseConsistentParameterSetNameDescription + ); + + /// + /// Method: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() => SourceType.Builtin; + + /// + /// GetSeverity: Retrieves the severity of the rule: error, warning of information. + /// + /// + public RuleSeverity GetSeverity() => RuleSeverity.Warning; + + /// + /// Method: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() => string.Format( + CultureInfo.CurrentCulture, Strings.SourceName + ); + } +} diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index 8d61c1c7f..c3b744803 100644 --- a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 +++ b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 @@ -63,7 +63,7 @@ Describe "Test Name parameters" { It "get Rules with no parameters supplied" { $defaultRules = Get-ScriptAnalyzerRule - $expectedNumRules = 70 + $expectedNumRules = 71 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because diff --git a/Tests/Rules/UseConsistentParameterSetName.tests.ps1 b/Tests/Rules/UseConsistentParameterSetName.tests.ps1 new file mode 100644 index 000000000..539ca078a --- /dev/null +++ b/Tests/Rules/UseConsistentParameterSetName.tests.ps1 @@ -0,0 +1,391 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +BeforeAll { + $ruleName = 'PSUseConsistentParameterSetName' +} + +Describe "UseConsistentParameterSetName" { + Context "When there are case mismatch violations between DefaultParameterSetName and ParameterSetName" { + It "detects case mismatch between DefaultParameterSetName and ParameterSetName" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='setone')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Severity | Should -Be 'Warning' + $violations[0].Message | Should -Match "DefaultParameterSetName 'SetOne' does not match the case of ParameterSetName 'setone'" + } + + It "detects multiple case mismatches with DefaultParameterSetName" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='setone')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SETONE')] + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 + $violations | ForEach-Object { $_.Severity | Should -Be 'Warning' } + } + } + + Context "When there are case mismatch violations between ParameterSetName values" { + It "detects case mismatch between different ParameterSetName values" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='setone')] + [string]$Parameter2, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter3 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Severity | Should -Be 'Warning' + $violations[0].Message | Should -Match "ParameterSetName 'setone' does not match the case of 'SetOne'" + } + + It "detects multiple case variations of the same parameter set name" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='setone')] + [string]$Parameter2, + + [Parameter(ParameterSetName='SETONE')] + [string]$Parameter3 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 # Two mismatches with the first occurrence + $violations | ForEach-Object { $_.Severity | Should -Be 'Warning' } + } + } + + Context "When DefaultParameterSetName is missing" { + It "warns when parameter sets are used but DefaultParameterSetName is not specified" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding()] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Severity | Should -Be 'Warning' + $violations[0].Message | Should -Match "uses parameter sets but does not specify a DefaultParameterSetName" + } + } + + Context "When a parameter is declared multiple times in the same parameter set" { + It "detects duplicate parameter declarations in the same parameter set (explicit)" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName='SetOne')] + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 + $violations | ForEach-Object { $_.Message | Should -Be "Parameter 'Parameter1' is declared in parameter-set 'SetOne' multiple times." } + } + + It "detects duplicate parameter declarations in the same parameter set (implicit via omitted ParameterSetName)" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter()] + [Parameter()] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 + $violations | ForEach-Object { $_.Message | Should -Be "Parameter 'Parameter1' is declared in parameter-set '__AllParameterSets' multiple times." } + } + + It "detects duplicate parameter declarations in explicit and implicit parameter sets" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName='__AllParameterSets')] + [Parameter()] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 + $violations | ForEach-Object { $_.Message | Should -Be "Parameter 'Parameter1' is declared in parameter-set '__AllParameterSets' multiple times." } + } + + + } + + Context "When ParameterSetNames contain inadvisable characters" { + It "detects ParameterSetName containing a new line" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName="Set`nOne")] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Message | Should -Match "should not contain new lines" + } + + It "detects ParameterSetName containing a carriage return" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName="Set`rOne")] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Message | Should -Match "should not contain new lines" + } + + It "detects ParameterSetName containing mixed newline types" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName="Set`r`nOne")] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + } + + It "detects DefaultParameterSetName containing a new line" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName="Set`nOne")] + param( + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Message | Should -Match "should not contain new lines" + } + + It "detects DefaultParameterSetName containing a carriage return" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName="Set`rOne")] + param( + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + $violations[0].Message | Should -Match "should not contain new lines" + } + + # Missing: DefaultParameterSetName with newlines + It "detects DefaultParameterSetName containing mixed newline types" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName="Set`r`nOne")] + param([string]$Parameter1) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 1 + } + + } + + Context "When there are no violations" { + It "does not flag functions without CmdletBinding" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It "does not flag functions without parameter sets" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding()] + param( + [string]$Parameter1, + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It "does not flag when DefaultParameterSetName and ParameterSetName cases match exactly" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It "does not flag when all ParameterSetName cases match exactly" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter2, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter3 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + # This could be a case where the function can be run without any parameters + # in the default set. + It "does not flag when DefaultParameterSetName doesn't match any ParameterSetName" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='Default')] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [Parameter(ParameterSetName='SetTwo')] + [string]$Parameter2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It "handles parameters without attributes correctly" { + $scriptDefinition = @' +function Test-Function { + [CmdletBinding(DefaultParameterSetName='SetOne')] + param( + [Parameter(ParameterSetName='SetOne')] + [string]$Parameter1, + + [string]$CommonParameter # No Parameter attribute + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + } + + Context "Real-world scenarios" { + It "handles complex parameter set definitions correctly" { + $scriptDefinition = @' +function Test-ComplexFunction { + [CmdletBinding(DefaultParameterSetName='ByName')] + param( + [Parameter(ParameterSetName='ByName', Mandatory)] + [string]$Name, + + [Parameter(ParameterSetName='ByName')] + [Parameter(ParameterSetName='ByID')] + [string]$ComputerName, + + [Parameter(ParameterSetName='ByID', Mandatory)] + [int]$ID + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 0 + } + + It "detects case issues in complex scenarios" { + $scriptDefinition = @' +function Test-ComplexFunction { + [CmdletBinding(DefaultParameterSetName='ByName')] + param( + [Parameter(ParameterSetName='byname', Mandatory)] + [string]$Name, + + [Parameter(ParameterSetName='ByName')] + [Parameter(ParameterSetName='ByID')] + [string]$ComputerName, + + [Parameter(ParameterSetName='byid', Mandatory)] + [int]$ID + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) + $violations.Count | Should -Be 2 # 'byname' and 'byid' case mismatches + $violations | ForEach-Object { $_.Severity | Should -Be 'Warning' } + } + } +} diff --git a/docs/Rules/README.md b/docs/Rules/README.md index 06f27d2da..aca398222 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -67,6 +67,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [UseCompatibleSyntax](./UseCompatibleSyntax.md) | Warning | No | Yes | | [UseCompatibleTypes](./UseCompatibleTypes.md) | Warning | No | Yes | | [UseConsistentIndentation](./UseConsistentIndentation.md) | Warning | No | Yes | +| [UseConsistentParameterSetName](./UseConsistentParameterSetName.md) | Warning | Yes | | | [UseConsistentWhitespace](./UseConsistentWhitespace.md) | Warning | No | Yes | | [UseCorrectCasing](./UseCorrectCasing.md) | Information | No | Yes | | [UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | Yes | | diff --git a/docs/Rules/UseConsistentParameterSetName.md b/docs/Rules/UseConsistentParameterSetName.md new file mode 100644 index 000000000..66f0d3fce --- /dev/null +++ b/docs/Rules/UseConsistentParameterSetName.md @@ -0,0 +1,116 @@ +--- +description: Use consistent parameter set names and proper parameter set configuration. +ms.date: 08/19/2025 +ms.topic: reference +title: UseConsistentParameterSetName +--- + +# UseConsistentParameterSetName + +**Severity Level: Warning** + +## Description + +Parameter set names in PowerShell are case-sensitive, unlike most other PowerShell elements. This rule ensures consistent casing and proper configuration of parameter sets to avoid runtime errors and improve code clarity. + +The rule performs five different checks: + +1. **Missing DefaultParameterSetName** - Warns when parameter sets are used but no default is specified +2. **Multiple parameter declarations** - Detects when a parameter is declared multiple times in the same parameter set. This is ultimately a runtime exception - this check helps catch it sooner. +3. **Case mismatch between DefaultParameterSetName and ParameterSetName** - Ensures consistent casing +4. **Case mismatch between different ParameterSetName values** - Ensures all references to the same parameter set use identical casing +5. **Parameter set names containing newlines** - Warns against using newline characters in parameter set names + +## How + +- Use a `DefaultParameterSetName` when defining multiple parameter sets +- Ensure consistent casing between `DefaultParameterSetName` and `ParameterSetName` values +- Use identical casing for all references to the same parameter set name +- Avoid declaring the same parameter multiple times in a single parameter set +- Do not use newline characters in parameter set names + +## Example + +### Wrong + +```powershell +# Missing DefaultParameterSetName +function Get-Data { + [CmdletBinding()] + param( + [Parameter(ParameterSetName='ByName')] + [string]$Name, + + [Parameter(ParameterSetName='ByID')] + [int]$ID + ) +} + +# Case mismatch between DefaultParameterSetName and ParameterSetName +function Get-Data { + [CmdletBinding(DefaultParameterSetName='ByName')] + param( + [Parameter(ParameterSetName='byname')] + [string]$Name, + + [Parameter(ParameterSetName='ByID')] + [int]$ID + ) +} + +# Inconsistent casing between ParameterSetName values +function Get-Data { + [CmdletBinding(DefaultParameterSetName='ByName')] + param( + [Parameter(ParameterSetName='ByName')] + [string]$Name, + + [Parameter(ParameterSetName='byname')] + [string]$DisplayName + ) +} + +# Multiple parameter declarations in same set +function Get-Data { + param( + [Parameter(ParameterSetName='ByName')] + [Parameter(ParameterSetName='ByName')] + [string]$Name + ) +} + +# Parameter set name with newline +function Get-Data { + param( + [Parameter(ParameterSetName="Set`nOne")] + [string]$Name + ) +} +``` + +### Correct + +```powershell +# Proper parameter set configuration +function Get-Data { + [CmdletBinding(DefaultParameterSetName='ByName')] + param( + [Parameter(ParameterSetName='ByName', Mandatory)] + [string]$Name, + + [Parameter(ParameterSetName='ByName')] + [Parameter(ParameterSetName='ByID')] + [string]$ComputerName, + + [Parameter(ParameterSetName='ByID', Mandatory)] + [int]$ID + ) +} +``` + +## Notes + +- Parameter set names are case-sensitive in PowerShell, making this different from most other PowerShell elements +- The first occurrence of a parameter set name in your code is treated as the canonical casing +- Parameters without [Parameter()] attributes are automatically part of all parameter sets +- It's a PowerShell best practice to always specify a DefaultParameterSetName when using parameter sets \ No newline at end of file