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

@@ -932,6 +932,18 @@
   </data>
   <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>

Tests/Rules/AvoidUsingArrayList.ps1

@@ -0,0 +1,19 @@
+using namespace system.collections
+
+# Using New-Object
+$List = New-Object ArrayList
+$List = New-Object 'ArrayList'
+$List = New-Object "ArrayList"
+$List = New-Object -Type ArrayList
+$List = New-Object -TypeName ArrayLIST
+$List = New-Object Collections.ArrayList
+$List = New-Object System.Collections.ArrayList
+
+# Using type initializer
+$List = [ArrayList](1,2,3)
+$List = [ArrayLIST]@(1,2,3)
+$List = [ArrayList]::new()
+$List = [Collections.ArrayList]::New()
+$List = [System.Collections.ArrayList]::new()
+
+1..3 | ForEach-Object { $null = $List.Add($_) }

Tests/Rules/AvoidUsingArrayList.tests.ps1

@@ -0,0 +1,64 @@
+# 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" {
+
+    BeforeDiscovery {
+        $violationFileName = "$PSScriptRoot\AvoidUsingArrayList.ps1"
+        $violationExtents = [Parser]::ParseFile($violationFileName, [ref] $null, [ref] $null).FindAll({
+            $Args[0] -is [AssignmentStatementAst] -and
+            $Args[0].Left.Extent.Text -eq '$List'
+        }, $false).Right.Extent
+    }
+
+    Context "When there are violations" {
+
+        BeforeAll {
+            $violationFileName = "$PSScriptRoot\AvoidUsingArrayList.ps1"
+            $violations = Invoke-ScriptAnalyzer $violationFileName | Where-Object RuleName -eq $ruleName
+            $violationLines = @{}
+            foreach ($violation in $violations) { $violationLines[$violation.Line] = $violation }
+        }
+
+        It "Should return 12 violations" {
+            $violations.Count | Should -Be 12
+        }
+
+        It "Each violation should contain" -ForEach $violationExtents {
+            $violation = $violationLines[$_.StartLineNumber]
+            $violation.Extent.Text | Should -Be $_.Text
+            $violation.Message     | Should -Be ($ruleMessage -f $_.Text)
+            $violation.Severity    | Should -Be Warning
+            $violation.ScriptName  | Should -Be AvoidUsingArrayList.ps1
+        }
+
+
+        It "Dynamic types shouldn't error"  {
+            # but aren't covered by the rule
+
+            $scriptDefinition = {
+                $type = "System.Collections.ArrayList"
+                New-Object -TypeName '$type'
+            }.ToString()
+
+            $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
+            $violations | Should -BeNullOrEmpty
+        }
+    }
+
+    Context "When there are no violations" {
+        It "returns no violations" {
+            $noViolations.Count | Should -Be 0
+        }
+    }
+}

Tests/Rules/AvoidUsingArrayListNoViolations.ps1

@@ -0,0 +1,19 @@
+using namespace System.Collections.Generic
+
+# Using a generic List
+$List = New-Object List[Object]
+1..3 | ForEach-Object { $List.Add($_) } # This will not return anything
+
+$List = [List[Object]]::new()
+1..3 | ForEach-Object { $List.Add($_) } # This will not return anything
+
+# Creating a fixed array by using the PowerShell pipeline
+$List = 1..3 | ForEach-Object { $_ }
+
+# This should not violate because there isn't a
+# `using namespace System.Collections` directive
+# and ArrayList could belong to another namespace
+$List = New-Object ArrayList
+$List = [ArrayList](1,2,3)
+$List = [ArrayList]@(1,2,3)
+$List = [ArrayList]::new()

docs/Rules/AvoidUsingArrayList.md

@@ -0,0 +1,52 @@
+---
+description: Avoid using ArrayList
+ms.date: 04/16/2025
+ms.topic: reference
+title: AvoidUsingArrayList
+---
+# AvoidUsingArrayList
+
+**Severity Level: Warning**
+
+## Description
+
+Per dotnet best practices, the
+[`ArrayList` class](https://learn.microsoft.com/dotnet/api/system.collections.arraylist)
+is not recommended for new development, the same recommendation applies to PowerShell:
+
+Avoid the ArrayList class for new development.
+The `ArrayList` class is a non-generic collection that can hold objects of any type. This is inline with the fact
+that PowerShell is a weakly typed language. However, the `ArrayList` class does not provide any explicit type
+safety and performance benefits of generic collections. Instead of using an `ArrayList`, consider using either a
+[`System.Collections.Generic.List[Object]`](https://learn.microsoft.com/dotnet/api/system.collections.generic.list-1)
+class or a fixed PowerShell array.
+Besides, the `ArrayList.Add` method returns the index of the added element which often unintentionally pollutes the
+PowerShell pipeline and therefore might cause unexpected issues.
+
+## How to Fix
+
+In cases where only the `Add` method is used, you might just replace the `ArrayList` class with a generic
+`List[Object]` class but you could also consider using the idiomatic PowerShell pipeline syntax instead.
+
+## Example
+
+### Wrong
+
+```powershell
+# Using an ArrayList
+$List = [System.Collections.ArrayList]::new()
+1..3 | ForEach-Object { $List.Add($_) } # Note that this will return the index of the added element
+```
+
+### Correct
+
+```powershell
+# Using a generic List
+$List = [System.Collections.Generic.List[Object]]::new()
+1..3 | ForEach-Object { $List.Add($_) } # This will not return anything
+```
+
+```PowerShell
+# Creating a fixed array by using the PowerShell pipeline
+$List = 1..3 | ForEach-Object { $_ }
+```

docs/Rules/README.md

@@ -28,6 +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         |                 |
 | [AvoidUsingBrokenHashAlgorithms](./AvoidUsingBrokenHashAlgorithms.md)                             | Warning     |        Yes         |                 |
 | [AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md)                                           | Warning     |        Yes         | Yes<sup>2</sup> |
 | [AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md)                           | Error       |        Yes         |                 |