Add new AvoidUsingArrayList rule

Rules/AvoidUsingArrayList.cs

@@ -0,0 +1,143 @@
+// 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
+
+namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
+{
+#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
+    {
+
+        /// <summary>
+        /// Analyzes the PowerShell AST for uses of the ArrayList class.
+        /// </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 IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
+        {
+            if (ast == null) { throw new ArgumentNullException(Strings.NullAstErrorMessage); }
+
+            // If there is an using statement for the Collections namespace, check for the full typename.
+            // Otherwise also check for the bare ArrayList name.
+            Regex arrayListName = null;
+            var sbAst = ast as ScriptBlockAst;
+            foreach (UsingStatementAst usingAst in sbAst.UsingStatements)
+            {
+                if (
+                    usingAst.UsingStatementKind == UsingStatementKind.Namespace &&
+                    (
+                        usingAst.Name.Value.Equals("Collections", StringComparison.OrdinalIgnoreCase) ||
+                        usingAst.Name.Value.Equals("System.Collections", StringComparison.OrdinalIgnoreCase)
+                    )
+                )
+                {
+                    arrayListName = new Regex(@"^((System\.)?Collections\.)?ArrayList$", RegexOptions.IgnoreCase);
+                    break;
+                }
+            }
+            if (arrayListName == null) { arrayListName = new Regex(@"^(System\.)?Collections\.ArrayList$", RegexOptions.IgnoreCase); }
+
+            // Find all type initializers that create a new instance of the ArrayList class.
+            IEnumerable<Ast> typeAsts = ast.FindAll(testAst =>
+                (
+                    testAst is ConvertExpressionAst convertAst &&
+                    convertAst.StaticType != null &&
+                    convertAst.StaticType.FullName == "System.Collections.ArrayList"
+                ) ||
+                (
+                    testAst is TypeExpressionAst typeAst &&
+                    typeAst.TypeName != null &&
+                    arrayListName.IsMatch(typeAst.TypeName.Name) &&
+                    typeAst.Parent is InvokeMemberExpressionAst parentAst &&
+                    parentAst.Member != null &&
+                    parentAst.Member is StringConstantExpressionAst memberAst &&
+                    memberAst.Value.Equals("new", StringComparison.OrdinalIgnoreCase)
+                ),
+                true
+            );
+
+            foreach (Ast typeAst in typeAsts)
+            {
+                yield return new DiagnosticRecord(
+                    string.Format(
+                        CultureInfo.CurrentCulture,
+                        Strings.AvoidUsingArrayListError,
+                        typeAst.Parent.Extent.Text),
+                    typeAst.Parent.Extent,
+                    GetName(),
+                    DiagnosticSeverity.Warning,
+                    fileName
+                );
+            }
+
+            // Find all New-Object cmdlets that create a new instance of the ArrayList class.
+            var newObjectCommands = ast.FindAll(testAst =>
+                testAst is CommandAst cmdAst &&
+                cmdAst.GetCommandName() != null &&
+                cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase),
+                true);
+
+            foreach (CommandAst cmd in newObjectCommands)
+            {
+                // Use StaticParameterBinder to reliably get parameter values
+                var bindingResult = StaticParameterBinder.BindCommand(cmd, true);
+
+                // Check for -TypeName parameter
+                if (
+                    bindingResult.BoundParameters.ContainsKey("TypeName") &&
+                    bindingResult.BoundParameters["TypeName"] != null &&
+                    arrayListName.IsMatch(bindingResult.BoundParameters["TypeName"].ConstantValue as string)
+                )
+                {
+                    yield return new DiagnosticRecord(
+                        string.Format(
+                            CultureInfo.CurrentCulture,
+                            Strings.AvoidUsingArrayListError,
+                            cmd.Extent.Text),
+                        cmd.Extent,
+                        GetName(),
+                        DiagnosticSeverity.Warning,
+                        fileName
+                    );
+                }
+
+            }
+
+
+        }
+
+        public string GetCommonName() => Strings.AvoidUsingArrayListCommonName;
+
+        public string GetDescription() => Strings.AvoidUsingArrayListDescription;
+
+        public string GetName() => string.Format(
+                CultureInfo.CurrentCulture,
+                Strings.NameSpaceFormat,
+                GetSourceName(),
+                Strings.AvoidUsingArrayListName);
+
+        public RuleSeverity GetSeverity() => RuleSeverity.Warning;
+
+        public string GetSourceName() => Strings.SourceName;
+
+        public SourceType GetSourceType() => SourceType.Builtin;
+    }
+}

Rules/Strings.resx

@@ -933,6 +933,18 @@
   <data name="AvoidSemicolonsAsLineTerminatorsError" xml:space="preserve">
     <value>Line ends with a semicolon</value>
   </data>
+  <data name="AvoidUsingArrayListCommonName" xml:space="preserve">
+    <value>Avoid using the ArrayList class</value>
+  </data>
+  <data name="AvoidUsingArrayListDescription" xml:space="preserve">
+      <value>Avoid using the ArrayList class in PowerShell scripts. Consider using generic collections or fixed arrays instead.</value>
+  </data>
+  <data name="AvoidUsingArrayListName" xml:space="preserve">
+      <value>AvoidUsingArrayList</value>
+  </data>
+  <data name="AvoidUsingArrayListError" xml:space="preserve">
+      <value>The ArrayList class is used in '{0}'. Consider using a generic collection or a fixed array instead.</value>
+  </data>
   <data name="PlaceOpenBraceName" xml:space="preserve">
     <value>PlaceOpenBrace</value>
   </data>

Tests/Rules/AvoidUsingArrayList.tests.ps1

@@ -0,0 +1,223 @@
+# Copyright (c) Microsoft Corporation. All rights reserved.
+# Licensed under the MIT License.
+
+using namespace System.Management.Automation.Language
+
+[Diagnostics.CodeAnalysis.SuppressMessage('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'False positive')]
+param()
+
+BeforeAll {
+    $ruleName = "PSAvoidUsingArrayList"
+    $ruleMessage = "The ArrayList class is used in '{0}'. Consider using a generic collection or a fixed array instead."
+}
+
+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
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $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})
+        }
+
+        It "Single quoted New-Object type" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object 'ArrayList'
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $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'})
+        }
+
+        It "Double quoted New-Object type" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object "ArrayList"
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $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"})
+        }
+
+        It "New-Object with full parameter name" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object -TypeName ArrayList
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {New-Object -TypeName ArrayList}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {New-Object -TypeName ArrayList})
+        }
+
+        It "New-Object with abbreviated parameter name and odd casing" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object -Type ArrayLIST
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {New-Object -Type ArrayLIST}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {New-Object -Type ArrayLIST})
+        }
+
+        It "New-Object with full type name" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object -TypeName System.Collections.ArrayList
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {New-Object -TypeName System.Collections.ArrayList}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {New-Object -TypeName System.Collections.ArrayList})
+        }
+
+        It "New-Object with semi full type name and odd casing" {
+            $scriptDefinition = $usingCollections + {
+                $List = New-Object COLLECTIONS.ArrayList
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {New-Object COLLECTIONS.ArrayList}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {New-Object COLLECTIONS.ArrayList})
+        }
+
+        It "Type initializer with 3 parameters" {
+            $scriptDefinition = $usingCollections + {
+                $List = [ArrayList](1,2,3)
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {[ArrayList](1,2,3)}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {[ArrayList](1,2,3)})
+        }
+
+        It "Type initializer with array parameters" {
+            $scriptDefinition = $usingCollections + {
+                $List = [ArrayList]@(1,2,3)
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {[ArrayList]@(1,2,3)}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {[ArrayList]@(1,2,3)})
+        }
+
+        It "Type initializer with new constructor" {
+            $scriptDefinition = $usingCollections + {
+                $List = [ArrayList]::new()
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {[ArrayList]::new()}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {[ArrayList]::new()})
+        }
+
+        It "Full type name initializer with new constructor" {
+            $scriptDefinition = $usingCollections + {
+                $List = [System.Collections.ArrayList]::new()
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {[System.Collections.ArrayList]::new()}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {[System.Collections.ArrayList]::new()})
+        }
+
+        It "Semi full type name initializer with new constructor and odd casing" {
+            $scriptDefinition = $usingCollections + {
+                $List = [COLLECTIONS.ArrayList]::new()
+                1..3 | ForEach-Object { $null = $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations.Count       | Should -Be 1
+            $violations.Severity    | Should -Be Warning
+            $violations.Extent.Text | Should -Be {[COLLECTIONS.ArrayList]::new()}.ToString()
+            $violations.Message     | Should -Be ($ruleMessage -f {[COLLECTIONS.ArrayList]::new()})
+        }
+    }
+
+    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]
+                1..3 | ForEach-Object { $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "[List[Object]]::new()" {
+            $scriptDefinition = {
+                $List = [List[Object]]::new()
+                1..3 | ForEach-Object { $List.Add($_) }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "Using the pipeline" {
+            $scriptDefinition = {
+                $List = 1..3 | ForEach-Object { $_ }
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations | Should -BeNullOrEmpty
+        }
+
+        It "Out of the namespace scope" {
+            $scriptDefinition = $usingGeneric + {
+                $List = New-Object ArrayList
+                $List = [ArrayList](1,2,3)
+                $List = [ArrayList]@(1,2,3)
+                $List = [ArrayList]::new()
+            }.ToString()
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations | Should -BeNullOrEmpty
+        }
+    }
+
+    Context "Test for potential errors" {
+
+        It "Dynamic types shouldn't error"  {
+            $scriptDefinition = {
+                $type = "System.Collections.ArrayList"
+                New-Object -TypeName '$type'
+            }.ToString()
+
+            $analyzer = { Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) }
+            $analyzer | Should -Not -Throw # but won't violate either (too complex to cover)
+        }
+    }
+}