Skip to content

Add new AvoidUsingArrayList rule#2174

Open
iRon7 wants to merge 16 commits intoPowerShell:mainfrom
iRon7:#2147AvoidArrayList
Open

Add new AvoidUsingArrayList rule#2174
iRon7 wants to merge 16 commits intoPowerShell:mainfrom
iRon7:#2147AvoidArrayList

Conversation

@iRon7
Copy link
Copy Markdown

@iRon7 iRon7 commented Apr 15, 2026

to warn when the ArrayList class is used in PowerShell scripts (#2147).
Add tests for both violations and non-violations of this rule.
Update documentation to include the new rule and its guidelines.
(For this, I followed Liam Peters' blog post, also note that this is my first formal C# contribution to any GitHub project)

PR Summary

PR Checklist

…lass 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.
Copy link
Copy Markdown
Contributor

@liamjpeters liamjpeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👋 @iRon7 - great that you've taken the plunge, and thanks for being one of 3 people to read my blog post (myself included) 😊.

I hope you don't mind, but I took a look over the PR and left some suggestions. I'm happy to discuss or elaborate on anything.

Thanks,

Comment thread .vscode/settings.json Outdated
Comment thread docs/Rules/AvoidUsingArrayList.md Outdated
Comment thread docs/Rules/AvoidUsingArrayList.md Outdated
Comment thread docs/Rules/AvoidUsingArrayList.md Outdated
Comment thread docs/Rules/README.md
| [AvoidShouldContinueWithoutForce](./AvoidShouldContinueWithoutForce.md) | Warning | Yes | |
| [AvoidTrailingWhitespace](./AvoidTrailingWhitespace.md) | Warning | Yes | |
| [AvoidUsingAllowUnencryptedAuthentication](./AvoidUsingAllowUnencryptedAuthentication.md) | Warning | Yes | |
| [AvoidUsingArrayList](./AvoidUsingArrayList.md) | Warning | Yes | |
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that this rule should be enabled by default

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that in a lot of cases the use of the ArrayList class might cause a PowerShell pitfall (due to unintentionally polluting the PowerShell pipeline with the Add method), I would really like to see it enabled by default.
Anyways, I leave it to your team to make the final decision on this.

Comment thread docs/Rules/AvoidUsingArrayList.md Outdated
Comment thread Rules/AvoidUsingArrayList.cs Outdated
Comment thread Tests/Rules/AvoidUsingArrayList.tests.ps1 Outdated
Comment thread Tests/Rules/AvoidUsingArrayList.tests.ps1 Outdated
Comment thread Rules/AvoidUsingArrayList.cs Outdated
@iRon7
Copy link
Copy Markdown
Author

iRon7 commented Apr 16, 2026

Liam,
Thanks you for your help and feedback, I learned a lot!

One general thought that came up:
Instead of testing all things afterwards (and still making a lot of copy-pasta misstakes, as I did), a small cmdlet like:

New-CSRule -Name <name> -Description <Description> -Message <Message> -Configurable ...

That prepares things as a stripped .cs rule and .ps1 test (without logic) along with the Strings.resx and the readme updates etc. could in my opinion be helpful and prevent a lot of issues at forehand.

@liamjpeters
Copy link
Copy Markdown
Contributor

Liam, Thanks you for your help and feedback, I learned a lot!

One general thought that came up: Instead of testing all things afterwards (and still making a lot of copy-pasta misstakes, as I did), a small cmdlet like:

New-CSRule -Name <name> -Description <Description> -Message <Message> -Configurable ...

That prepares things as a stripped .cs rule and .ps1 test (without logic) along with the Strings.resx and the readme updates etc. could in my opinion be helpful and prevent a lot of issues at forehand.

There's the RuleMaker module. I've tried it before with mixed results. It needs a little TLC (copyright is outdated, doesn't append to string resources super well etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants