Skip to content

Commit ef4c0de

Browse files
liamjpetersliamjpeters
committed
Added Rule UseSingleValueFromPipelineParameter
1 parent ea70855 commit ef4c0de

File tree

6 files changed

+625
-1
lines changed

6 files changed

+625
-1
lines changed

‎Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1224,4 +1224,16 @@
12241224
<data name="AvoidUsingAllowUnencryptedAuthenticationName" xml:space="preserve">
12251225
<value>AvoidUsingAllowUnencryptedAuthentication</value>
12261226
</data>
1227+
<data name="UseSingleValueFromPipelineParameterCommonName" xml:space="preserve">
1228+
<value>Use a single ValueFromPipeline parameter per parameter set</value>
1229+
</data>
1230+
<data name="UseSingleValueFromPipelineParameterDescription" xml:space="preserve">
1231+
<value>Use at most a single ValueFromPipeline parameter per parameter set to avoid undefined or unexpected behaviour.</value>
1232+
</data>
1233+
<data name="UseSingleValueFromPipelineParameterError" xml:space="preserve">
1234+
<value>Multiple parameters ({0}) in parameter set '{1}' are marked as ValueFromPipeline. Only one parameter per parameter set should accept pipeline input.</value>
1235+
</data>
1236+
<data name="UseSingleValueFromPipelineParameterName" xml:space="preserve">
1237+
<value>UseSingleValueFromPipelineParameter</value>
1238+
</data>
12271239
</root>

‎Rules/UseSingleValueFromPipelineParameter.cs

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Globalization;
8+
using System.Linq;
9+
using System.Management.Automation.Language;
10+
#if !CORECLR
11+
using System.ComponentModel.Composition;
12+
#endif
13+
14+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
15+
{
16+
#if !CORECLR
17+
[Export(typeof(IScriptRule))]
18+
#endif
19+
20+
/// <summary>
21+
/// Rule that identifies parameter blocks with multiple parameters in
22+
/// the same parameter set that are marked as ValueFromPipeline=true, which
23+
/// can cause undefined behavior.
24+
/// </summary>
25+
public class UseSingleValueFromPipelineParameter : IScriptRule
26+
{
27+
private const string AllParameterSetsName = "__AllParameterSets";
28+
29+
/// <summary>
30+
/// Analyzes the PowerShell AST for parameter sets with multiple ValueFromPipeline parameters.
31+
/// </summary>
32+
/// <param name="ast">The PowerShell Abstract Syntax Tree to analyze.</param>
33+
/// <param name="fileName">The name of the file being analyzed (for diagnostic reporting).</param>
34+
/// <returns>A collection of diagnostic records for each violating parameter.</returns>
35+
public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
36+
{
37+
if (ast == null)
38+
{
39+
yield break;
40+
}
41+
// Find all param blocks that have a Parameter attribute with
42+
// ValueFromPipeline set to true.
43+
var paramBlocks = ast.FindAll(testAst => testAst is ParamBlockAst, true)
44+
.Where(paramBlock => paramBlock.FindAll(
45+
attributeAst => attributeAst is AttributeAst attr &&
46+
ParameterAttributeAstHasValueFromPipeline(attr),
47+
true
48+
).Any());
49+
50+
foreach (var paramBlock in paramBlocks)
51+
{
52+
// Find all parameter declarations in the current param block
53+
// Convert the generic ast objects into ParameterAst Objects
54+
// For each ParameterAst, find all it's attributes that have
55+
// ValueFromPipeline set to true (either explicitly or
56+
// implicitly). Flatten the results into a single collection of
57+
// Annonymous objects relating the parameter with it's attribute
58+
// and then group them by parameter set name.
59+
//
60+
//
61+
// https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_parameter_sets?#reserved-parameter-set-name
62+
//
63+
// The default parameter set name is '__AllParameterSets'.
64+
// Not specifying a parameter set name and using the parameter
65+
// set name '__AllParameterSets' are equivalent, so we shouldn't
66+
// treat them like they're different just because one is an
67+
// empty string and the other is not.
68+
//
69+
// Filter the list to only keep parameter sets that have more
70+
// than one ValueFromPipeline parameter.
71+
var parameterSetGroups = paramBlock.FindAll(n => n is ParameterAst, true)
72+
.Cast<ParameterAst>()
73+
.SelectMany(parameter => parameter.FindAll(
74+
a => a is AttributeAst attr && ParameterAttributeAstHasValueFromPipeline(attr),
75+
true
76+
).Cast<AttributeAst>().Select(attr => new { Parameter = parameter, Attribute = attr }))
77+
.GroupBy(item => GetParameterSetForAttribute(item.Attribute) ?? AllParameterSetsName)
78+
.Where(group => group.Count() > 1);
79+
80+
81+
foreach (var group in parameterSetGroups)
82+
{
83+
// __AllParameterSets being the default name is...obscure.
84+
// Instead we'll show the user "default". It's more than
85+
// likely the user has not specified a parameter set name,
86+
// so default will make sense. If they have used 'default'
87+
// as their parameter set name, then we're still correct.
88+
var parameterSetName = group.Key == AllParameterSetsName ? "default" : group.Key;
89+
90+
// Create a concatenated string of parameter names that
91+
// conflict in this parameter set
92+
var parameterNames = string.Join(", ", group.Select(item => item.Parameter.Name.VariablePath.UserPath));
93+
94+
// We emit a diagnostic record for each offending parameter
95+
// attribute in the parameter set so it's obvious where all the
96+
// occurrences are.
97+
foreach (var item in group)
98+
{
99+
var message = string.Format(CultureInfo.CurrentCulture,
100+
Strings.UseSingleValueFromPipelineParameterError,
101+
parameterNames,
102+
parameterSetName);
103+
104+
yield return new DiagnosticRecord(
105+
message,
106+
item.Attribute.Extent,
107+
GetName(),
108+
DiagnosticSeverity.Warning,
109+
fileName,
110+
parameterSetName);
111+
}
112+
}
113+
}
114+
}
115+
116+
/// <summary>
117+
/// Returns whether the specified AttributeAst represents a Parameter attribute
118+
/// that has the ValueFromPipeline named argument set to true (either explicitly or
119+
/// implicitly).
120+
/// </summary>
121+
/// <param name="attributeAst">The Parameter attribute to examine.</param>
122+
/// <returns>Whether the attribute has the ValueFromPipeline named argument set to true.</returns>
123+
private static bool ParameterAttributeAstHasValueFromPipeline(AttributeAst attributeAst)
124+
{
125+
// Exit quickly if the attribute is null, has no named arguments, or
126+
// is not a parameter attribute.
127+
if (attributeAst?.NamedArguments == null ||
128+
!string.Equals(attributeAst.TypeName?.Name, "Parameter", StringComparison.OrdinalIgnoreCase))
129+
{
130+
return false;
131+
}
132+
133+
return attributeAst.NamedArguments
134+
.OfType<NamedAttributeArgumentAst>()
135+
.Any(namedArg => string.Equals(
136+
namedArg?.ArgumentName,
137+
"ValueFromPipeline",
138+
StringComparison.OrdinalIgnoreCase
139+
// Helper.Instance.GetNamedArgumentAttributeValue handles both explicit ($true)
140+
// and implicit (no value specified) ValueFromPipeline declarations
141+
) && Helper.Instance.GetNamedArgumentAttributeValue(namedArg));
142+
}
143+
144+
/// <summary>
145+
/// Gets the ParameterSetName value from a Parameter attribute.
146+
/// </summary>
147+
/// <param name="attributeAst">The Parameter attribute to examine.</param>
148+
/// <returns>The parameter set name, or null if not found or empty.</returns>
149+
private static string GetParameterSetForAttribute(AttributeAst attributeAst)
150+
{
151+
// Exit quickly if the attribute is null, has no named arguments, or
152+
// is not a parameter attribute.
153+
if (attributeAst?.NamedArguments == null ||
154+
!string.Equals(attributeAst.TypeName.Name, "Parameter", StringComparison.OrdinalIgnoreCase))
155+
{
156+
return null;
157+
}
158+
159+
return attributeAst.NamedArguments
160+
.OfType<NamedAttributeArgumentAst>()
161+
.Where(namedArg => string.Equals(
162+
namedArg?.ArgumentName,
163+
"ParameterSetName",
164+
StringComparison.OrdinalIgnoreCase
165+
))
166+
.Select(namedArg => namedArg?.Argument)
167+
.OfType<StringConstantExpressionAst>()
168+
.Select(stringConstAst => stringConstAst?.Value)
169+
.FirstOrDefault(value => !string.IsNullOrWhiteSpace(value));
170+
}
171+
172+
public string GetCommonName() => Strings.UseSingleValueFromPipelineParameterCommonName;
173+
174+
public string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription;
175+
176+
public string GetName() => Strings.UseSingleValueFromPipelineParameterName;
177+
178+
public RuleSeverity GetSeverity() => RuleSeverity.Warning;
179+
180+
public string GetSourceName() => Strings.SourceName;
181+
182+
public SourceType GetSourceType() => SourceType.Builtin;
183+
}
184+
}

‎Tests/Engine/GetScriptAnalyzerRule.tests.ps1

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Describe "Test Name parameters" {
6363

6464
It "get Rules with no parameters supplied" {
6565
$defaultRules = Get-ScriptAnalyzerRule
66-
$expectedNumRules = 70
66+
$expectedNumRules = 71
6767
if ($PSVersionTable.PSVersion.Major -le 4)
6868
{
6969
# for PSv3 PSAvoidGlobalAliases is not shipped because

0 commit comments

Comments
 (0)