Add rule for missing process block when command supports the pipeline

RuleDocumentation/README.md

@@ -50,6 +50,7 @@
 |[UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | |
 |[UseLiteralInitializerForHashtable](./UseLiteralInitializerForHashtable.md) | Warning | |
 |[UseOutputTypeCorrectly](./UseOutputTypeCorrectly.md) | Information | |
+|[UseProcessBlockForPipelineCommands](./UseProcessBlockForPipelineCommands.md) | Warning | |
 |[UsePSCredentialType](./UsePSCredentialType.md) | Warning | |
 |[UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | |
 |[UseSingularNouns<sup>*</sup>](./UseSingularNouns.md) | Warning | |

RuleDocumentation/UseProcessBlockForPipelineCommands.md

@@ -0,0 +1,62 @@
+# UseProcessBlockForPipelineCommands
+
+**Severity Level: Warning**
+
+## Description
+
+Functions that support pipeline input should always handle parameter input in a process block. Unexpected behavior can result if input is handled directly in the body of a function where parameters declare pipeline support.
+
+## Example
+
+### Wrong
+
+``` PowerShell
+Function Get-Number
+{
+	[CmdletBinding()]
+	Param(
+		[Parameter(ValueFromPipeline)]
+		[int]
+		$Number
+	)
+
+	$Number
+}
+```
+
+#### Result
+
+```
+PS C:\> 1..5 | Broken
+5
+```
+
+### Correct
+
+``` PowerShell
+Function Get-Number
+{
+	[CmdletBinding()]
+	Param(
+		[Parameter(ValueFromPipeline)]
+		[int]
+		$Number
+	)
+
+	process
+	{
+		$Number
+	}
+}
+```
+
+#### Result
+
+```
+PS C:\> 1..5 | Correct
+1
+2
+3
+4
+5
+```

Rules/Strings.resx

@@ -1080,4 +1080,16 @@
   <data name="UseCorrectCasingName" xml:space="preserve">
     <value>UseCorrectCasing</value>
   </data>
-</root>
+    <data name="UseProcessBlockForPipelineCommandsCommonName" xml:space="preserve">
+    <value>Use process block for pipeline commands.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandsDescription" xml:space="preserve">
+    <value>If a parameter accepts input via the pipeline, the function should use at least a process block to correctly support pipeline features.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandsError" xml:space="preserve">
+    <value>Commands that support pipeline input should return output in a process block.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandsName" xml:space="preserve">
+    <value>UseProcessBlockForPipelineCommands</value>
+  </data>
+</root>

Rules/UseProcessBlockForPipelineCommands.cs

@@ -0,0 +1,96 @@
+using System;
+using System.Collections.Generic;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+#if !CORECLR
+using System.ComponentModel.Composition;
+#endif
+using System.Globalization;
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    public class UseProcessBlockForPipelineCommands : IScriptRule
+    {
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
+
+            IEnumerable<Ast> functionAsts = ast.FindAll(testAst => testAst is FunctionDefinitionAst, true);
+
+            foreach (FunctionDefinitionAst funcAst in functionAsts)
+            {
+                if
+                (
+                    funcAst.Body.ParamBlock == null
+                    || funcAst.Body.ParamBlock.Attributes == null
+                    || funcAst.Body.ParamBlock.Parameters == null
+                    || funcAst.Body.ProcessBlock != null
+                )
+                { continue; }
+
+                foreach (var paramAst in funcAst.Body.ParamBlock.Parameters)
+                {
+                    foreach (var paramAstAttribute in paramAst.Attributes)
+                    {
+                        if (!(paramAstAttribute is AttributeAst)) { continue;  }
+
+                        var namedArguments = (paramAstAttribute as AttributeAst).NamedArguments;
+                        if (namedArguments == null) { continue; }
+
+                        foreach (NamedAttributeArgumentAst namedArgument in namedArguments)
+                        {
+                            if
+                            (
+                                !String.Equals(namedArgument.ArgumentName, "valuefrompipeline", StringComparison.OrdinalIgnoreCase)
+                                && !String.Equals(namedArgument.ArgumentName, "valuefrompipelinebypropertyname", StringComparison.OrdinalIgnoreCase)
+                            )
+                            { continue; }
+
+                            yield return new DiagnosticRecord(
+                                string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandsError, paramAst.Name.VariablePath.UserPath),
+                                paramAst.Name.Extent,
+                                GetName(),
+                                DiagnosticSeverity.Warning,
+                                fileName,
+                                paramAst.Name.VariablePath.UserPath
+                            );
+                        }
+                    }
+                }
+            }
+        }
+
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.UseProcessBlockForPipelineCommandsName);
+        }
+
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandsCommonName);
+        }
+
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandsDescription);
+        }
+
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}

Tests/Engine/GetScriptAnalyzerRule.tests.ps1

@@ -59,7 +59,7 @@ Describe "Test Name parameters" {
 
         It "get Rules with no parameters supplied" {
 			$defaultRules = Get-ScriptAnalyzerRule
-            $expectedNumRules = 60
+            $expectedNumRules = 61
             if ((Test-PSEditionCoreClr) -or (Test-PSVersionV3) -or (Test-PSVersionV4))
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because

Tests/Rules/UseProcessBlockForPipelineCommands.tests.ps1

@@ -0,0 +1,37 @@
+Describe "UseProcessBlockForPipelineCommands" {
+    BeforeAll {
+        $RuleName = 'PSUseProcessBlockForPipelineCommands'
+        $NoProcessBlock = 'function BadFunc1 { [CmdletBinding()] param ([Parameter(ValueFromPipeline)]$Param1) }'
+        $NoProcessBlockByPropertyName = 'function $BadFunc2 { [CmdletBinding()] param ([Parameter(ValueFromPipelineByPropertyName)]$Param1) }'
+        $HasProcessBlock = 'function GoodFunc1 { [CmdletBinding()] param ([Parameter(ValueFromPipeline)]$Param1) process { } }'
+        $HasProcessBlockByPropertyName = 'function GoodFunc2 { [CmdletBinding()] param ([Parameter(ValueFromPipelineByPropertyName)]$Param1) process { } }'
+        $NoAttribute = 'function GoodFunc3 { [CmdletBinding()] param ([Parameter()]$Param1) }'
+		$HasTypeDeclaration = 'function GoodFunc3 { [CmdletBinding()] param ([Parameter()][string]$Param1) }'
+    }
+
+    Context "When there are violations" {
+        $Cases = @(
+            @{ScriptDefinition = $NoProcessBlock; Name = "NoProcessBlock"}
+            @{ScriptDefinition = $NoProcessBlockByPropertyName; Name = "NoProcessBlockByPropertyName"}
+        )
+        It "has 1 violation for function <Name>" {
+            param ($ScriptDefinition)
+
+            Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName | Should Not BeNullOrEmpty
+        } -TestCases $Cases
+    }
+
+    Context "When there are no violations" {
+        $Cases = @(
+            @{ScriptDefinition = $HasProcessBlock; Name = "HasProcessBlock" }
+            @{ScriptDefinition = $HasProcessBlockByPropertyName; Name = "HasProcessBlockByPropertyName" }
+            @{ScriptDefinition = $NoAttribute; Name = "NoAttribute" }
+			@{ScriptDefinition = $HasTypeDeclaration; Name = "HasTypeDeclaration"}
+        )
+        It "has no violations for function <Name>" {
+            param ($ScriptDefinition)
+
+            Invoke-ScriptAnalyzer -ScriptDefinition $ScriptDefinition -IncludeRule $RuleName | Should BeNullOrEmpty
+        } -TestCases $Cases
+    }
+}