Add AvoidMultipleTypeAttributes rule (#1705)

PowerShell · Aug 27, 2021 · 8db488d · 8db488d
1 parent 6dbf6a1
commit 8db488d
Show file tree

Hide file tree

Showing 7 changed files with 247 additions and 2 deletions.
diff --git a/RuleDocumentation/AvoidMultipleTypeAttributes.md b/RuleDocumentation/AvoidMultipleTypeAttributes.md
@@ -0,0 +1,50 @@
+# AvoidMultipleTypeAttributes
+
+**Severity Level: Warning**
+
+## Description
+
+Parameters should not have more than one type specifier. Multiple type specifiers on parameters will cause a runtime error.
+
+## How
+
+Ensure each parameter has only 1 type specifier.
+
+## Example
+
+### Wrong
+
+``` PowerShell
+function Test-Script
+{
+    [CmdletBinding()]
+    Param
+    (
+        [String]
+        $Param1,
+
+        [switch]
+        [bool]
+        $Switch
+    )
+    ...
+}
+```
+
+### Correct
+
+``` PowerShell
+function Test-Script
+{
+    [CmdletBinding()]
+    Param
+    (
+        [String]
+        $Param1,
+
+        [switch]
+        $Switch
+    )
+    ...
+}
+```
diff --git a/RuleDocumentation/README.md b/RuleDocumentation/README.md
@@ -13,6 +13,7 @@
 |[AvoidGlobalVars](./AvoidGlobalVars.md) | Warning | |
 |[AvoidInvokingEmptyMembers](./AvoidInvokingEmptyMembers.md) | Warning | |
 |[AvoidLongLines](./AvoidLongLines.md) | Warning | |
+|[AvoidMultipleTypeAttributes](./AvoidMultipleTypeAttributes.md) | Warning | |
 |[AvoidOverwritingBuiltInCmdlets](./AvoidOverwritingBuiltInCmdlets.md) | Warning | |
 |[AvoidNullOrEmptyHelpMessageAttribute](./AvoidNullOrEmptyHelpMessageAttribute.md) | Warning | |
 |[AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | |

diff --git a/Rules/AvoidMultipleTypeAttributes.cs b/Rules/AvoidMultipleTypeAttributes.cs
@@ -0,0 +1,104 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+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
+{
+    /// <summary>
+    /// AvoidMultipleTypeAttributes: Check that parameter does not be assigned to multiple types.
+    /// </summary>
+#if !CORECLR
+    [Export(typeof(IScriptRule))]
+#endif
+    public sealed class AvoidMultipleTypeAttributes : IScriptRule
+    {
+        /// <summary>
+        /// AvoidMultipleTypeAttributes: Check that parameter does not be assigned to multiple types.
+        /// </summary>
+        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast is null) 
+            {
+                throw new ArgumentNullException(Strings.NullAstErrorMessage);
+            }
+
+            // Finds all ParamAsts.
+            IEnumerable<Ast> paramAsts = ast.FindAll(testAst => testAst is ParameterAst, searchNestedScriptBlocks: true);
+
+            // Iterates all ParamAsts and check the number of its types.
+            foreach (ParameterAst paramAst in paramAsts)
+            {
+                if (paramAst.Attributes.Where(typeAst => typeAst is TypeConstraintAst).Count() > 1)
+                {
+                    yield return new DiagnosticRecord(
+                        String.Format(CultureInfo.CurrentCulture, Strings.AvoidMultipleTypeAttributesError, paramAst.Name),
+                        paramAst.Name.Extent,
+                        GetName(), 
+                        DiagnosticSeverity.Warning, 
+                        fileName);
+                }
+            }
+        }
+
+        /// <summary>
+        /// GetName: Retrieves the name of this rule.
+        /// </summary>
+        /// <returns>The name of this rule</returns>
+        public string GetName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidMultipleTypeAttributesName);
+        }
+
+        /// <summary>
+        /// GetCommonName: Retrieves the common name of this rule.
+        /// </summary>
+        /// <returns>The common name of this rule</returns>
+        public string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidMultipleTypeAttributesCommonName);
+        }
+
+        /// <summary>
+        /// GetDescription: Retrieves the description of this rule.
+        /// </summary>
+        /// <returns>The description of this rule</returns>
+        public string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidMultipleTypeAttributesDescription);
+        }
+
+        /// <summary>
+        /// GetSourceType: Retrieves the type of the rule, Builtin, Managed or Module.
+        /// </summary>
+        public SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
+
+        /// <summary>
+        /// GetSeverity: Retrieves the severity of the rule: error, warning or information.
+        /// </summary>
+        /// <returns></returns>
+        public RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
+
+        /// <summary>
+        /// GetSourceName: Retrieves the name of the module/assembly the rule is from.
+        /// </summary>
+        public string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
+    }
+}
diff --git a/Rules/Strings.Designer.cs b/Rules/Strings.Designer.cs
diff --git a/Rules/Strings.resx b/Rules/Strings.resx
@@ -1152,4 +1152,16 @@
   <data name="InvalidSyntaxAroundProcessBlockError" xml:space="preserve">
     <value>When using an explicit process block, no preceding code is allowed, only begin, end and dynamicparams blocks.</value>
   </data>
+  <data name="AvoidMultipleTypeAttributesCommonName" xml:space="preserve">
+    <value>Avoid multiple type specifiers on parameters</value>
+  </data>
+  <data name="AvoidMultipleTypeAttributesDescription" xml:space="preserve">
+    <value>Prameter should not have more than one type specifier.</value>
+  </data>
+  <data name="AvoidMultipleTypeAttributesError" xml:space="preserve">
+    <value>Parameter '{0}' has more than one type specifier.</value>
+  </data>
+  <data name="AvoidMultipleTypeAttributesName" xml:space="preserve">
+    <value>AvoidMultipleTypeAttributes</value>
+  </data>
 </root>
diff --git 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 = 65
+            $expectedNumRules = 66
             if ($PSVersionTable.PSVersion.Major -le 4)
             {
                 # for PSv3 PSAvoidGlobalAliases is not shipped because

diff --git a/Tests/Rules/AvoidMultipleTypeAttributes.tests.ps1 b/Tests/Rules/AvoidMultipleTypeAttributes.tests.ps1
@@ -0,0 +1,42 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+BeforeAll {
+    $ruleName = "AvoidMultipleTypeAttributes"
+
+    $settings = @{
+        IncludeRules = @($ruleName)
+    }
+}
+
+Describe 'AvoidMultipleTypeAttributes' {
+    It 'Correctly diagnoses and corrects <Script>' -TestCases @(
+        @{ Script = 'function F1 ($s1, $p1){}' }
+        @{ Script = 'function F2 ([int] $s2, [int] $p2){}' }
+        @{ Script = 'function F3 ([int][switch] $s3, [int] $p3){}';Extent = @{ StartCol = 28; EndCol = 31 }; Message = 'Parameter ''$s3'' has more than one type specifier.' }
+        @{ Script = 'function F4 ([int][ref] $s4, [int] $p4){}';Extent = @{ StartCol = 25; EndCol = 28 }; Message = 'Parameter ''$s4'' has more than one type specifier.' }
+        @{ Script = 'function F5 ([int][switch][boolean] $s5, [int] $p5){}';Extent = @{ StartCol = 37; EndCol = 40 }; Message = 'Parameter ''$s5'' has more than one type specifier.' }
+        @{ Script = 'function F6 ([ValidateSet()][int] $s6, [int] $p6){}' }
+        @{ Script = 'function F7 ([Parameter(Mandatory=$true)][ValidateSet()][int] $s7, [int] $p7){}' }
+    ) {
+        param([string]$Script, $Extent, $Message)
+
+        $diagnostics = Invoke-ScriptAnalyzer -ScriptDefinition $Script
+
+        if (-not $Extent)
+        {
+            $diagnostics | Should -BeNullOrEmpty
+            return
+        }
+
+        $expectedStartLine = if ($Extent.StartLine) { $Extent.StartLine } else { 1 }
+        $expectedEndLine = if ($Extent.EndLine) { $Extent.EndLine } else { 1 }
+
+        $diagnostics.Extent.StartLineNumber | Should -BeExactly $expectedStartLine
+        $diagnostics.Extent.EndLineNumber | Should -BeExactly $expectedEndLine
+        $diagnostics.Extent.StartColumnNumber | Should -BeExactly $Extent.StartCol
+        $diagnostics.Extent.EndColumnNumber | Should -BeExactly $Extent.EndCol
+
+        $diagnostics.Message | Should -BeExactly $Message
+    }
+}