Add PSAvoidUsingNewObject Rule

[DrSkillIssue]

PR Summary

#2046

Adds a new built-in rule PSAvoidUsingNewObject that flags usage of the New-Object cmdlet and recommends using type literals with ::new() syntax instead for better performance and more idiomatic PowerShell code.

What Changed

• New Rule: AvoidUsingNewObject
• Severity: Warning
• Scope: Flags New-Object -TypeName usage while preserving New-Object -ComObject (COM objects cannot use type literals)

Key Features

• Smart COM Object Detection: Excludes New-Object -ComObject calls since they cannot be replaced with type literals
• Parameter Splatting Support: Handles complex scenarios where parameters are passed via hashtable splatting (New-Object @params)
• Variable Chain Resolution: Recursively follows variable assignments to determine if splatted parameters contain COM object references
• Comprehensive AST Analysis: Supports both direct parameter usage and various splatting patterns including $using: variables

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

[DrSkillIssue]

@microsoft-github-policy-service agree

[liamjpeters]

👋 Hey @DrSkillIssue, thanks for putting this together; you've clearly put a lot of thought into the many ways you could try and sneak a -ComObject in 😎.

Some things to look into:

• When checking for splatting use of ComObject in a hashtable, you dismiss keys that are less than 3 characters. You do this to avoid false positives, but I'm not sure what would lead to a false positive?

  https://github.com/DrSkillIssue/PSScriptAnalyzer/blob/26c779cdf5c42da4ac5e5edc4a09e7954d8f85ad/Rules/AvoidUsingNewObject.cs#L423-L425

  Skipping 1 and 2 character keys does miss the below, which are both valid. New-Object only has 1 parameter starting with a C, so C and Co are both valid (if not ideal) ways to supply the ComObject parameter.

  $Splat = @{
      'C' = 'Word.Application'
  }
  New-Object @Splat
  
  $Splat2 = @{
      'Co' = 'Word.Application'
  }
  New-Object @Splat2

• When a switch parameter (such as -Strict or -Verbose) precedes -ComObject in the command expression, a violation is still emitted.

  New-Object -Verbose -ComObject 'Word.Application'

  This is probably coming from the HasComObjectParameter function.

  https://github.com/DrSkillIssue/PSScriptAnalyzer/blob/26c779cdf5c42da4ac5e5edc4a09e7954d8f85ad/Rules/AvoidUsingNewObject.cs#L134-L138

  It's looping through the CommandElements of the CommandAst. Each time it encounters a CommandParameterAst it sets waitingForParamValue to true, which skips the next element (assuming it's a parameter value).

  I like to use Jason Shirk's Show-AST to visualise this stuff. CommandParameterAst can be sequential, without a variable expression or constant expression between them.

  

• You're getting 2 failing tests (you can run tests locally using .\build.ps1 -Test):

  1. You need to add an entry for your rule into the table in docs/Rules/README.md. The format should be obvious from all the other rules.
  2. You need to increment the rule count of the default rules. With your new rule, there'd be 71.
     

     PSScriptAnalyzer/Tests/Engine/GetScriptAnalyzerRule.tests.ps1

     Lines 64 to 76 in ea70855

     	It "get Rules with no parameters supplied" {
     	$defaultRules = Get-ScriptAnalyzerRule
     	$expectedNumRules = 70
     	if ($PSVersionTable.PSVersion.Major -le 4)
     	{
     	# for PSv3 PSAvoidGlobalAliases is not shipped because
     	# it uses StaticParameterBinder.BindCommand which is
     	# available only on PSv4 and above
     	$expectedNumRules--
     	}
     	$defaultRules.Count | Should -Be $expectedNumRules
     	}

• Regarding tests: it's great that you've got so many test cases documented for passing and failing scenarios. It would be so much better if these were all individual tests.

  Currently there are only 3 tests. One for the error message description, one for there being 0 violations in one file, another for there being 20 violations in the other file.

  Small focused tests help pinpoint any future regressions and ensure coverage across edge-cases (rather than a number of violations going from 20 to 19 - it would show which test case has changed).

• You've changed some formatting in Engine/Commands/InvokeScriptAnalyzerCommand.cs. This formatting change is the only change you're making in that file and it's not related to your new rule or this PR.

I really liked your rule documentation, lots of detail and further reading links. ⭐

[bergmeister]

@DrSkillIssue There are 2 test failures, can you please look into fixing them?

[bergmeister]

Once my comments are being addressed, I'd be happy to approve, the most important one is that I think rule should not be enabled by default to start with

[bergmeister]

please tidy up diff

[bergmeister]

there are a few areas like this one where indentation could be reduced. Here we could e.g. negate statement and continue with next loop member so that other code can just be below at same indentation level: if (assignment.Right is not CommandExpressionAst cmdExpr) { continue; }

[bergmeister]

this file is to replicate setting used by PSGallery to scan code when users upload new module and send email if there are violations. I don't even know whether this file is still in sync as of today but this file should only be updated by PSGallery members who know the rules used by PSGallery (and consider adding new ones).
Can you comment on whether this file is up to date and if not open separate PR to sync please @alerickson ?

[bergmeister]

I am not sure what the purpose of this settings file is but until then I'd not recommend touching it please

[bergmeister]

With this inheritance rule will run by default but I fear it will cause a big backlash as many people run PSSA in CI. I suggest to inherit from ConfigurableRule so that it's not enabled by default. See this PR for an example of how to: #2124