diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs new file mode 100644 index 000000000..e57032565 --- /dev/null +++ b/Rules/AvoidUsingNewObject.cs @@ -0,0 +1,247 @@ +// 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 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 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 + { + + /// + /// 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. + /// 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() as string) is string commandName && + commandName.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; } + } + + string correction = null; + 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; + } + 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 2f2b1e1a9..739efd72c 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -348,6 +348,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..d50cddb30 --- /dev/null +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -0,0 +1,480 @@ +# 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 (-not $succeeded) { + $not = if ($Negate) { ' not' } + $failureMessage = "Expected '$ActualValue'$Not to 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 '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 + $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..3402c33c3 --- /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 a 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]]::new([StringComparer]::InvariantCultureIgnoreCase) +``` + +## Configuration + +```powershell +Rules = @{ + PSAvoidUsingNewObject = @{ + 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 fac3c7d40..3985325ef 100644 --- a/docs/Rules/README.md +++ b/docs/Rules/README.md @@ -38,6 +38,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 | |