Skip to content

Commit 973d2e7

Browse files
committed
Better UsingStatements handling and disable AvoidUsingArrayList by default
1 parent 5a66672 commit 973d2e7

3 files changed

Lines changed: 47 additions & 48 deletions

File tree

Rules/AvoidUsingArrayList.cs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using System.Management.Automation.Language;
1111
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
1212
using System.Text.RegularExpressions;
13+
using System.Linq;
1314

1415
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
1516
{
@@ -26,7 +27,7 @@ public class AvoidUsingArrayList : ConfigurableRule
2627
/// Construct an object of AvoidUsingArrayList type.
2728
/// </summary>
2829
public AvoidUsingArrayList() {
29-
Enable = true;
30+
Enable = false;
3031
}
3132

3233
/// <summary>
@@ -37,24 +38,25 @@ public AvoidUsingArrayList() {
3738
/// <returns>A an enumerable type containing the violations</returns>
3839
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3940
{
40-
if (ast == null) { throw new ArgumentNullException(Strings.NullAstErrorMessage); }
41+
if (ast == null) { throw new ArgumentNullException(nameof(ast), Strings.NullAstErrorMessage); }
4142

4243
// If there is an using statement for the Collections namespace, check for the full typename.
4344
// Otherwise also check for the bare ArrayList name.
4445
Regex arrayListName = null;
45-
var sbAst = ast as ScriptBlockAst;
46-
foreach (UsingStatementAst usingAst in sbAst.UsingStatements)
47-
{
48-
if (
49-
usingAst.UsingStatementKind == UsingStatementKind.Namespace &&
50-
(
51-
usingAst.Name.Value.Equals("Collections", StringComparison.OrdinalIgnoreCase) ||
52-
usingAst.Name.Value.Equals("System.Collections", StringComparison.OrdinalIgnoreCase)
53-
)
54-
)
46+
if (ast is ScriptBlockAst sbAst) {
47+
foreach (UsingStatementAst usingAst in sbAst.UsingStatements.Cast<UsingStatementAst>())
5548
{
56-
arrayListName = new Regex(@"^((System\.)?Collections\.)?ArrayList$", RegexOptions.IgnoreCase);
57-
break;
49+
if (
50+
usingAst.UsingStatementKind == UsingStatementKind.Namespace &&
51+
(
52+
usingAst.Name.Value.Equals("Collections", StringComparison.OrdinalIgnoreCase) ||
53+
usingAst.Name.Value.Equals("System.Collections", StringComparison.OrdinalIgnoreCase)
54+
)
55+
)
56+
{
57+
arrayListName = new Regex(@"^((System\.)?Collections\.)?ArrayList$", RegexOptions.IgnoreCase);
58+
break;
59+
}
5860
}
5961
}
6062
if (arrayListName == null) { arrayListName = new Regex(@"^(System\.)?Collections\.ArrayList$", RegexOptions.IgnoreCase); }

Tests/Rules/AvoidUsingArrayList.tests.ps1

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,21 @@ BeforeAll {
1515

1616
Describe "AvoidArrayList" {
1717

18+
BeforeAll {
19+
$settings = @{
20+
IncludeRules = @($ruleName)
21+
Rules = @{ $ruleName = @{ Enable = $true } }
22+
}
23+
}
24+
1825
Context "When there are violations" {
1926

2027
It "Unquoted New-Object type" {
2128
$scriptDefinition = $usingCollections + {
2229
$List = New-Object ArrayList
2330
1..3 | ForEach-Object { $null = $List.Add($_) }
2431
}.ToString()
25-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
32+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
2633
$violations.Count | Should -Be 1
2734
$violations.Severity | Should -Be Warning
2835
$violations.Extent.Text | Should -Be {New-Object ArrayList}.ToString()
@@ -34,7 +41,7 @@ Describe "AvoidArrayList" {
3441
$List = New-Object 'ArrayList'
3542
1..3 | ForEach-Object { $null = $List.Add($_) }
3643
}.ToString()
37-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
44+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
3845
$violations.Count | Should -Be 1
3946
$violations.Severity | Should -Be Warning
4047
$violations.Extent.Text | Should -Be {New-Object 'ArrayList'}.ToString()
@@ -46,7 +53,7 @@ Describe "AvoidArrayList" {
4653
$List = New-Object "ArrayList"
4754
1..3 | ForEach-Object { $null = $List.Add($_) }
4855
}.ToString()
49-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
56+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
5057
$violations.Count | Should -Be 1
5158
$violations.Severity | Should -Be Warning
5259
$violations.Extent.Text | Should -Be {New-Object "ArrayList"}.ToString()
@@ -58,7 +65,7 @@ Describe "AvoidArrayList" {
5865
$List = New-Object -TypeName ArrayList
5966
1..3 | ForEach-Object { $null = $List.Add($_) }
6067
}.ToString()
61-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
68+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
6269
$violations.Count | Should -Be 1
6370
$violations.Severity | Should -Be Warning
6471
$violations.Extent.Text | Should -Be {New-Object -TypeName ArrayList}.ToString()
@@ -70,7 +77,7 @@ Describe "AvoidArrayList" {
7077
$List = New-Object -Type ArrayLIST
7178
1..3 | ForEach-Object { $null = $List.Add($_) }
7279
}.ToString()
73-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
80+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
7481
$violations.Count | Should -Be 1
7582
$violations.Severity | Should -Be Warning
7683
$violations.Extent.Text | Should -Be {New-Object -Type ArrayLIST}.ToString()
@@ -82,7 +89,7 @@ Describe "AvoidArrayList" {
8289
$List = New-Object -TypeName System.Collections.ArrayList
8390
1..3 | ForEach-Object { $null = $List.Add($_) }
8491
}.ToString()
85-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
92+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
8693
$violations.Count | Should -Be 1
8794
$violations.Severity | Should -Be Warning
8895
$violations.Extent.Text | Should -Be {New-Object -TypeName System.Collections.ArrayList}.ToString()
@@ -94,7 +101,7 @@ Describe "AvoidArrayList" {
94101
$List = New-Object COLLECTIONS.ArrayList
95102
1..3 | ForEach-Object { $null = $List.Add($_) }
96103
}.ToString()
97-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
104+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
98105
$violations.Count | Should -Be 1
99106
$violations.Severity | Should -Be Warning
100107
$violations.Extent.Text | Should -Be {New-Object COLLECTIONS.ArrayList}.ToString()
@@ -106,7 +113,7 @@ Describe "AvoidArrayList" {
106113
$List = [ArrayList](1,2,3)
107114
1..3 | ForEach-Object { $null = $List.Add($_) }
108115
}.ToString()
109-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
116+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
110117
$violations.Count | Should -Be 1
111118
$violations.Severity | Should -Be Warning
112119
$violations.Extent.Text | Should -Be {[ArrayList](1,2,3)}.ToString()
@@ -118,7 +125,7 @@ Describe "AvoidArrayList" {
118125
$List = [ArrayList]@(1,2,3)
119126
1..3 | ForEach-Object { $null = $List.Add($_) }
120127
}.ToString()
121-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
128+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
122129
$violations.Count | Should -Be 1
123130
$violations.Severity | Should -Be Warning
124131
$violations.Extent.Text | Should -Be {[ArrayList]@(1,2,3)}.ToString()
@@ -130,7 +137,7 @@ Describe "AvoidArrayList" {
130137
$List = [ArrayList]::new()
131138
1..3 | ForEach-Object { $null = $List.Add($_) }
132139
}.ToString()
133-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
140+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
134141
$violations.Count | Should -Be 1
135142
$violations.Severity | Should -Be Warning
136143
$violations.Extent.Text | Should -Be {[ArrayList]::new()}.ToString()
@@ -142,7 +149,7 @@ Describe "AvoidArrayList" {
142149
$List = [System.Collections.ArrayList]::new()
143150
1..3 | ForEach-Object { $null = $List.Add($_) }
144151
}.ToString()
145-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
152+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
146153
$violations.Count | Should -Be 1
147154
$violations.Severity | Should -Be Warning
148155
$violations.Extent.Text | Should -Be {[System.Collections.ArrayList]::new()}.ToString()
@@ -154,7 +161,7 @@ Describe "AvoidArrayList" {
154161
$List = [COLLECTIONS.ArrayList]::new()
155162
1..3 | ForEach-Object { $null = $List.Add($_) }
156163
}.ToString()
157-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
164+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
158165
$violations.Count | Should -Be 1
159166
$violations.Severity | Should -Be Warning
160167
$violations.Extent.Text | Should -Be {[COLLECTIONS.ArrayList]::new()}.ToString()
@@ -169,7 +176,7 @@ Describe "AvoidArrayList" {
169176
$List = New-Object List[Object]
170177
1..3 | ForEach-Object { $List.Add($_) }
171178
}.ToString()
172-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
179+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
173180
$violations | Should -BeNullOrEmpty
174181
}
175182

@@ -178,15 +185,15 @@ Describe "AvoidArrayList" {
178185
$List = [List[Object]]::new()
179186
1..3 | ForEach-Object { $List.Add($_) }
180187
}.ToString()
181-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
188+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
182189
$violations | Should -BeNullOrEmpty
183190
}
184191

185192
It "Using the pipeline" {
186193
$scriptDefinition = {
187194
$List = 1..3 | ForEach-Object { $_ }
188195
}.ToString()
189-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
196+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
190197
$violations | Should -BeNullOrEmpty
191198
}
192199

@@ -197,26 +204,19 @@ Describe "AvoidArrayList" {
197204
$List = [ArrayList]@(1,2,3)
198205
$List = [ArrayList]::new()
199206
}.ToString()
200-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
207+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
201208
$violations | Should -BeNullOrEmpty
202209
}
203210
}
204211

205212
Context "Disabled" {
206213

207-
BeforeAll {
208-
$settings = @{
209-
IncludeRules = @($ruleName)
210-
Rules = @{ $ruleName = @{ Enable = $false } }
211-
}
212-
}
213-
214214
It "New-Object type" {
215215
$scriptDefinition = $usingCollections + {
216216
$List = New-Object ArrayList
217217
1..3 | ForEach-Object { $null = $List.Add($_) }
218218
}.ToString()
219-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
219+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
220220
$violations | Should -BeNullOrEmpty
221221
}
222222

@@ -225,7 +225,7 @@ Describe "AvoidArrayList" {
225225
$List = [ArrayList](1,2,3)
226226
1..3 | ForEach-Object { $null = $List.Add($_) }
227227
}.ToString()
228-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
228+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
229229
$violations | Should -BeNullOrEmpty
230230
}
231231

@@ -234,17 +234,17 @@ Describe "AvoidArrayList" {
234234
$List = [ArrayList]::new()
235235
1..3 | ForEach-Object { $null = $List.Add($_) }
236236
}.ToString()
237-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
237+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName)
238238
$violations | Should -BeNullOrEmpty
239239
}
240240
}
241241

242-
Context "Explicitly enabled" {
242+
Context "Explicitly disabled" {
243243

244244
BeforeAll {
245245
$settings = @{
246246
IncludeRules = @($ruleName)
247-
Rules = @{ $ruleName = @{ Enable = $true } }
247+
Rules = @{ $ruleName = @{ Enable = $false } }
248248
}
249249
}
250250

@@ -254,10 +254,7 @@ Describe "AvoidArrayList" {
254254
1..3 | ForEach-Object { $null = $List.Add($_) }
255255
}.ToString()
256256
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
257-
$violations.Count | Should -Be 1
258-
$violations.Severity | Should -Be Warning
259-
$violations.Extent.Text | Should -Be {New-Object ArrayList}.ToString()
260-
$violations.Message | Should -Be ($ruleMessage -f {New-Object ArrayList})
257+
$violations | Should -BeNullOrEmpty
261258
}
262259
}
263260

@@ -269,7 +266,7 @@ Describe "AvoidArrayList" {
269266
New-Object -TypeName $type
270267
}.ToString()
271268

272-
$analyzer = { Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -IncludeRule @($ruleName) }
269+
$analyzer = { Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings }
273270
$analyzer | Should -Not -Throw # but won't violate either (too complex to cover)
274271
}
275272
}

docs/Rules/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ The PSScriptAnalyzer contains the following rule definitions.
2828
| [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | |
2929
| [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | |
3030
| [AvoidUsingAllowUnencryptedAuthentication](./AvoidUsingAllowUnencryptedAuthentication.md) | Warning | Yes | |
31-
| [AvoidUsingArrayList](./AvoidUsingArrayList.md) | Warning | Yes | Yes |
31+
| [AvoidUsingArrayList](./AvoidUsingArrayList.md) | Warning | No | Yes |
3232
| [AvoidUsingBrokenHashAlgorithms](./AvoidUsingBrokenHashAlgorithms.md) | Warning | Yes | |
3333
| [AvoidUsingCmdletAliases](./AvoidUsingCmdletAliases.md) | Warning | Yes | Yes<sup>2</sup> |
3434
| [AvoidUsingComputerNameHardcoded](./AvoidUsingComputerNameHardcoded.md) | Error | Yes | |

0 commit comments

Comments
 (0)