Add UseSingleValueFromPipelineParameter Rule

Rules/Strings.resx

@@ -1236,4 +1236,16 @@
   <data name="AvoidReservedWordsAsFunctionNamesError" xml:space="preserve">
     <value>The reserved word '{0}' was used as a function name. This should be avoided.</value>
   </data>
+  <data name="UseSingleValueFromPipelineParameterCommonName" xml:space="preserve">
+    <value>Use a single ValueFromPipeline parameter per parameter set</value>
+  </data>
+  <data name="UseSingleValueFromPipelineParameterDescription" xml:space="preserve">
+    <value>Use at most a single ValueFromPipeline parameter per parameter set to avoid undefined or unexpected behaviour.</value>
+  </data>
+  <data name="UseSingleValueFromPipelineParameterError" xml:space="preserve">
+    <value>Multiple parameters ({0}) in parameter set '{1}' are marked as ValueFromPipeline. Only one parameter per parameter set should accept pipeline input.</value>
+  </data>
+  <data name="UseSingleValueFromPipelineParameterName" xml:space="preserve">
+    <value>UseSingleValueFromPipelineParameter</value>
+  </data>
 </root>

Rules/UseSingleValueFromPipelineParameter.cs

@@ -0,0 +1,188 @@
+// 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
+
+    /// <summary>
+    /// Rule that identifies parameter blocks with multiple parameters in
+    /// the same parameter set that are marked as ValueFromPipeline=true, which
+    /// can cause undefined behavior.
+    /// </summary>
+    public class UseSingleValueFromPipelineParameter : ConfigurableRule
+    {
+        private const string AllParameterSetsName = "__AllParameterSets";
+
+        /// <summary>
+        /// Analyzes the PowerShell AST for parameter sets with multiple ValueFromPipeline parameters.
+        /// </summary>
+        /// <param name="ast">The PowerShell Abstract Syntax Tree to analyze.</param>
+        /// <param name="fileName">The name of the file being analyzed (for diagnostic reporting).</param>
+        /// <returns>A collection of diagnostic records for each violating parameter.</returns>
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException(Strings.NullAstErrorMessage);
+            }
+            // 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<ParameterAst>()
+                    .SelectMany(parameter => parameter.FindAll(
+                        a => a is AttributeAst attr && ParameterAttributeAstHasValueFromPipeline(attr),
+                        true
+                    ).Cast<AttributeAst>().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);
+                    }
+                }
+            }
+        }
+
+        /// <summary>
+        /// Returns whether the specified AttributeAst represents a Parameter attribute
+        /// that has the ValueFromPipeline named argument set to true (either explicitly or
+        /// implicitly).
+        /// </summary>
+        /// <param name="attributeAst">The Parameter attribute to examine.</param>
+        /// <returns>Whether the attribute has the ValueFromPipeline named argument set to true.</returns>
+        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<NamedAttributeArgumentAst>()
+                .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));
+        }
+
+        /// <summary>
+        /// Gets the ParameterSetName value from a Parameter attribute.
+        /// </summary>
+        /// <param name="attributeAst">The Parameter attribute to examine.</param>
+        /// <returns>The parameter set name, or null if not found or empty.</returns>
+        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<NamedAttributeArgumentAst>()
+                .Where(namedArg => string.Equals(
+                    namedArg?.ArgumentName,
+                    "ParameterSetName",
+                    StringComparison.OrdinalIgnoreCase
+                ))
+                .Select(namedArg => namedArg?.Argument)
+                .OfType<StringConstantExpressionAst>()
+                .Select(stringConstAst => stringConstAst?.Value)
+                .FirstOrDefault(value => !string.IsNullOrWhiteSpace(value));
+        }
+
+        public override string GetCommonName() => Strings.UseSingleValueFromPipelineParameterCommonName;
+
+        public override string GetDescription() => Strings.UseSingleValueFromPipelineParameterDescription;
+
+        public override string GetName() => string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.UseSingleValueFromPipelineParameterName);
+
+        public override RuleSeverity GetSeverity() => RuleSeverity.Warning;
+
+        public override string GetSourceName() => Strings.SourceName;
+
+        public override SourceType GetSourceType() => SourceType.Builtin;
+    }
+}

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