Skip to content

Commit 6dadae3

Browse files
iRon7liamjpetersCopilot
authored
Add new AvoidUsingArrayList rule (#2174)
* Implemented the AvoidUsingArrayList rule to warn when the ArrayList class is used in PowerShell scripts. Added tests for both violations and non-violations of this rule. Updated documentation to include the new rule and its guidelines. * Testing-Commit-CSpell-issue * Apply suggestion from @liamjpeters I clearly used several other rules as a kind of template...🤪 Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Update docs/Rules/AvoidUsingArrayList.md Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Updated rule help * Changed "unintentionally" * Update Rules/AvoidUsingArrayList.cs Co-authored-by: Liam Peters <liamjpeters@gmail.com> * Update Tests/Rules/AvoidUsingArrayList.tests.ps1 Co-authored-by: Liam Peters <liamjpeters@gmail.com> * ArrayListName could be null * Resolved camelCase * Remove ComponentModel namespace * Fixed tests * Updated Tests * fixed and tested empty (dynamic) BoundParameter * Robuster Pester tests * Configurable (enable by default) * Fixed ConstantValue null check test and rule * Better UsingStatements handling and disable AvoidUsingArrayList by default * `[ArrayList]::new()` without a `using namespace System.Collections` * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Resolve Copilot suggestions and resolved `Method not found: ScriptBlockAst.get_UsingStatements()` error --------- Co-authored-by: Liam Peters <liamjpeters@gmail.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 97f30d6 commit 6dadae3

5 files changed

Lines changed: 550 additions & 0 deletions

File tree

Rules/AvoidUsingArrayList.cs

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
#if !CORECLR
7+
using System.ComponentModel.Composition;
8+
#endif
9+
using System.Globalization;
10+
using System.Management.Automation.Language;
11+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
12+
using System.Text.RegularExpressions;
13+
using System.Linq;
14+
15+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
16+
{
17+
/// <summary>
18+
/// AvoidUsingArrayList: Checks for use of the ArrayList class
19+
/// </summary>
20+
#if !CORECLR
21+
[Export(typeof(IScriptRule))]
22+
#endif
23+
public class AvoidUsingArrayList : ConfigurableRule
24+
{
25+
26+
/// <summary>
27+
/// Construct an object of AvoidUsingArrayList type.
28+
/// </summary>
29+
public AvoidUsingArrayList() {
30+
Enable = false;
31+
}
32+
33+
/// <summary>
34+
/// Analyzes the given ast to find the [violation]
35+
/// </summary>
36+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
37+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
38+
/// <returns>A an enumerable type containing the violations</returns>
39+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
40+
{
41+
if (ast == null) { throw new ArgumentNullException(nameof(ast), Strings.NullAstErrorMessage); }
42+
43+
// If there is an using statement for the Collections namespace, check for the full typename.
44+
// Otherwise also check for the bare ArrayList name.
45+
Regex arrayListName = null;
46+
if (ast is ScriptBlockAst sbAst) {
47+
// sbAst.UsingStatements causes an error: Method not found: ScriptBlockAst.get_UsingStatements()
48+
IEnumerable<Ast> usingStatements = usingStatements = sbAst.FindAll(testAst => testAst is UsingStatementAst, false);
49+
foreach (UsingStatementAst usingAst in usingStatements.Cast<UsingStatementAst>())
50+
{
51+
if (
52+
usingAst.UsingStatementKind == UsingStatementKind.Namespace &&
53+
(
54+
usingAst.Name.Value.Equals("Collections", StringComparison.OrdinalIgnoreCase) ||
55+
usingAst.Name.Value.Equals("System.Collections", StringComparison.OrdinalIgnoreCase)
56+
)
57+
)
58+
{
59+
arrayListName = new Regex(@"^((System\.)?Collections\.)?ArrayList$", RegexOptions.IgnoreCase);
60+
break;
61+
}
62+
}
63+
}
64+
if (arrayListName == null) { arrayListName = new Regex(@"^(System\.)?Collections\.ArrayList$", RegexOptions.IgnoreCase); }
65+
66+
// Find all type initializers that create a new instance of the ArrayList class.
67+
IEnumerable<Ast> typeAsts = ast.FindAll(testAst =>
68+
(
69+
testAst is ConvertExpressionAst convertAst &&
70+
convertAst.StaticType != null &&
71+
convertAst.StaticType.FullName == "System.Collections.ArrayList"
72+
) ||
73+
(
74+
testAst is TypeExpressionAst typeAst &&
75+
typeAst.TypeName != null &&
76+
arrayListName.IsMatch(typeAst.TypeName.Name) &&
77+
typeAst.Parent is InvokeMemberExpressionAst parentAst &&
78+
parentAst.Member != null &&
79+
parentAst.Member is StringConstantExpressionAst memberAst &&
80+
memberAst.Value.Equals("new", StringComparison.OrdinalIgnoreCase)
81+
),
82+
true
83+
);
84+
85+
foreach (Ast typeAst in typeAsts)
86+
{
87+
IScriptExtent Extent = typeAst is ConvertExpressionAst? typeAst.Extent : typeAst.Parent.Extent;
88+
yield return new DiagnosticRecord(
89+
string.Format(
90+
CultureInfo.CurrentCulture,
91+
Strings.AvoidUsingArrayListError,
92+
Extent.Text),
93+
Extent,
94+
GetName(),
95+
DiagnosticSeverity.Warning,
96+
fileName
97+
);
98+
}
99+
100+
// Find all New-Object cmdlets that create a new instance of the ArrayList class.
101+
var newObjectCommands = ast.FindAll(testAst =>
102+
testAst is CommandAst cmdAst &&
103+
cmdAst.GetCommandName() != null &&
104+
cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase),
105+
true);
106+
107+
foreach (CommandAst cmd in newObjectCommands)
108+
{
109+
// Use StaticParameterBinder to reliably get parameter values
110+
var bindingResult = StaticParameterBinder.BindCommand(cmd, true);
111+
112+
// Check for -TypeName parameter
113+
if (
114+
bindingResult.BoundParameters.TryGetValue("TypeName", out ParameterBindingResult typeNameBinding) &&
115+
typeNameBinding.ConstantValue is string typeName &&
116+
arrayListName.IsMatch(typeName)
117+
)
118+
{
119+
yield return new DiagnosticRecord(
120+
string.Format(
121+
CultureInfo.CurrentCulture,
122+
Strings.AvoidUsingArrayListError,
123+
cmd.Extent.Text),
124+
cmd.Extent,
125+
GetName(),
126+
DiagnosticSeverity.Warning,
127+
fileName
128+
);
129+
}
130+
}
131+
}
132+
133+
/// <summary>
134+
/// Retrieves the common name of this rule.
135+
/// </summary>
136+
public override string GetCommonName()
137+
{
138+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingArrayListCommonName);
139+
}
140+
141+
/// <summary>
142+
/// Retrieves the description of this rule.
143+
/// </summary>
144+
public override string GetDescription()
145+
{
146+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingArrayListDescription);
147+
}
148+
149+
/// <summary>
150+
/// Retrieves the name of this rule.
151+
/// </summary>
152+
public override string GetName()
153+
{
154+
return string.Format(
155+
CultureInfo.CurrentCulture,
156+
Strings.NameSpaceFormat,
157+
GetSourceName(),
158+
Strings.AvoidUsingArrayListName);
159+
}
160+
161+
/// <summary>
162+
/// Retrieves the severity of the rule: error, warning or information.
163+
/// </summary>
164+
public override RuleSeverity GetSeverity()
165+
{
166+
return RuleSeverity.Warning;
167+
}
168+
169+
/// <summary>
170+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
171+
/// </summary>
172+
/// <returns></returns>
173+
public DiagnosticSeverity GetDiagnosticSeverity()
174+
{
175+
return DiagnosticSeverity.Warning;
176+
}
177+
178+
/// <summary>
179+
/// Retrieves the name of the module/assembly the rule is from.
180+
/// </summary>
181+
public override string GetSourceName()
182+
{
183+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
184+
}
185+
186+
/// <summary>
187+
/// Retrieves the type of the rule, Builtin, Managed or Module.
188+
/// </summary>
189+
public override SourceType GetSourceType()
190+
{
191+
return SourceType.Builtin;
192+
}
193+
}
194+
}
195+

Rules/Strings.resx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -945,6 +945,18 @@
945945
<data name="AvoidSemicolonsAsLineTerminatorsError" xml:space="preserve">
946946
<value>Line ends with a semicolon</value>
947947
</data>
948+
<data name="AvoidUsingArrayListCommonName" xml:space="preserve">
949+
<value>Avoid using the ArrayList class</value>
950+
</data>
951+
<data name="AvoidUsingArrayListDescription" xml:space="preserve">
952+
<value>Avoid using the ArrayList class in PowerShell scripts. Consider using generic collections or fixed arrays instead.</value>
953+
</data>
954+
<data name="AvoidUsingArrayListName" xml:space="preserve">
955+
<value>AvoidUsingArrayList</value>
956+
</data>
957+
<data name="AvoidUsingArrayListError" xml:space="preserve">
958+
<value>The ArrayList class is used in '{0}'. Consider using a generic collection or a fixed array instead.</value>
959+
</data>
948960
<data name="PlaceOpenBraceName" xml:space="preserve">
949961
<value>PlaceOpenBrace</value>
950962
</data>

0 commit comments

Comments
 (0)