Add rule for missing process block when command supports the pipeline (…

…#1373) * Add rule for missing process block #1368 * Fix build by regenerating strongly typed files by running '.\New-StronglyTypedCsFileForResx.ps1 Rules' * Update strongly typed resources via Visual Studio * minimise diff in resx file * refactor UseProcessBlockForPipelineCommands rule * add tests and docs for UseProcessBlockForPipelineCommands rule * Fix logic non-pipeline function and add test case * fixing some test failures * incrememt number of expected rules * fix style suggestions for PR #1373 * fix rule readme.md * clean diff for Strings.resx to avoid future merge conflicts * Fix last 2 rule documentation test failures by fixing markdown link * Update Rules/Strings.resx Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]> * Update Rules/Strings.resx Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]> * Update Rules/Strings.resx Co-Authored-By: Christoph Bergmeister [MVP] <[email protected]> * empty-commit to re-trigger CI due to sporadic failure * Update Rules/Strings.resx Co-Authored-By: Robert Holt <[email protected]> * Update Rules/Strings.resx Co-Authored-By: Robert Holt <[email protected]> * corrections to use process block rule and tests - candidate for merge * More style fixes for #1373 * update localized strings * replace implicitly typed vars
PowerShell · Dec 9, 2019 · 4bc3911 · 4bc3911
1 parent 20c4611
commit 4bc3911
Show file tree

Hide file tree

Showing 7 changed files with 304 additions and 57 deletions.
diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md
@@ -50,6 +50,7 @@
 |[UseDeclaredVarsMoreThanAssignments](./UseDeclaredVarsMoreThanAssignments.md) | Warning | |
 |[UseLiteralInitializerForHashtable](./UseLiteralInitializerForHashtable.md) | Warning | |
 |[UseOutputTypeCorrectly](./UseOutputTypeCorrectly.md) | Information | |
+|[UseProcessBlockForPipelineCommand](./UseProcessBlockForPipelineCommand.md) | Warning | |
 |[UsePSCredentialType](./UsePSCredentialType.md) | Warning | |
 |[UseShouldProcessForStateChangingFunctions](./UseShouldProcessForStateChangingFunctions.md) | Warning | |
 |[UseSingularNouns<sup>*</sup>](./UseSingularNouns.md) | Warning | |

diff --git a/RuleDocumentation/UseProcessBlockForPipelineCommand.md b/RuleDocumentation/UseProcessBlockForPipelineCommand.md
@@ -0,0 +1,62 @@
+# UseProcessBlockForPipelineCommand
+
+**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 | Get-Number
+5
+```
+
+### Correct
+
+``` PowerShell
+Function Get-Number
+{
+	[CmdletBinding()]
+	Param(
+		[Parameter(ValueFromPipeline)]
+		[int]
+		$Number
+	)
+	
+	process
+	{
+		$Number
+	}
+}
+```
+
+#### Result
+
+```
+PS C:\> 1..5 | Get-Number
+1
+2
+3
+4
+5
+```
diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs
diff --git a/Rules/Strings.resx b/Rules/Strings.resx
@@ -1080,4 +1080,16 @@
   <data name="UseCorrectCasingName" xml:space="preserve">
     <value>UseCorrectCasing</value>
   </data>
+  <data name="UseProcessBlockForPipelineCommandCommonName" xml:space="preserve">
+    <value>Use process block for command that accepts input from pipeline.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandDescription" xml:space="preserve">
+    <value>If a command parameter takes its value from the pipeline, the command must use a process block to bind the input objects from the pipeline to that parameter.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandError" xml:space="preserve">
+    <value>Command accepts pipeline input but has not defined a process block.</value>
+  </data>
+  <data name="UseProcessBlockForPipelineCommandName" xml:space="preserve">
+    <value>UseProcessBlockForPipelineCommand</value>
+  </data>
 </root>
diff --git a/Rules/UseProcessBlockForPipelineCommand.cs b/Rules/UseProcessBlockForPipelineCommand.cs
@@ -0,0 +1,98 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+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 UseProcessBlockForPipelineCommand : IScriptRule
+    {
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null)
+            {
+                throw new ArgumentNullException(Strings.NullAstErrorMessage);
+            }
+
+            IEnumerable<Ast> scriptblockAsts = ast.FindAll(testAst => testAst is ScriptBlockAst, true);
+
+            foreach (ScriptBlockAst scriptblockAst in scriptblockAsts)
+            {
+                if (scriptblockAst.ProcessBlock != null
+                    || scriptblockAst.ParamBlock?.Attributes == null
+                    || scriptblockAst.ParamBlock.Parameters == null)
+                {
+                    continue;
+                }
+
+                foreach (ParameterAst paramAst in scriptblockAst.ParamBlock.Parameters)
+                {
+                    foreach (AttributeBaseAst paramAstAttribute in paramAst.Attributes)
+                    {
+                        if (!(paramAstAttribute is AttributeAst paramAttributeAst)) { continue; }
+
+                        if (paramAttributeAst.NamedArguments == null) { continue; }
+
+                        foreach (NamedAttributeArgumentAst namedArgument in paramAttributeAst.NamedArguments)
+                        {
+                            if (!namedArgument.ArgumentName.Equals("valuefrompipeline", StringComparison.OrdinalIgnoreCase)
+                                && !namedArgument.ArgumentName.Equals("valuefrompipelinebypropertyname", StringComparison.OrdinalIgnoreCase))
+                            {
+                                continue;
+                            }
+
+                            yield return new DiagnosticRecord(
+                                string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandError, 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.UseProcessBlockForPipelineCommandName);
+        }
+
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandCommonName);
+        }
+
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.UseProcessBlockForPipelineCommandDescription);
+        }
+
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}
diff --git a/Tests/Engine/GetScriptAnalyzerRule.tests.ps1 b/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