Configurable (enable by default)

Author: iRon7

Rules/AvoidUsingArrayList.cs

@@ -1,37 +1,41 @@
 // 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.Management.Automation.Language;
-using System.Text.RegularExpressions;
-
 #if !CORECLR
 using System.ComponentModel.Composition;
 #endif
+using System.Globalization;
+using System.Management.Automation.Language;
+using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
+using System.Text.RegularExpressions;
 
 namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
 {
+    /// <summary>
+    /// AvoidUsingArrayList: Checks for use of the ArrayList class
+    /// </summary>
 #if !CORECLR
     [Export(typeof(IScriptRule))]
 #endif
-
-    /// <summary>
-    /// Rule that warns when the ArrayList class is used in a PowerShell script.
-    /// </summary>
-    public class AvoidUsingArrayListAsFunctionNames : IScriptRule
+    public class AvoidUsingArrayList : ConfigurableRule
     {
 
         /// <summary>
-        /// Analyzes the PowerShell AST for uses of the ArrayList class.
+        /// Construct an object of AvoidUsingArrayList type.
         /// </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 violation.</returns>
+        public AvoidUsingArrayList() {
+            Enable = true;
+        }
 
-        public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        /// <summary>
+        /// Analyzes the given ast to find the [violation]
+        /// </summary>
+        /// <param name="ast">AST to be analyzed. This should be non-null</param>
+        /// <param name="fileName">Name of file that corresponds to the input AST.</param>
+        /// <returns>A an enumerable type containing the violations</returns>
+        public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
         {
             if (ast == null) { throw new ArgumentNullException(Strings.NullAstErrorMessage); }
 
@@ -118,26 +122,69 @@ testAst is CommandAst cmdAst &&
                         fileName
                     );
                 }
-
             }
-
-
         }
 
-        public string GetCommonName() => Strings.AvoidUsingArrayListCommonName;
+        /// <summary>
+        /// Retrieves the common name of this rule.
+        /// </summary>
+        public override string GetCommonName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingArrayListCommonName);
+        }
 
-        public string GetDescription() => Strings.AvoidUsingArrayListDescription;
+        /// <summary>
+        /// Retrieves the description of this rule.
+        /// </summary>
+        public override string GetDescription()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingArrayListDescription);
+        }
 
-        public string GetName() => string.Format(
+        /// <summary>
+        /// Retrieves the name of this rule.
+        /// </summary>
+        public override string GetName()
+        {
+            return string.Format(
                 CultureInfo.CurrentCulture,
                 Strings.NameSpaceFormat,
                 GetSourceName(),
                 Strings.AvoidUsingArrayListName);
+        }
 
-        public RuleSeverity GetSeverity() => RuleSeverity.Warning;
+        /// <summary>
+        /// Retrieves the severity of the rule: error, warning or information.
+        /// </summary>
+        public override RuleSeverity GetSeverity()
+        {
+            return RuleSeverity.Warning;
+        }
 
-        public string GetSourceName() => Strings.SourceName;
+        /// <summary>
+        /// Gets the severity of the returned diagnostic record: error, warning, or information.
+        /// </summary>
+        /// <returns></returns>
+        public DiagnosticSeverity GetDiagnosticSeverity()
+        {
+            return DiagnosticSeverity.Warning;
+        }
+
+        /// <summary>
+        /// Retrieves the name of the module/assembly the rule is from.
+        /// </summary>
+        public override string GetSourceName()
+        {
+            return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
+        }
 
-        public SourceType GetSourceType() => SourceType.Builtin;
+        /// <summary>
+        /// Retrieves the type of the rule, Builtin, Managed or Module.
+        /// </summary>
+        public override SourceType GetSourceType()
+        {
+            return SourceType.Builtin;
+        }
     }
-}
+}
+

Tests/Rules/AvoidUsingArrayList.tests.ps1

@@ -9,16 +9,14 @@ param()
 BeforeAll {
     $ruleName = "PSAvoidUsingArrayList"
     $ruleMessage = "The ArrayList class is used in '{0}'. Consider using a generic collection or a fixed array instead."
+    $usingCollections = 'using namespace system.collections' + [Environment]::NewLine
+    $usingGeneric = 'using namespace System.Collections.Generic' + [Environment]::NewLine
 }
 
 Describe "AvoidArrayList" {
 
     Context "When there are violations" {
 
-        BeforeAll {
-            $usingCollections = 'using namespace system.collections' + [Environment]::NewLine
-        }
-
         It "Unquoted New-Object type" {
             $scriptDefinition = $usingCollections + {
                 $List = New-Object ArrayList
@@ -166,10 +164,6 @@ Describe "AvoidArrayList" {
 
     Context "When there are no violations" {
 
-        BeforeAll {
-            $usingGeneric = 'using namespace System.Collections.Generic' + [Environment]::NewLine
-        }
-
         It "New-Object List[Object]" {
             $scriptDefinition = {
                 $List = New-Object List[Object]
@@ -208,6 +202,65 @@ Describe "AvoidArrayList" {
         }
     }
 
+    Context "Disabled" {
+
+        BeforeAll {
+            $settings = @{
+                IncludeRules = @($ruleName)
+                Rules        = @{ $ruleName = @{ Enable = $false } }
+            }
+        }
+
+        It "New-Object type" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object ArrayList
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "Type initializer" {
+            $scriptDefinition = $usingCollections + {
+                $List = [ArrayList](1,2,3)
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "New constructor" {
+            $scriptDefinition = $usingCollections + {
+                $List = [ArrayList]::new()
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $violations | Should -BeNullOrEmpty
+        }
+    }
+
+    Context "Explicitly enabled" {
+
+        BeforeAll {
+            $settings = @{
+                IncludeRules = @($ruleName)
+                Rules        = @{ $ruleName = @{ Enable = $true } }
+            }
+        }
+
+        It "New-Object type" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object ArrayList
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {New-Object ArrayList}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {New-Object ArrayList})
+        }
+    }
+
     Context "Test for potential errors" {
 
         It "Dynamic types shouldn't error"  {

docs/Rules/README.md

@@ -28,7 +28,7 @@ The PSScriptAnalyzer contains the following rule definitions.
 | [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md)                           | Warning     |        Yes         |                 |
 | [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md)                                           | Warning     |        Yes         |                 |
 | [AvoidUsingAllowUnencryptedAuthentication](./AvoidUsingAllowUnencryptedAuthentication.md)         | Warning     |        Yes         |                 |
-| [AvoidUsingArrayList](./AvoidUsingArrayList.md)                                                   | Warning     |        Yes         |                 |
+| [AvoidUsingArrayList](./AvoidUsingArrayList.md)                                                   | Warning     |        Yes         |       Yes       |
 | [AvoidUsingBrokenHashAlgorithms](./AvoidUsingBrokenHashAlgorithms.md)                             | Warning     |        Yes         |                 |
 | [AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md)                                           | Warning     |        Yes         | Yes<sup>2</sup> |
 | [AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md)                           | Error       |        Yes         |                 |