From de338e08cfcf4480d3315fb6ed0f196e6f6e21a3 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Fri, 8 Aug 2025 23:08:19 +0100 Subject: [PATCH 1/5] Added Rule UseSingleValueFromPipelineParameter --- Rules/Strings.resx | 12 + Rules/UseSingleValueFromPipelineParameter.cs | 184 ++++++++++ ...SingleValueFromPipelineParameter.Tests.ps1 | 326 ++++++++++++++++++ docs/Rules/README.md | 1 + .../UseSingleValueFromPipelineParameter.md | 101 ++++++ 5 files changed, 624 insertions(+) create mode 100644 Rules/UseSingleValueFromPipelineParameter.cs create mode 100644 Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 create mode 100644 docs/Rules/UseSingleValueFromPipelineParameter.md diff --git a/Rules/Strings.resx b/Rules/Strings.resx index c7645c9cf..a60f478cb 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -1236,4 +1236,16 @@ The reserved word '{0}' was used as a function name. This should be avoided. + + Use a single ValueFromPipeline parameter per parameter set + + + Use at most a single ValueFromPipeline parameter per parameter set to avoid undefined or unexpected behaviour. + + + Multiple parameters ({0}) in parameter set '{1}' are marked as ValueFromPipeline. Only one parameter per parameter set should accept pipeline input. + + + UseSingleValueFromPipelineParameter + \ No newline at end of file diff --git a/Rules/UseSingleValueFromPipelineParameter.cs b/Rules/UseSingleValueFromPipelineParameter.cs new file mode 100644 index 000000000..42ae92f67 --- /dev/null +++ b/Rules/UseSingleValueFromPipelineParameter.cs @@ -0,0 +1,184 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Management.Automation.Language; +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that identifies parameter blocks with multiple parameters in + /// the same parameter set that are marked as ValueFromPipeline=true, which + /// can cause undefined behavior. + /// + public class UseSingleValueFromPipelineParameter : IScriptRule + { + private const string AllParameterSetsName = "__AllParameterSets"; + + /// + /// Analyzes the PowerShell AST for parameter sets with multiple ValueFromPipeline parameters. + /// + /// The PowerShell Abstract Syntax Tree to analyze. + /// The name of the file being analyzed (for diagnostic reporting). + /// A collection of diagnostic records for each violating parameter. + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) + { + yield break; + } + // Find all param blocks that have a Parameter attribute with + // ValueFromPipeline set to true. + var paramBlocks = ast.FindAll(testAst => testAst is ParamBlockAst, true) + .Where(paramBlock => paramBlock.FindAll( + attributeAst => attributeAst is AttributeAst attr && + ParameterAttributeAstHasValueFromPipeline(attr), + true + ).Any()); + + foreach (var paramBlock in paramBlocks) + { + // Find all parameter declarations in the current param block + // Convert the generic ast objects into ParameterAst Objects + // For each ParameterAst, find all it's attributes that have + // ValueFromPipeline set to true (either explicitly or + // implicitly). Flatten the results into a single collection of + // Annonymous objects relating the parameter with it's attribute + // and then group them by parameter set name. + // + // + // https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parameter_sets?#reserved-parameter-set-name + // + // The default parameter set name is '__AllParameterSets'. + // Not specifying a parameter set name and using the parameter + // set name '__AllParameterSets' are equivalent, so we shouldn't + // treat them like they're different just because one is an + // empty string and the other is not. + // + // Filter the list to only keep parameter sets that have more + // than one ValueFromPipeline parameter. + var parameterSetGroups = paramBlock.FindAll(n => n is ParameterAst, true) + .Cast() + .SelectMany(parameter => parameter.FindAll( + a => a is AttributeAst attr && ParameterAttributeAstHasValueFromPipeline(attr), + true + ).Cast().Select(attr => new { Parameter = parameter, Attribute = attr })) + .GroupBy(item => GetParameterSetForAttribute(item.Attribute) ?? AllParameterSetsName) + .Where(group => group.Count() > 1); + + + foreach (var group in parameterSetGroups) + { + // __AllParameterSets being the default name is...obscure. + // Instead we'll show the user "default". It's more than + // likely the user has not specified a parameter set name, + // so default will make sense. If they have used 'default' + // as their parameter set name, then we're still correct. + var parameterSetName = group.Key == AllParameterSetsName ? "default" : group.Key; + + // Create a concatenated string of parameter names that + // conflict in this parameter set + var parameterNames = string.Join(", ", group.Select(item => item.Parameter.Name.VariablePath.UserPath)); + + // We emit a diagnostic record for each offending parameter + // attribute in the parameter set so it's obvious where all the + // occurrences are. + foreach (var item in group) + { + var message = string.Format(CultureInfo.CurrentCulture, + Strings.UseSingleValueFromPipelineParameterError, + parameterNames, + parameterSetName); + + yield return new DiagnosticRecord( + message, + item.Attribute.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName, + parameterSetName); + } + } + } + } + + /// + /// Returns whether the specified AttributeAst represents a Parameter attribute + /// that has the ValueFromPipeline named argument set to true (either explicitly or + /// implicitly). + /// + /// The Parameter attribute to examine. + /// Whether the attribute has the ValueFromPipeline named argument set to true. + private static bool ParameterAttributeAstHasValueFromPipeline(AttributeAst attributeAst) + { + // Exit quickly if the attribute is null, has no named arguments, or + // is not a parameter attribute. + if (attributeAst?.NamedArguments == null || + !string.Equals(attributeAst.TypeName?.Name, "Parameter", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + return attributeAst.NamedArguments + .OfType() + .Any(namedArg => string.Equals( + namedArg?.ArgumentName, + "ValueFromPipeline", + StringComparison.OrdinalIgnoreCase + // Helper.Instance.GetNamedArgumentAttributeValue handles both explicit ($true) + // and implicit (no value specified) ValueFromPipeline declarations + ) && Helper.Instance.GetNamedArgumentAttributeValue(namedArg)); + } + + /// + /// Gets the ParameterSetName value from a Parameter attribute. + /// + /// The Parameter attribute to examine. + /// The parameter set name, or null if not found or empty. + private static string GetParameterSetForAttribute(AttributeAst attributeAst) + { + // Exit quickly if the attribute is null, has no named arguments, or + // is not a parameter attribute. + if (attributeAst?.NamedArguments == null || + !string.Equals(attributeAst.TypeName.Name, "Parameter", StringComparison.OrdinalIgnoreCase)) + { + return null; + } + + return attributeAst.NamedArguments + .OfType() + .Where(namedArg => string.Equals( + namedArg?.ArgumentName, + "ParameterSetName", + StringComparison.OrdinalIgnoreCase + )) + .Select(namedArg => namedArg?.Argument) + .OfType() + .Select(stringConstAst => stringConstAst?.Value) + .FirstOrDefault(value => !string.IsNullOrWhiteSpace(value)); + } + + public string GetCommonName() => Strings.UseSingleValueFromPipelineParameterCommonName; + + public string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription; + + public string GetName() => Strings.UseSingleValueFromPipelineParameterName; + + public RuleSeverity GetSeverity() => RuleSeverity.Warning; + + public string GetSourceName() => Strings.SourceName; + + public SourceType GetSourceType() => SourceType.Builtin; + } +} \ No newline at end of file diff --git a/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 new file mode 100644 index 000000000..e301408d8 --- /dev/null +++ b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 @@ -0,0 +1,326 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +BeforeAll { + $ruleName = 'UseSingleValueFromPipelineParameter' +} + +Describe 'UseSingleValueFromPipelineParameter' { + + Context 'When multiple parameters have ValueFromPipeline in same parameter set' { + + It "Should flag explicit ValueFromPipeline=`$true in default parameter set" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $InputObject, + + [Parameter(ValueFromPipeline=$true)] + $AnotherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 2 + $violations[0].Message | Should -Match "Multiple parameters \(InputObject, AnotherParam\) in parameter set 'default'" + } + + It 'Should flag implicit ValueFromPipeline in default parameter set' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline)] + $InputObject, + + [Parameter(ValueFromPipeline)] + $SecondParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 2 + } + + It 'Should flag mixed explicit and implicit ValueFromPipeline' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $InputObject, + + [Parameter(ValueFromPipeline)] + $SecondParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 2 + } + + It 'Should flag multiple parameters in named parameter set' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true, ParameterSetName='MySet')] + $InputObject, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='MySet')] + $SecondParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 2 + $violations[0].Message | Should -Match "parameter set 'MySet'" + } + + It 'Should flag three parameters in same parameter set' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $First, + + [Parameter(ValueFromPipeline=$true)] + $Second, + + [Parameter(ValueFromPipeline=$true)] + $Third + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 3 + $violations[0].Message | Should -Match 'Multiple parameters \(First, Second, Third\)' + } + } + + Context 'When parameters are in different parameter sets' { + + It 'Should not flag parameters in different parameter sets' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set1')] + $InputObject1, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set2')] + $InputObject2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + + It 'Should handle mix of named and default parameter sets correctly' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $DefaultSetParam, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='NamedSet')] + $NamedSetParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } + + Context 'When only one parameter has ValueFromPipeline' { + + It 'Should not flag single ValueFromPipeline parameter' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $InputObject, + + [Parameter()] + $OtherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } + + Context 'When ValueFromPipeline is explicitly set to false' { + + It "Should not flag parameters with ValueFromPipeline=`$false" { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$false)] + $InputObject, + + [Parameter(ValueFromPipeline=$false)] + $AnotherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + + It 'Should only flag the true ValueFromPipeline parameter' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + $TrueParam, + + [Parameter(ValueFromPipeline=$false)] + $FalseParam, + + [Parameter()] + $NoValueParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } + + Context 'When non-Parameter attributes have ValueFromPipeline property' { + + It 'Should not flag custom attributes with ValueFromPipeline property' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + [CustomAttribute(ValueFromPipeline=$true)] + $InputObject, + + [CustomAttribute(ValueFromPipeline=$true)] + $NonPipelineParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + + It 'Should not flag ValidateSet with ValueFromPipeline property' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true)] + [ValidateSet('Value1', 'Value2', ValueFromPipeline=$true)] + $InputObject, + + [ValidateSet('Value1', 'Value2', ValueFromPipeline=$true)] + $NonPipelineParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } + + Context 'When there are no Parameter attributes' { + + It 'Should not flag functions without Parameter attributes' { + $scriptDefinition = @' +function Test-Function { + param( + $InputObject, + $AnotherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + + It 'Should not flag functions with only non-ValueFromPipeline Parameter attributes' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(Mandatory=$true)] + $InputObject, + + [Parameter(Position=0)] + $AnotherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } + + Context 'Complex parameter set scenarios' { + + It 'Should flag violations in multiple parameter sets independently' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set1')] + $Set1Param1, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set1')] + $Set1Param2, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set2')] + $Set2Param1, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='Set2')] + $Set2Param2 + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 4 # 2 violations per parameter set, each parameter gets flagged + + # Check that both parameter sets are mentioned in violations + $violationMessages = $violations.Message -join ' ' + $violationMessages | Should -Match "parameter set 'Set1'" + $violationMessages | Should -Match "parameter set 'Set2'" + } + + It 'Should handle __AllParameterSets parameter set name correctly' { + $scriptDefinition = @' +function Test-Function { + param( + [Parameter(ValueFromPipeline=$true, ParameterSetName='__AllParameterSets')] + $ExplicitAllSets, + + [Parameter(ValueFromPipeline=$true)] + $ImplicitAllSets + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 2 + $violations[0].Message | Should -Match "parameter set 'default'" + } + } + + Context 'Suppression scenarios' { + + It 'Should be suppressible by parameter set name' { + $scriptDefinition = @' +function Test-Function { + [Diagnostics.CodeAnalysis.SuppressMessage('UseSingleValueFromPipelineParameter', 'MySet')] + param( + [Parameter(ValueFromPipeline=$true, ParameterSetName='MySet')] + $InputObject, + + [Parameter(ValueFromPipeline=$true, ParameterSetName='MySet')] + $AnotherParam + ) +} +'@ + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations.Count | Should -Be 0 + } + } +} \ No newline at end of file diff --git a/docs/Rules/README.md b/docs/Rules/README.md index da1058bc2..be503d0ec 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -76,6 +76,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md) | Warning | Yes | | | [UsePSCredentialType](./UsePSCredentialType.md) | Warning | Yes | | | [UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | Yes | | +| [UseSingleValueFromPipelineParameter](./UseSingleValueFromPipelineParameter.md) | Warning | Yes | | | [UseSingularNouns](./UseSingularNouns.md) | Warning | Yes | Yes | | [UseSupportsShouldProcess](./UseSupportsShouldProcess.md) | Warning | Yes | | | [UseToExportFieldsInManifest](./UseToExportFieldsInManifest.md) | Warning | Yes | | diff --git a/docs/Rules/UseSingleValueFromPipelineParameter.md b/docs/Rules/UseSingleValueFromPipelineParameter.md new file mode 100644 index 000000000..0b9f2a1cd --- /dev/null +++ b/docs/Rules/UseSingleValueFromPipelineParameter.md @@ -0,0 +1,101 @@ +--- +description: Use at most a single ValueFromPipeline parameter per parameter set. +ms.date: 08/08/2025 +ms.topic: reference +title: UseSingleValueFromPipelineParameter +--- +# UseSingleValueFromPipelineParameter + +**Severity Level: Warning** + +## Description + +Parameter sets should have at most one parameter marked as +`ValueFromPipeline=true`. + +This rule identifies functions where multiple parameters within the same +parameter set have `ValueFromPipeline` set to `true` (either explicitly or +implicitly). + +## How + +Ensure that only one parameter per parameter set accepts pipeline input by +value. If you need multiple parameters to accept different types of pipeline +input, use separate parameter sets. + +## Example + +### Wrong + +```powershell +function Process-Data { + [CmdletBinding()] + param( + [Parameter(ValueFromPipeline)] + [string]$InputData, + + [Parameter(ValueFromPipeline)] + [string]$ProcessingMode + ) + + process { + Write-Output "$ProcessingMode`: $InputData" + } +} +``` + + +### Correct + +```powershell +function Process-Data { + [CmdletBinding()] + param( + [Parameter(ValueFromPipeline)] + [string]$InputData, + + [Parameter(Mandatory)] + [string]$ProcessingMode + ) + process { + Write-Output "$ProcessingMode`: $InputData" + } +} +``` +## Suppression + +To suppress this rule for a specific parameter set, use the `SuppressMessage` +attribute with the parameter set name: + +```powershell +function Process-Data { + [Diagnostics.CodeAnalysis.SuppressMessage('UseSingleValueFromPipelineParameter', 'MyParameterSet')] + [CmdletBinding()] + param( + [Parameter(ValueFromPipeline, ParameterSetName='MyParameterSet')] + [string]$InputData, + + [Parameter(ValueFromPipeline, ParameterSetName='MyParameterSet')] + [string]$ProcessingMode + ) + process { + Write-Output "$ProcessingMode`: $InputData" + } +} +``` + +For the default parameter set, use `'default'` as the suppression target: + +```powershell +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseSingleValueFromPipelineParameter', 'default')] +``` + +## Notes + +- This rule applies to both explicit `ValueFromPipeline=$true` and implicit + `ValueFromPipeline` (which defaults to `$true`) +- Parameters with `ValueFromPipeline=$false` are not flagged by this rule +- The rule correctly handles the default parameter set (`__AllParameterSets`) + and named parameter sets +- Different parameter sets can each have their own single `ValueFromPipeline` + parameter without triggering this rule From 9b6cbaff4225c085352a8ee37615d8533b040f07 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Sat, 9 Aug 2025 13:33:21 +0100 Subject: [PATCH 2/5] Correct rule name for consistency and to fix failing docs tests --- Rules/UseSingleValueFromPipelineParameter.cs | 6 +++++- Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 | 4 ++-- docs/Rules/UseSingleValueFromPipelineParameter.md | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Rules/UseSingleValueFromPipelineParameter.cs b/Rules/UseSingleValueFromPipelineParameter.cs index 42ae92f67..be123566b 100644 --- a/Rules/UseSingleValueFromPipelineParameter.cs +++ b/Rules/UseSingleValueFromPipelineParameter.cs @@ -173,7 +173,11 @@ private static string GetParameterSetForAttribute(AttributeAst attributeAst) public string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription; - public string GetName() => Strings.UseSingleValueFromPipelineParameterName; + public string GetName() => string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.UseSingleValueFromPipelineParameterName); public RuleSeverity GetSeverity() => RuleSeverity.Warning; diff --git a/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 index e301408d8..77acb0898 100644 --- a/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 +++ b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 @@ -2,7 +2,7 @@ # Licensed under the MIT License. BeforeAll { - $ruleName = 'UseSingleValueFromPipelineParameter' + $ruleName = 'PSUseSingleValueFromPipelineParameter' } Describe 'UseSingleValueFromPipelineParameter' { @@ -309,7 +309,7 @@ function Test-Function { It 'Should be suppressible by parameter set name' { $scriptDefinition = @' function Test-Function { - [Diagnostics.CodeAnalysis.SuppressMessage('UseSingleValueFromPipelineParameter', 'MySet')] + [Diagnostics.CodeAnalysis.SuppressMessage('PSUseSingleValueFromPipelineParameter', 'MySet')] param( [Parameter(ValueFromPipeline=$true, ParameterSetName='MySet')] $InputObject, diff --git a/docs/Rules/UseSingleValueFromPipelineParameter.md b/docs/Rules/UseSingleValueFromPipelineParameter.md index 0b9f2a1cd..9cac391f5 100644 --- a/docs/Rules/UseSingleValueFromPipelineParameter.md +++ b/docs/Rules/UseSingleValueFromPipelineParameter.md @@ -69,7 +69,7 @@ attribute with the parameter set name: ```powershell function Process-Data { - [Diagnostics.CodeAnalysis.SuppressMessage('UseSingleValueFromPipelineParameter', 'MyParameterSet')] + [Diagnostics.CodeAnalysis.SuppressMessage('PSUseSingleValueFromPipelineParameter', 'MyParameterSet')] [CmdletBinding()] param( [Parameter(ValueFromPipeline, ParameterSetName='MyParameterSet')] @@ -93,7 +93,7 @@ For the default parameter set, use `'default'` as the suppression target: ## Notes - This rule applies to both explicit `ValueFromPipeline=$true` and implicit - `ValueFromPipeline` (which defaults to `$true`) + `ValueFromPipeline` (which is the same as using `=$true`) - Parameters with `ValueFromPipeline=$false` are not flagged by this rule - The rule correctly handles the default parameter set (`__AllParameterSets`) and named parameter sets From 834059d5b13bd838ae22aaed9ed689624c5c4792 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Mon, 18 Aug 2025 16:31:44 +0100 Subject: [PATCH 3/5] Update handling of empty AST passed to AnalyzeScript --- Rules/UseSingleValueFromPipelineParameter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/UseSingleValueFromPipelineParameter.cs b/Rules/UseSingleValueFromPipelineParameter.cs index be123566b..d88d0ec44 100644 --- a/Rules/UseSingleValueFromPipelineParameter.cs +++ b/Rules/UseSingleValueFromPipelineParameter.cs @@ -36,7 +36,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) { - yield break; + throw new ArgumentNullException(Strings.NullAstErrorMessage); } // Find all param blocks that have a Parameter attribute with // ValueFromPipeline set to true. From f2266aaaad79b20bcc543c958b925cff655a423e Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Thu, 23 Oct 2025 19:33:02 +0100 Subject: [PATCH 4/5] Fixup built-in rule count test --- Tests/Engine/GetScriptAnalyzerRule.tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 index c3b744803..fbd076af5 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 = 71 + $expectedNumRules = 72 if ($PSVersionTable.PSVersion.Major -le 4) { # for PSv3 PSAvoidGlobalAliases is not shipped because From 14a6c8e91b6ed11e2143c15447299d3b28576824 Mon Sep 17 00:00:00 2001 From: Liam Peters Date: Thu, 23 Oct 2025 19:33:47 +0100 Subject: [PATCH 5/5] Make rule not enabled by default --- Rules/UseSingleValueFromPipelineParameter.cs | 16 +++---- ...SingleValueFromPipelineParameter.Tests.ps1 | 43 +++++++++++-------- docs/Rules/README.md | 2 +- 3 files changed, 35 insertions(+), 26 deletions(-) diff --git a/Rules/UseSingleValueFromPipelineParameter.cs b/Rules/UseSingleValueFromPipelineParameter.cs index d88d0ec44..d3f661ce4 100644 --- a/Rules/UseSingleValueFromPipelineParameter.cs +++ b/Rules/UseSingleValueFromPipelineParameter.cs @@ -22,7 +22,7 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// the same parameter set that are marked as ValueFromPipeline=true, which /// can cause undefined behavior. /// - public class UseSingleValueFromPipelineParameter : IScriptRule + public class UseSingleValueFromPipelineParameter : ConfigurableRule { private const string AllParameterSetsName = "__AllParameterSets"; @@ -32,7 +32,7 @@ public class UseSingleValueFromPipelineParameter : IScriptRule /// The PowerShell Abstract Syntax Tree to analyze. /// The name of the file being analyzed (for diagnostic reporting). /// A collection of diagnostic records for each violating parameter. - public IEnumerable AnalyzeScript(Ast ast, string fileName) + public override IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) { @@ -169,20 +169,20 @@ private static string GetParameterSetForAttribute(AttributeAst attributeAst) .FirstOrDefault(value => !string.IsNullOrWhiteSpace(value)); } - public string GetCommonName() => Strings.UseSingleValueFromPipelineParameterCommonName; + public override string GetCommonName() => Strings.UseSingleValueFromPipelineParameterCommonName; - public string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription; + public override string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription; - public string GetName() => string.Format( + public override string GetName() => string.Format( CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseSingleValueFromPipelineParameterName); - public RuleSeverity GetSeverity() => RuleSeverity.Warning; + public override RuleSeverity GetSeverity() => RuleSeverity.Warning; - public string GetSourceName() => Strings.SourceName; + public override string GetSourceName() => Strings.SourceName; - public SourceType GetSourceType() => SourceType.Builtin; + public override SourceType GetSourceType() => SourceType.Builtin; } } \ No newline at end of file diff --git a/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 index 77acb0898..945130b9e 100644 --- a/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 +++ b/Tests/Rules/UseSingleValueFromPipelineParameter.Tests.ps1 @@ -3,6 +3,15 @@ BeforeAll { $ruleName = 'PSUseSingleValueFromPipelineParameter' + + $settings = @{ + IncludeRules = @($ruleName) + Rules = @{ + $ruleName = @{ + Enable = $true + } + } + } } Describe 'UseSingleValueFromPipelineParameter' { @@ -21,7 +30,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 $violations[0].Message | Should -Match "Multiple parameters \(InputObject, AnotherParam\) in parameter set 'default'" } @@ -38,7 +47,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 } @@ -54,7 +63,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 } @@ -70,7 +79,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 $violations[0].Message | Should -Match "parameter set 'MySet'" } @@ -90,7 +99,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 3 $violations[0].Message | Should -Match 'Multiple parameters \(First, Second, Third\)' } @@ -110,7 +119,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } @@ -126,7 +135,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } @@ -145,7 +154,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } @@ -164,7 +173,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } @@ -183,7 +192,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } @@ -203,7 +212,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } @@ -220,7 +229,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } @@ -236,7 +245,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } @@ -252,7 +261,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } @@ -277,7 +286,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 4 # 2 violations per parameter set, each parameter gets flagged # Check that both parameter sets are mentioned in violations @@ -298,7 +307,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 2 $violations[0].Message | Should -Match "parameter set 'default'" } @@ -319,7 +328,7 @@ function Test-Function { ) } '@ - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule $ruleName + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $settings $violations.Count | Should -Be 0 } } diff --git a/docs/Rules/README.md b/docs/Rules/README.md index be503d0ec..90fb43f6c 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -76,7 +76,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md) | Warning | Yes | | | [UsePSCredentialType](./UsePSCredentialType.md) | Warning | Yes | | | [UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | Yes | | -| [UseSingleValueFromPipelineParameter](./UseSingleValueFromPipelineParameter.md) | Warning | Yes | | +| [UseSingleValueFromPipelineParameter](./UseSingleValueFromPipelineParameter.md) | Warning | No | | | [UseSingularNouns](./UseSingularNouns.md) | Warning | Yes | Yes | | [UseSupportsShouldProcess](./UseSupportsShouldProcess.md) | Warning | Yes | | | [UseToExportFieldsInManifest](./UseToExportFieldsInManifest.md) | Warning | Yes | |