Skip to content

PSAvoidDefaultValueForMandatoryParameter: Fix param block and parameter set handling #2121

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

PSAvoidDefaultValueForMandatoryParameter: Fix param block and parameter set handling #2121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1abe00e
Updated rule
liamjpeters Aug 18, 2025
8be946c
Remove no-longer relevant comment
liamjpeters Aug 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 98 additions & 52 deletions Rules/AvoidDefaultValueForMandatoryParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Management.Automation.Language;
#if !CORECLR
using System.ComponentModel.Composition;
Expand All @@ -27,59 +28,107 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
{
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);

// Finds all functionAst
IEnumerable<Ast> functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);

foreach (FunctionDefinitionAst funcAst in functionAsts)
// Find all ParameterAst which are children of a ParamBlockAst. This
// doesn't pick up where they appear as children of a
// FunctionDefinitionAst. i.e.
//
// function foo ($a,$b){} -> $a and $b are `ParameterAst`
//
// Include only parameters which have a default value (as without
// one this rule would never alert)
// Include only parameters where ALL parameter attributes have the
// mandatory named argument set to true (implicitly or explicitly)

var mandatoryParametersWithDefaultValues =
ast.FindAll(testAst => testAst is ParamBlockAst, true)
.Cast<ParamBlockAst>()
.Where(pb => pb.Parameters?.Count > 0)
.SelectMany(pb => pb.Parameters)
.Where(paramAst =>
paramAst.DefaultValue != null &&
HasMandatoryInAllParameterAttributes(
paramAst,
StringComparer.OrdinalIgnoreCase
)
);

// Report diagnostics for each parameter that violates the rule
foreach (var parameter in mandatoryParametersWithDefaultValues)
{
if (funcAst.Body != null && funcAst.Body.ParamBlock != null
&& funcAst.Body.ParamBlock.Attributes != null && funcAst.Body.ParamBlock.Parameters != null)
{
foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
{
bool mandatory = false;

// check that param is mandatory
foreach (var paramAstAttribute in paramAst.Attributes)
{
if (paramAstAttribute is AttributeAst)
{
var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments;
if (namedArguments != null)
{
foreach (NamedAttributeArgumentAst namedArgument in namedArguments)
{
if (String.Equals(namedArgument.ArgumentName, "mandatory", StringComparison.OrdinalIgnoreCase))
{
// 3 cases: [Parameter(Mandatory)], [Parameter(Mandatory=$true)] and [Parameter(Mandatory=value)] where value is not equal to 0.
if (namedArgument.ExpressionOmitted
|| (String.Equals(namedArgument.Argument.Extent.Text, "$true", StringComparison.OrdinalIgnoreCase))
|| (int.TryParse(namedArgument.Argument.Extent.Text, out int mandatoryValue) && mandatoryValue != 0))
{
mandatory = true;
break;
}
}
}
}
}
}

if (!mandatory)
{
break;
}

if (paramAst.DefaultValue != null)
{
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidDefaultValueForMandatoryParameterError, paramAst.Name.VariablePath.UserPath),
paramAst.Name.Extent, GetName(), DiagnosticSeverity.Warning, fileName, paramAst.Name.VariablePath.UserPath);
}
}
}
yield return new DiagnosticRecord(
string.Format(
CultureInfo.CurrentCulture,
Strings.AvoidDefaultValueForMandatoryParameterError,
parameter.Name.VariablePath.UserPath
),
parameter.Name.Extent,
GetName(),
DiagnosticSeverity.Warning,
fileName,
parameter.Name.VariablePath.UserPath
);
}
}

/// <summary>
/// Determines if a parameter is mandatory in all of its Parameter attributes.
/// A parameter may have multiple [Parameter] attributes for different parameter sets.
/// This method returns true only if ALL [Parameter] attributes have Mandatory=true.
/// </summary>
/// <param name="paramAst">The parameter AST to examine</param>
/// <param name="comparer">String comparer for case-insensitive attribute name matching</param>
/// <returns>
/// True if the parameter has at least one [Parameter] attribute and ALL of them
/// have the Mandatory named argument set to true (explicitly or implicitly).
/// False if the parameter has no [Parameter] attributes or if any [Parameter]
/// attribute does not have Mandatory=true.
/// </returns>
private static bool HasMandatoryInAllParameterAttributes(ParameterAst paramAst, StringComparer comparer)
{
var parameterAttributes = paramAst.Attributes.OfType<AttributeAst>()
.Where(attr => IsParameterAttribute(attr.TypeName?.Name, comparer))
.ToList();

return parameterAttributes.Count > 0 &&
parameterAttributes.All(attr => HasMandatoryArgument(attr, comparer));
}

/// <summary>
/// Determines if an attribute type name represents a PowerShell Parameter attribute.
/// Checks for both the short form "Parameter" and full form "ParameterAttribute".
/// </summary>
/// <param name="typeName">The attribute type name to check</param>
/// <param name="comparer">String comparer for case-insensitive matching</param>
/// <returns>
/// True if the type name is "Parameter" or "ParameterAttribute" (case-insensitive).
/// False otherwise.
/// </returns>
private static bool IsParameterAttribute(string typeName, StringComparer comparer)
{
return comparer.Equals(typeName, "parameter");
}

/// <summary>
/// Determines if a Parameter attribute has the Mandatory named argument set to true.
/// Handles both explicit (Mandatory=$true) and implicit (Mandatory) cases.
/// Uses the Helper.Instance.GetNamedArgumentAttributeValue method to evaluate
/// the mandatory argument value.
/// </summary>
/// <param name="attr">The Parameter attribute AST to examine</param>
/// <param name="comparer">String comparer for case-insensitive argument name matching</param>
/// <returns>
/// True if the attribute has a "Mandatory" named argument that evaluates to true.
/// False if there is no "Mandatory" argument or if it evaluates to false.
/// </returns>
private static bool HasMandatoryArgument(AttributeAst attr, StringComparer comparer)
{
return attr.NamedArguments?.OfType<NamedAttributeArgumentAst>()
.Any(namedArg =>
comparer.Equals(namedArg?.ArgumentName, "mandatory") &&
Helper.Instance.GetNamedArgumentAttributeValue(namedArg)
) == true;
}

/// <summary>
/// GetName: Retrieves the name of this rule.
/// </summary>
Expand Down Expand Up @@ -134,6 +183,3 @@ public string GetSourceName()
}
}




173 changes: 156 additions & 17 deletions Tests/Rules/AvoidDefaultValueForMandatoryParameter.tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,37 +6,176 @@ BeforeAll {
}

Describe "AvoidDefaultValueForMandatoryParameter" {
Context "When there are violations" {
It "has 1 provide default value for mandatory parameter violation with CmdletBinding" {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ [CmdletBinding()]Param([Parameter(Mandatory)]$Param1=''defaultValue'') }' |
Where-Object { $_.RuleName -eq $ruleName }

Context "Basic mandatory parameter violations" {
It "should flag mandatory parameter with default value (implicit)" {
$script = 'Function Test { Param([Parameter(Mandatory)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}

It "should flag mandatory parameter with default value (explicit true)" {
$script = 'Function Test { Param([Parameter(Mandatory=$true)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}

It "should flag mandatory parameter with default value (numeric true)" {
$script = 'Function Test { Param([Parameter(Mandatory=1)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}
}

Context "Parameter sets (multiple Parameter attributes)" {
It "should NOT flag parameter mandatory in some but not all parameter sets" {
$script = @'
Function Test {
Param(
[Parameter(Mandatory, ParameterSetName='Set1')]
[Parameter(ParameterSetName='Set2')]
$Param = 'default'
)
}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "should flag parameter mandatory in ALL parameter sets" {
$script = @'
Function Test {
Param(
[Parameter(Mandatory, ParameterSetName='Set1')]
[Parameter(Mandatory, ParameterSetName='Set2')]
$Param = 'default'
)
}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}

It "has 1 provide default value for mandatory=$true parameter violation without CmdletBinding" {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=$true)]$Param1=''defaultValue'') }' |
Where-Object { $_.RuleName -eq $ruleName }
It "should handle mixed mandatory/non-mandatory in multiple parameter sets" {
$script = @'
Function Test {
Param(
[Parameter(Mandatory=$true, ParameterSetName='Set1')]
[Parameter(Mandatory=$false, ParameterSetName='Set2')]
[Parameter(ParameterSetName='Set3')]
$Param = 'default'
)
}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}
}

Context "Script-level param blocks" {
It "should flag mandatory parameters with defaults in script-level param blocks" {
$script = @'
Param(
[Parameter(Mandatory)]
$ScriptParam = 'default'
)
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}

It "returns violations when the parameter is specified as mandatory=1 and has a default value" {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=1)]$Param1=''defaultValue'') }' |
Where-Object { $_.RuleName -eq $ruleName }
It "should NOT flag non-mandatory parameters in script-level param blocks" {
$script = @'
Param(
[Parameter()]
$ScriptParam = 'default'
)
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}
}

Context "Non-Parameter attributes" {
It "should NOT flag non-Parameter attributes with Mandatory property" {
$script = 'Function Test { Param([MyCustomAttribute(Mandatory)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "should NOT flag parameters with only validation attributes" {
$script = 'Function Test { Param([ValidateNotNull()]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}
}

Context "Valid scenarios (no violations)" {
It "should NOT flag mandatory parameters without default values" {
$script = 'Function Test { Param([Parameter(Mandatory)]$Param) }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "should NOT flag non-mandatory parameters with default values" {
$script = 'Function Test { Param([Parameter(Mandatory=$false)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "should NOT flag parameters without Parameter attributes" {
$script = 'Function Test { Param($Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "should NOT flag mandatory=0 parameters" {
$script = 'Function Test { Param([Parameter(Mandatory=0)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}
}

Context "Complex scenarios" {
It "should handle multiple parameters with mixed violations" {
$script = @'
Function Test {
Param(
[Parameter(Mandatory)]$BadParam = "default",
[Parameter()]$GoodParam = "default",
[Parameter(Mandatory)]$AnotherBadParam = "default"
)
}
'@
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 2
}

It "should work with CmdletBinding" {
$script = 'Function Test { [CmdletBinding()]Param([Parameter(Mandatory)]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}
}

Context "When there are no violations" {
It "has 1 provide default value for mandatory parameter violation" {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=$false)]$Param1=''val1'', [Parameter(Mandatory)]$Param2=''val2'', $Param3=''val3'') }' |
Where-Object { $_.RuleName -eq $ruleName }
Context "Edge cases" {
It "should handle empty param blocks gracefully" {
$script = 'Function Test { Param() }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}

It "returns no violations when the parameter is specified as mandatory=0 and has a default value" {
$violations = Invoke-ScriptAnalyzer -ScriptDefinition 'Function foo{ Param([Parameter(Mandatory=0)]$Param1=''val1'') }' |
Where-Object { $_.RuleName -eq $ruleName }
It "should handle null/empty default values" {
$script = 'Function Test { Param([Parameter(Mandatory)]$Param = $null) }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 1
}

It "should handle parameters with multiple non-Parameter attributes" {
$script = 'Function Test { Param([ValidateNotNull()][Alias("P")]$Param = "default") }'
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $script | Where-Object { $_.RuleName -eq $ruleName }
$violations.Count | Should -Be 0
}
}

}