From e14adb2deb691c6b36ba6156ffae8b4e89e9e8c2 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Tue, 9 Jun 2026 19:52:28 +0200 Subject: [PATCH 1/4] Add new AvoidUsingNewObject rule to flag usage of New-Object cmdlet and suggest type initializer as a fix. --- Rules/AvoidUsingNewObject.cs | 232 +++++++++++ Rules/Strings.resx | 15 + Tests/Rules/AvoidUsingNewObject.tests.ps1 | 467 ++++++++++++++++++++++ docs/Rules/AvoidUsingNewObject.md | 78 ++++ docs/Rules/README.md | 1 + 5 files changed, 793 insertions(+) create mode 100644 Rules/AvoidUsingNewObject.cs create mode 100644 Tests/Rules/AvoidUsingNewObject.tests.ps1 create mode 100644 docs/Rules/AvoidUsingNewObject.md diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs new file mode 100644 index 000000000..2cf8d65c8 --- /dev/null +++ b/Rules/AvoidUsingNewObject.cs @@ -0,0 +1,232 @@ +// 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.Linq; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + + /// + /// Rule that reports an warning when the New-Object cmdlet is used in a script. + /// The rule implements a correction that suggests using type-casting or type constructor. + /// + /// Note: + /// In case a automatic correction isn't available, the rule won't report any violation either. + /// This is because if there isn't an automatic correction available, it means that the there + /// isn't a simple type-casting or type constructor that can be used as a replacement that + /// would be more efficient or has a better syntax than using New-Object. + /// In other words, if the `-ComObject` parameter is used, or both the parameters + /// `-ArgumentList` and `-Property` are used, there won't be a simple type initializer + /// available and the rule won't report any violation for the `New-Object` cmdlet. + /// + public class AvoidUsingNewObject : ConfigurableRule + { + + /// + /// Construct an object of AvoidUsingNewObject type. + /// + public AvoidUsingNewObject() { + Enable = false; + } + + /// + /// Analyzes the given ast to find the [violation] + /// + /// AST to be analyzed. This should be non-null + /// Name of file that corresponds to the input AST. + /// A an enumerable type containing the violations + public override IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + IEnumerable newObjectAsts = ast.FindAll(testAst => + testAst is CommandAst cmdAst && + cmdAst.GetCommandName() != null && + cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase), + true + ).Cast(); + + foreach (CommandAst cmdAst in newObjectAsts) + { + // Use StaticParameterBinder to reliably get parameter values + var bindingResult = StaticParameterBinder.BindCommand(cmdAst, true); + + // Check for `-TypeName` and either a `-ArgumentList` or `-Property`. + // But not both, as that would mean there isn't a simple + // type initializer available as a replacement. + if ( + bindingResult.BoundParameters.Count == 2 && + bindingResult.BoundParameters.TryGetValue("TypeName", out ParameterBindingResult asTypeName) && + asTypeName.ConstantValue is string typeName + ) { + Boolean isProperty = bindingResult.BoundParameters.TryGetValue("Property", out ParameterBindingResult boundResult); + if (!isProperty) + { + Boolean isArgument = bindingResult.BoundParameters.TryGetValue("ArgumentList", out ParameterBindingResult argumentResult); + if (isArgument) { boundResult = argumentResult; } else { continue; } + } + + string correction = null; + if (isProperty) + { + correction = "[" + typeName + "]" + boundResult.Value.Extent.Text; + } + else if (boundResult.ConstantValue != null) + { + string valueText = boundResult.Value.Extent.Text; + if ( + boundResult.Value is StringConstantExpressionAst stringConstant && + stringConstant.StringConstantType == StringConstantType.BareWord + ) + { + valueText = '"' + valueText.Replace("\"", string.Empty) + '"'; // Test""123 --> "Test123" + } + correction = "[" + typeName + "]" + valueText; + } + else if ( + boundResult.Value is ParenExpressionAst parenExpressionAst && + !parenExpressionAst.Pipeline.Extent.Text.StartsWith(",") + ) + { + correction = "[" +typeName + "]::new" + parenExpressionAst.Extent.Text; + } + else if ( + boundResult.Value is ArrayExpressionAst arrayExpressionAst && + !arrayExpressionAst.SubExpression.Extent.Text.StartsWith(",") + ) + { + correction = "[" + typeName + "]::new(" + arrayExpressionAst.SubExpression.Extent.Text + ")"; + } + else if ( + boundResult.Value is SubExpressionAst subExpressionAst && + !subExpressionAst.SubExpression.Extent.Text.StartsWith(",") + ) + { + correction = "[" + typeName + "]::new(" + subExpressionAst.SubExpression.Extent.Text + ")"; + } + else if (boundResult.Value is VariableExpressionAst) + { + // correction == $null + + // The correction is inconclusive, as we can't be sure if the variable contains + // a single value that can be used in a type initializer, + // or if it contains multiple values that would require splatting, + // which can't be automatically determined from the AST. + } + else + { + correction = "[" + typeName + "]::new(" + boundResult.Value.Extent.Text + ")"; + } + + DiagnosticRecord diagnosticRecord = new DiagnosticRecord( + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidUsingNewObjectError, + typeName + ), + cmdAst.Extent, + GetName(), + DiagnosticSeverity.Warning, + fileName, + typeName + ); + + if (correction != null) + { + diagnosticRecord.SuggestedCorrections = new List { + new CorrectionExtent( + cmdAst.Extent.StartLineNumber, + cmdAst.Extent.EndLineNumber, + cmdAst.Extent.StartColumnNumber, + cmdAst.Extent.EndColumnNumber, + correction, + fileName, + string.Format( + CultureInfo.CurrentCulture, + Strings.AvoidUsingNewObjectCorrectionDescription, + typeName + ) + ) + }; + } + + yield return diagnosticRecord; + } + } + } + + /// + /// Retrieves the common name of this rule. + /// + public override string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); + } + + /// + /// Retrieves the description of this rule. + /// + public override string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); + } + + /// + /// Retrieves the name of this rule. + /// + public override string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidUsingNewObjectName); + } + + /// + /// Retrieves the severity of the rule: error, warning or information. + /// + public override RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the severity of the returned diagnostic record: error, warning, or information. + /// + /// + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Retrieves the name of the module/assembly the rule is from. + /// + public override string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Retrieves the type of the rule, Builtin, Managed or Module. + /// + public override SourceType GetSourceType() + { + return SourceType.Builtin; + } + } +} + diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 2a04fd759..fad34a43a 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -336,6 +336,21 @@ File '{0}' uses Console.'{1}'. Using Console to write is not recommended because it may not work in all hosts or there may even be no hosts at all. Use Write-Output instead. + + Avoid Using New-Object + + + Avoid using the 'New-Object' cmdlet to create objects as it might perform poorly. + + + AvoidUsingNewObject + + + Avoid using the 'New-Object' cmdlet to create objects of type '{0}' as it might perform poorly. + + + Use the type initializer '[{0}]' to construct or cast the intended object. + Avoid using the Write-Host cmdlet. Instead, use Write-Output, Write-Verbose, or Write-Information. Because Write-Host is host-specific, its implementation might vary unpredictably. Also, prior to PowerShell 5.0, Write-Host did not write to a stream, so users cannot suppress it, capture its value, or redirect it. diff --git a/Tests/Rules/AvoidUsingNewObject.tests.ps1 b/Tests/Rules/AvoidUsingNewObject.tests.ps1 new file mode 100644 index 000000000..22658441c --- /dev/null +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -0,0 +1,467 @@ +# 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')] +[Diagnostics.CodeAnalysis.SuppressMessage('PSUseApprovedVerbs', '', Justification = 'Required for custom test')] +param() + +BeforeDiscovery { + function Should-Parse([string] $ActualValue, [switch] $Negate, [string] $Because) { + $errors = $Null + $null = [Parser]::ParseInput($ActualValue, [ref]$null, [ref]$errors) + $succeeded = -not $errors -xor $Negate + if ($succeeded) { + $not = if ($Negate) { ' not' } + $failureMessage = "Expected '$ActualValue' to$Not parse$(if($Because) { " because $Because"})." + } + + return [PSCustomObject]@{ + Succeeded = $succeeded + FailureMessage = $failureMessage + } + } + + $ShouldParseSplat = @{ + Name = 'Parse' + InternalName = 'Should-Parse' + Test = ${function:Should-Parse} + SupportsArrayInput = $true + } + Add-ShouldOperator @ShouldParseSplat +} + +BeforeAll { + $ruleName = "PSAvoidUsingNewObject" + $ruleMessage = "Avoid using the 'New-Object' cmdlet to create objects of type '{0}' as it might perform poorly." + $correctionDescription = "Use the type initializer '[{0}]' to construct or cast the intended object." +} + +Describe "AvoidUsingNewObject" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $true } } + } + } + + Context "Examples" { + + It 'Version "1.2.3"' { + $scriptDefinition = { $Version = New-Object -TypeName Version -ArgumentList "1.2.3" }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object -TypeName Version -ArgumentList "1.2.3"' + $violations.Message | Should -Be ($ruleMessage -f 'Version') + $violations.RuleSuppressionID | Should -Be 'Version' + $violations.SuggestedCorrections.Text | Should -Be '[Version]"1.2.3"' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'Version') + } + + It 'PSCustomObject Property' { + $scriptDefinition = { + for ($i = 0; $i -lt 100000; $i++) { + $resultObject = New-Object PSCustomObject -Property @{ + Name = "Name$i" + Value = $i + } + } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -BeLike 'New-Object PSCustomObject -Property @{*}' + $violations.Message | Should -Be ($ruleMessage -f 'PSCustomObject') + $violations.RuleSuppressionID | Should -Be 'PSCustomObject' + $violations.SuggestedCorrections.Text | Should -BeLike '`[PSCustomObject`]@{*}' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'PSCustomObject') + } + + It 'HashSet InvariantCultureIgnoreCase' { + $scriptDefinition = { + $hashSet = New-Object ` + -TypeName 'System.Collections.Generic.HashSet[String]' ` + -ArgumentList ([StringComparer]::InvariantCultureIgnoreCase) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Not -BeNullOrEmpty + $violations.Message | Should -Be ($ruleMessage -f 'System.Collections.Generic.HashSet[String]') + $violations.RuleSuppressionID | Should -Be 'System.Collections.Generic.HashSet[String]' + $violations.SuggestedCorrections.Text | Should -Be '[System.Collections.Generic.HashSet[String]]::new([StringComparer]::InvariantCultureIgnoreCase)' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'System.Collections.Generic.HashSet[String]') + } + } + + Context "Cast" { + + It "String 'Hello World'" { + $scriptDefinition = { $String = New-Object String 'Hello World' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be "New-Object String 'Hello World'" + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections.Text | Should -Be "[String]'Hello World'" + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String') + } + + It "String ArgumentList 'Hello World'" { + $scriptDefinition = { $String = New-Object -TypeName String -ArgumentList 'Hello World' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be "New-Object -TypeName String -ArgumentList 'Hello World'" + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections.Text | Should -Be "[String]'Hello World'" + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String') + } + + It 'String ArgumentList Test' { + $scriptDefinition = { $String = New-Object -TypeName String -ArgumentList Test }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object -TypeName String -ArgumentList Test' + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections.Text | Should -Be '[String]"Test"' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String') + } + } + + Context 'Construct' { + + It 'String ArgumentList ($Test)' { + $scriptDefinition = { $String = New-Object -TypeName String -ArgumentList ($Test) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object -TypeName String -ArgumentList ($Test)' + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections.Text | Should -Be '[String]::new($Test)' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String') + } + + It 'int[] (1..3)' { + $scriptDefinition = { $Integers = New-Object int[] (1..3) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object int[] (1..3)' + $violations.Message | Should -Be ($ruleMessage -f 'int[]') + $violations.RuleSuppressionID | Should -Be 'int[]' + $violations.SuggestedCorrections.Text | Should -Be '[int[]]::new(1..3)' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'int[]') + } + + It 'HashSet[String] ($strings, $ignoreCase)' { + $scriptDefinition = { + $strings = [string[]]('a', 'A') + $ignoreCase = [StringComparer]::InvariantCultureIgnoreCase + $hashSet = New-Object "System.Collections.Generic.HashSet[String]" ($strings, $ignoreCase) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object "System.Collections.Generic.HashSet[String]" ($strings, $ignoreCase)' + $violations.Message | Should -Be ($ruleMessage -f 'System.Collections.Generic.HashSet[String]') + $violations.RuleSuppressionID | Should -Be 'System.Collections.Generic.HashSet[String]' + $violations.SuggestedCorrections.Text | Should -Be '[System.Collections.Generic.HashSet[String]]::new($strings, $ignoreCase)' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'System.Collections.Generic.HashSet[String]') + } + } + + Context "Permit" { + + It 'ComObject' { + $scriptDefinition = { + $IE1 = New-Object -ComObject InternetExplorer.Application -Property @{ Navigate2="www.microsoft.com"; Visible = $true } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'Write-Host' { + $scriptDefinition = { + Write-Host New-Object String 123 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'Splat' { + $scriptDefinition = { + for ($i = 0; $i -lt 100000; $i++) { + $Splat = @{ + TypeName = 'PSCustomObject' + Property = @{ + Name = "Name$i" + Value = $i + } + } + $resultObject = New-Object @Splat + } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'Property and ArgumentList' { + $scriptDefinition = { + Class MyClass { + $Foo + $Bar + MyClass($Bar) { $this.Bar = $Bar } + } + $MyObject = New-Object -TypeName MyClass -Property @{ Foo = 1 } -ArgumentList 2 + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context 'Specials' { + + It 'Variable property' { + $scriptDefinition = { New-Object MyClass -Property $properties }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]$properties' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Integer argument' { + $scriptDefinition = { New-Object MyClass 1 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]1' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Bare string argument' { + $scriptDefinition = { New-Object MyClass Test }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]"Test"' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Single quoted string argument' { + $scriptDefinition = { New-Object MyClass 'Test' }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be "[MyClass]'Test'" + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Double quoted string argument' { + $scriptDefinition = { New-Object MyClass "Test" }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]"Test"' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Bare string argument with embedded double quotes' { + $scriptDefinition = { New-Object MyClass Test""123 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]"Test123"' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument list' { + $scriptDefinition = { New-Object MyClass 1, 2 }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new(1, 2)' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument group' { + $scriptDefinition = { New-Object MyClass (1, 2) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new(1, 2)' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument array' { + $scriptDefinition = { New-Object MyClass @(1, 2) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new(1, 2)' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument subexpression' { + $scriptDefinition = { New-Object MyClass $(1,2) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new(1,2)' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument group with single item' { + $scriptDefinition = { New-Object MyClass (,1) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new((,1))' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument array with single item' { + $scriptDefinition = { New-Object MyClass @(,1) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new(@(,1))' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument subexpression with single item' { + $scriptDefinition = { New-Object MyClass $(,1) }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.SuggestedCorrections.Text | Should -Be '[MyClass]::new($(,1))' + $violations.SuggestedCorrections.Text | Should -Parse + } + + It 'Argument variable' { + # The SuggestedCorrections is inconclusive, as we can't be sure if the variable contains + # a single value that can be used in a type initializer, + # or if it contains multiple values that would require splatting, + # which can't be automatically determined from the AST. + + $scriptDefinition = { New-Object String $Test }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object String $Test' + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections | Should -BeNullOrEmpty + } + + } + + Context "Disable" { + + BeforeAll { + $Settings = @{ + IncludeRules = @($ruleName) + Rules = @{ $ruleName = @{ Enable = $false } } + } + } + + It 'Version "1.2.3"' { + $scriptDefinition = { $Version = New-Object -TypeName Version -ArgumentList "1.2.3" }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'PSCustomObject Property' { + $scriptDefinition = { + for ($i = 0; $i -lt 100000; $i++) { + $resultObject = New-Object PSCustomObject -Property @{ + Name = "Name$i" + Value = $i + } + } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'HashSet InvariantCultureIgnoreCase' { + $scriptDefinition = { + $hashSet = New-Object ` + -TypeName 'System.Collections.Generic.HashSet[String]' ` + -ArgumentList ([StringComparer]::InvariantCultureIgnoreCase) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Suppress" { + + It 'Version "1.2.3"' { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingNewObject', '', Justification = 'Test')] # All ids + param() + $Version = New-Object -TypeName Version -ArgumentList "1.2.3" + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'PSCustomObject Property' { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingNewObject', 'PSCustomObject', Justification = 'Test')] + param() + for ($i = 0; $i -lt 100000; $i++) { + $resultObject = New-Object PSCustomObject -Property @{ + Name = "Name$i" + Value = $i + } + } + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + + It 'HashSet InvariantCultureIgnoreCase' { + $scriptDefinition = { + [Diagnostics.CodeAnalysis.SuppressMessage('PSAvoidUsingNewObject', 'System.Collections.Generic.HashSet[String]', Justification = 'Test')] + param() + $hashSet = New-Object ` + -TypeName 'System.Collections.Generic.HashSet[String]' ` + -ArgumentList ([StringComparer]::InvariantCultureIgnoreCase) + }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations | Should -BeNullOrEmpty + } + } + + Context "Fix" { + + It "MyClass Properties" { + $tempFile = Join-Path $TestDrive 'TestScript1.ps1' + Set-Content -LiteralPath $tempFile -Value {New-Object MyClass -Property $properties} -NoNewLine + $violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix + Get-Content -LiteralPath $tempFile -Raw | Should -Be {[MyClass]$properties}.ToString() + } + + It 'PSCustomObject Property' { + $tempFile = Join-Path $TestDrive 'TestScript2.ps1' + Set-Content -LiteralPath $tempFile -Value { + for ($i = 0; $i -lt 100000; $i++) { + $resultObject = New-Object PSCustomObject -Property @{ + Name = "Name$i" + Value = $i + } + } + } + $violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix + $Content = Get-Content -LiteralPath $tempFile -Raw + $Content | Should -BeLike '*[PSCustomObject]*' + $Content | Should -Parse + } + + It 'HashSet InvariantCultureIgnoreCase' { + $tempFile = Join-Path $TestDrive 'TestScript3.ps1' + Set-Content -LiteralPath $tempFile -Value { + $hashSet = New-Object ` + -TypeName 'System.Collections.Generic.HashSet[String]' ` + -ArgumentList ([StringComparer]::InvariantCultureIgnoreCase) + } + $violations = Invoke-ScriptAnalyzer -Path $tempFile -Settings $Settings -fix + $Content = (Get-Content -LiteralPath $tempFile -Raw).Trim() + $Content | Should -Be '$hashSet = [System.Collections.Generic.HashSet[String]]::new([StringComparer]::InvariantCultureIgnoreCase)' + } + } +} \ No newline at end of file diff --git a/docs/Rules/AvoidUsingNewObject.md b/docs/Rules/AvoidUsingNewObject.md new file mode 100644 index 000000000..c069ea5ab --- /dev/null +++ b/docs/Rules/AvoidUsingNewObject.md @@ -0,0 +1,78 @@ +--- +description: Avoid using the `New-Object` cmdlet +ms.date: 06/06/2026 +ms.topic: reference +title: AvoidUsingNewObject +--- + +# AvoidUsingNewObject + + + +**Severity Level: Warning** + + +## Description + +Avoid using the `New-Object` cmdlet to create objects as it might perform poorly. +Instead, use type initializer to construct or cast the intended object. + +## Example + +### Wrong + +```powershell +# Create a version object using New-Object +$Version = New-Object -TypeName Version -ArgumentList "1.2.3" +``` + +```powershell +# Create a custom object using New-Object +for ($i = 0; $i -lt 100000; $i++) { + $resultObject = New-Object PSCustomObject -Property @{ + Name = "Name$i" + Value = $i + } +} +``` + +```powershell +$hashSet = New-Object -TypeName 'System.Collections.Generic.HashSet[String]' -ArgumentList ([StringComparer]::InvariantCultureIgnoreCase) +``` + +### Correct + +```powershell +# Create a version object using the type constructor +$Version = [Version]"1.2.3" +``` + +```powershell +# Create a custom object using a hashtable and type-casting +for ($i = 0; $i -lt 100000; $i++) { + $resultObject = [PSCustomObject]@{ + Name = "Name$i" + Value = $i + } +} +``` + +```powershell +$hashSet = [System.Collections.Generic.HashSet[String]]([StringComparer]::InvariantCultureIgnoreCase) +``` + +## Configuration + +```powershell +Rules = @{ + PSAvoidNewObject = @{ + Enable = $true + } +} +``` + +### Parameters + +- `Enable`: **bool** (Default value is `$false`) + + Enable or disable the rule during ScriptAnalyzer invocation. diff --git a/docs/Rules/README.md b/docs/Rules/README.md index fca031e33..b4a14213c 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -36,6 +36,7 @@ The PSScriptAnalyzer contains the following rule definitions. | [AvoidUsingDoubleQuotesForConstantString](./AvoidUsingDoubleQuotesForConstantString.md) | Information | No | | | [AvoidUsingEmptyCatchBlock](./AvoidUsingEmptyCatchBlock.md) | Warning | Yes | | | [AvoidUsingInvokeExpression](./AvoidUsingInvokeExpression.md) | Warning | Yes | | +| [AvoidUsingNewObject](./AvoidUsingNewObject.md) | Warning | No | Yes | | [AvoidUsingPlainTextForPassword](./AvoidUsingPlainTextForPassword.md) | Warning | Yes | | | [AvoidUsingPositionalParameters](./AvoidUsingPositionalParameters.md) | Warning | Yes | | | [AvoidUsingUsernameAndPasswordParams](./AvoidUsingUsernameAndPasswordParams.md) | Error | Yes | | From 0aad726963f6b89c11943a4a44c28c63af979e33 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 10 Jun 2026 10:06:19 +0200 Subject: [PATCH 2/4] Implemented feedback from CoPilot --- Rules/AvoidUsingNewObject.cs | 20 +++++++++++++++----- Tests/Rules/AvoidUsingNewObject.tests.ps1 | 4 ++-- docs/Rules/AvoidUsingNewObject.md | 4 ++-- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs index 2cf8d65c8..3e52bc5ae 100644 --- a/Rules/AvoidUsingNewObject.cs +++ b/Rules/AvoidUsingNewObject.cs @@ -23,13 +23,23 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules /// The rule implements a correction that suggests using type-casting or type constructor. /// /// Note: - /// In case a automatic correction isn't available, the rule won't report any violation either. - /// This is because if there isn't an automatic correction available, it means that the there - /// isn't a simple type-casting or type constructor that can be used as a replacement that - /// would be more efficient or has a better syntax than using New-Object. - /// In other words, if the `-ComObject` parameter is used, or both the parameters + /// In most cases if there isn't an automatic correction isn't available, + /// the rule won't report any violation either. + /// This is because if there isn't an automatic correction available, it generally means + /// that there isn't a simple type-casting or type constructor that can be used that would + /// be more efficient or has a better syntax than using New-Object. + /// In other words, if the common `-Verbose` parameter is used, or both the parameters /// `-ArgumentList` and `-Property` are used, there won't be a simple type initializer /// available and the rule won't report any violation for the `New-Object` cmdlet. + /// + /// Nevertheless, there are still some cases where the `New-Object` cmdlet might be + /// replaceable with a type initializer that would be more efficient or has a better syntax, + /// but an automatic correction can't be provided. + /// For example if the `-ArgumentList` parameter is used with a variable, + /// the rule will report a violation, but won't be able to provide a correction, + /// as it's not possible to determine from the AST alone whether the variable contains a + /// single value that can be used in a type initializer, + /// or if it contains multiple values that would require splatting. /// public class AvoidUsingNewObject : ConfigurableRule { diff --git a/Tests/Rules/AvoidUsingNewObject.tests.ps1 b/Tests/Rules/AvoidUsingNewObject.tests.ps1 index 22658441c..cc06001ea 100644 --- a/Tests/Rules/AvoidUsingNewObject.tests.ps1 +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -12,9 +12,9 @@ BeforeDiscovery { $errors = $Null $null = [Parser]::ParseInput($ActualValue, [ref]$null, [ref]$errors) $succeeded = -not $errors -xor $Negate - if ($succeeded) { + if (-not $succeeded) { $not = if ($Negate) { ' not' } - $failureMessage = "Expected '$ActualValue' to$Not parse$(if($Because) { " because $Because"})." + $failureMessage = "Expected '$ActualValue'$Not to parse$(if($Because) { " because $Because"})." } return [PSCustomObject]@{ diff --git a/docs/Rules/AvoidUsingNewObject.md b/docs/Rules/AvoidUsingNewObject.md index c069ea5ab..61ad64da7 100644 --- a/docs/Rules/AvoidUsingNewObject.md +++ b/docs/Rules/AvoidUsingNewObject.md @@ -58,14 +58,14 @@ for ($i = 0; $i -lt 100000; $i++) { ``` ```powershell -$hashSet = [System.Collections.Generic.HashSet[String]]([StringComparer]::InvariantCultureIgnoreCase) +$hashSet = [System.Collections.Generic.HashSet[String]]::new([StringComparer]::InvariantCultureIgnoreCase) ``` ## Configuration ```powershell Rules = @{ - PSAvoidNewObject = @{ + PSAvoidUsingNewObject = @{ Enable = $true } } From d9e0e6b59df40e33963fd9a4829b9ace7220de9b Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 10 Jun 2026 10:45:38 +0200 Subject: [PATCH 3/4] Implemented more CoPilot suggestions --- Rules/AvoidUsingNewObject.cs | 19 ++++++++++++------- Tests/Rules/AvoidUsingNewObject.tests.ps1 | 17 +++++++++++++++-- docs/Rules/AvoidUsingNewObject.md | 2 +- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs index 3e52bc5ae..f9ed8596d 100644 --- a/Rules/AvoidUsingNewObject.cs +++ b/Rules/AvoidUsingNewObject.cs @@ -19,11 +19,11 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif /// - /// Rule that reports an warning when the New-Object cmdlet is used in a script. + /// Rule that reports a warning when the New-Object cmdlet is used in a script. /// The rule implements a correction that suggests using type-casting or type constructor. /// /// Note: - /// In most cases if there isn't an automatic correction isn't available, + /// In most cases if there isn't an automatic correction available, /// the rule won't report any violation either. /// This is because if there isn't an automatic correction available, it generally means /// that there isn't a simple type-casting or type constructor that can be used that would @@ -63,8 +63,8 @@ public override IEnumerable AnalyzeScript(Ast ast, string file IEnumerable newObjectAsts = ast.FindAll(testAst => testAst is CommandAst cmdAst && - cmdAst.GetCommandName() != null && - cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase), + (cmdAst.GetCommandName() as string) is string commandName && + commandName.Equals("New-Object", StringComparison.OrdinalIgnoreCase), true ).Cast(); @@ -77,7 +77,7 @@ testAst is CommandAst cmdAst && // But not both, as that would mean there isn't a simple // type initializer available as a replacement. if ( - bindingResult.BoundParameters.Count == 2 && + bindingResult.BoundParameters.Count <= 2 && bindingResult.BoundParameters.TryGetValue("TypeName", out ParameterBindingResult asTypeName) && asTypeName.ConstantValue is string typeName ) { @@ -85,11 +85,16 @@ asTypeName.ConstantValue is string typeName if (!isProperty) { Boolean isArgument = bindingResult.BoundParameters.TryGetValue("ArgumentList", out ParameterBindingResult argumentResult); - if (isArgument) { boundResult = argumentResult; } else { continue; } + if (isArgument) { boundResult = argumentResult; } } string correction = null; - if (isProperty) + if (boundResult == null) + { + // No `-Property` or `-ArgumentList` parameter was used, so we suggest a parameterless constructor call. + correction = "[" + typeName + "]::new()"; + } + else if (isProperty) { correction = "[" + typeName + "]" + boundResult.Value.Extent.Text; } diff --git a/Tests/Rules/AvoidUsingNewObject.tests.ps1 b/Tests/Rules/AvoidUsingNewObject.tests.ps1 index cc06001ea..d50cddb30 100644 --- a/Tests/Rules/AvoidUsingNewObject.tests.ps1 +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -144,6 +144,19 @@ Describe "AvoidUsingNewObject" { Context 'Construct' { + It 'Empty string' { + $scriptDefinition = { $String = New-Object String }.ToString() + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations.Count | Should -Be 1 + $violations.Severity | Should -Be Warning + $violations.Extent.Text | Should -Be 'New-Object String' + $violations.Message | Should -Be ($ruleMessage -f 'String') + $violations.RuleSuppressionID | Should -Be 'String' + $violations.SuggestedCorrections.Text | Should -Be '[String]::new()' + $violations.SuggestedCorrections.Text | Should -Parse + $violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String') + } + It 'String ArgumentList ($Test)' { $scriptDefinition = { $String = New-Object -TypeName String -ArgumentList ($Test) }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings @@ -202,7 +215,7 @@ Describe "AvoidUsingNewObject" { $scriptDefinition = { Write-Host New-Object String 123 }.ToString() - $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings + $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations | Should -BeNullOrEmpty } @@ -267,7 +280,7 @@ Describe "AvoidUsingNewObject" { $violations.SuggestedCorrections.Text | Should -Parse } - It 'Double quoted string argument' { + It 'Double quoted string argument' { $scriptDefinition = { New-Object MyClass "Test" }.ToString() $violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings $violations.SuggestedCorrections.Text | Should -Be '[MyClass]"Test"' diff --git a/docs/Rules/AvoidUsingNewObject.md b/docs/Rules/AvoidUsingNewObject.md index 61ad64da7..3402c33c3 100644 --- a/docs/Rules/AvoidUsingNewObject.md +++ b/docs/Rules/AvoidUsingNewObject.md @@ -15,7 +15,7 @@ title: AvoidUsingNewObject ## Description Avoid using the `New-Object` cmdlet to create objects as it might perform poorly. -Instead, use type initializer to construct or cast the intended object. +Instead, use a type initializer to construct or cast the intended object. ## Example From e061335231b26f5f55d7dfcdf470a70a535e7743 Mon Sep 17 00:00:00 2001 From: iRon7 Date: Wed, 10 Jun 2026 11:10:54 +0200 Subject: [PATCH 4/4] Resolved minor the grammar --- Rules/AvoidUsingNewObject.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs index f9ed8596d..e57032565 100644 --- a/Rules/AvoidUsingNewObject.cs +++ b/Rules/AvoidUsingNewObject.cs @@ -56,7 +56,7 @@ public AvoidUsingNewObject() { /// /// AST to be analyzed. This should be non-null /// Name of file that corresponds to the input AST. - /// A an enumerable type containing the violations + /// An enumerable type containing the violations public override IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);