From 51ab7041788e4a96049846bb0bfc2e9b305e592b Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Sun, 15 Jun 2025 11:26:10 -0500 Subject: [PATCH 1/4] feat: PSAvoidUsingNewObject --- .../Commands/InvokeScriptAnalyzerCommand.cs | 10 +- Engine/Settings/PSGallery.psd1 | 1 + Engine/Settings/ScriptFunctions.psd1 | 3 +- Rules/AvoidUsingNewObject.cs | 373 ++++++++++++++++++ Rules/Strings.resx | 12 + Tests/Rules/AvoidUsingNewObject.ps1 | 90 +++++ Tests/Rules/AvoidUsingNewObject.tests.ps1 | 27 ++ .../Rules/AvoidUsingNewObjectNoViolations.ps1 | 86 ++++ 8 files changed, 596 insertions(+), 6 deletions(-) create mode 100644 Rules/AvoidUsingNewObject.cs create mode 100644 Tests/Rules/AvoidUsingNewObject.ps1 create mode 100644 Tests/Rules/AvoidUsingNewObject.tests.ps1 create mode 100644 Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index a444327e0..edcc79cba 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -340,11 +340,11 @@ protected override void BeginProcessing() settingsObj?.CustomRulePath?.ToArray(), this.SessionState, combRecurseCustomRulePath); - combRulePaths = rulePaths == null - ? settingsCustomRulePath - : settingsCustomRulePath == null - ? rulePaths - : rulePaths.Concat(settingsCustomRulePath).ToArray(); + combRulePaths = rulePaths == null + ? settingsCustomRulePath + : settingsCustomRulePath == null + ? rulePaths + : rulePaths.Concat(settingsCustomRulePath).ToArray(); } catch (Exception exception) { diff --git a/Engine/Settings/PSGallery.psd1 b/Engine/Settings/PSGallery.psd1 index 38fd93a4f..f900b2c30 100644 --- a/Engine/Settings/PSGallery.psd1 +++ b/Engine/Settings/PSGallery.psd1 @@ -16,6 +16,7 @@ 'PSAvoidGlobalVars', 'PSUseDeclaredVarsMoreThanAssignments', 'PSAvoidUsingInvokeExpression', + 'PSAvoidUsingNewObject', 'PSAvoidUsingPlainTextForPassword', 'PSAvoidUsingComputerNameHardcoded', 'PSUsePSCredentialType', diff --git a/Engine/Settings/ScriptFunctions.psd1 b/Engine/Settings/ScriptFunctions.psd1 index b394abddf..79c39a9d5 100644 --- a/Engine/Settings/ScriptFunctions.psd1 +++ b/Engine/Settings/ScriptFunctions.psd1 @@ -7,5 +7,6 @@ 'PSAvoidUsingPositionalParameters', 'PSAvoidGlobalVars', 'PSUseDeclaredVarsMoreThanAssignments', - 'PSAvoidUsingInvokeExpression') + 'PSAvoidUsingInvokeExpression', + 'PSAvoidUsingNewObject') } \ No newline at end of file diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs new file mode 100644 index 000000000..965fd288d --- /dev/null +++ b/Rules/AvoidUsingNewObject.cs @@ -0,0 +1,373 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +using System; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Management.Automation.Language; +using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; + +#if !CORECLR +using System.ComponentModel.Composition; +#endif + +namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules +{ + /// + /// AvoidUsingNewObject: Check to make sure that New-Object is not used. + /// +#if !CORECLR + [Export(typeof(IScriptRule))] +#endif + public class AvoidUsingNewObject : IScriptRule + { + private const string _cmdletName = "New-Object"; + private const string _comObjectParameterName = "ComObject"; + private const string _typeNameParameterName = "TypeName"; + + private Ast _rootAst; + + /// + /// AnalyzeScript: Analyzes the given Ast and returns DiagnosticRecords based on the analysis. + /// + /// The script's ast + /// The name of the script file being analyzed + /// The results of the analysis + public IEnumerable AnalyzeScript(Ast ast, string fileName) + { + if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); + + // Required to query New-Object Parameter Splats / Variable reassignments + _rootAst = ast; + + IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); + + foreach (CommandAst cmdAst in commandAsts) + { + if (cmdAst.GetCommandName() == null) continue; + + if (!string.Equals(cmdAst.GetCommandName(), _cmdletName, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (IsComObject(cmdAst)) + { + continue; + } + + yield return new DiagnosticRecord( + GetError(), + cmdAst.Extent, + GetName(), + GetDiagnosticSeverity(), + fileName, + cmdAst.GetCommandName()); + } + } + + /// + /// Determines if the New-Object command is creating a COM object. + /// + /// The CommandAst representing the New-Object command + /// True if the command is creating a COM object, false otherwise + private bool IsComObject(CommandAst cmdAst) + { + foreach (var element in cmdAst.CommandElements) + { + if (element is CommandParameterAst cmdParameterAst) + { + if (string.Equals(cmdParameterAst.ParameterName, _comObjectParameterName, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + else if (string.Equals(cmdParameterAst.ParameterName, _typeNameParameterName, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + continue; + } + else if (element is VariableExpressionAst splattedVarAst && splattedVarAst.Splatted) + { + return ProcessParameterSplat(splattedVarAst); + } + else if (element is UsingExpressionAst usingExprAst && usingExprAst.SubExpression is VariableExpressionAst usingVarAst) + { + if (!usingVarAst.Splatted) + { + continue; + } + + return ProcessParameterSplat(usingVarAst); + } + } + + return false; + } + + /// + /// Processes a Parameter Splat to determine if it contains COM object parameter. + /// + /// The VariableExpressionAst representing the splatted variable + /// True if the variable contains COM object parameter, false otherwise + private bool ProcessParameterSplat(VariableExpressionAst splattedVarAst) + { + var variableName = Helper.Instance.VariableNameWithoutScope(splattedVarAst.VariablePath); + var visitedVariables = new HashSet(StringComparer.OrdinalIgnoreCase); + + return ProcessVariable(variableName, visitedVariables); + } + + /// + /// Recursively processes variable assignments to trace back to the original hashtable definition. + /// + /// The name of the variable to process + /// Set of already visited variables to prevent infinite recursion + /// True if the variable chain leads to a COM object parameter, false otherwise + private bool ProcessVariable(string variableName, HashSet visitedVariables) + { + // Prevent infinite recursion + if (visitedVariables.Contains(variableName)) + { + return false; + } + + visitedVariables.Add(variableName); + + var assignments = FindAssignmentsInScope(variableName); + + foreach (var assignment in assignments) + { + var hashtableAst = GetHashtableFromAssignment(assignment); + if (hashtableAst != null) + { + return CheckHashtableForComObjectKey(hashtableAst); + } + + var sourceVariable = GetVariableFromAssignment(assignment); + if (sourceVariable != null) + { + var result = ProcessVariable(sourceVariable, visitedVariables); + + if (result) + { + return true; + } + } + } + + return false; + } + + /// + /// Extracts the source variable name from a variable-to-variable assignment. + /// + /// The AssignmentStatementAst to analyze + /// The name of the source variable, or null if not a variable assignment + private static string GetVariableFromAssignment(AssignmentStatementAst assignmentAst) + { + var rightHandSide = assignmentAst.Right; + + if (rightHandSide is PipelineAst pipelineAst && + pipelineAst.PipelineElements.Count == 1 && + pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr && + cmdExpr.Expression is VariableExpressionAst varExpr) + { + return Helper.Instance.VariableNameWithoutScope(varExpr.VariablePath); + } + + if (rightHandSide is CommandExpressionAst commandExpr && + commandExpr.Expression is VariableExpressionAst directVarExpr) + { + return Helper.Instance.VariableNameWithoutScope(directVarExpr.VariablePath); + } + + return null; + } + + /// + /// Finds all assignment statements in the current scope that assign to the specified variable. + /// + /// The name of the variable to search for + /// A list of AssignmentStatementAst objects that assign to the variable + private List FindAssignmentsInScope(string variableName) + { + var assignments = new List(); + + var allAssignments = _rootAst.FindAll(ast => ast is AssignmentStatementAst, true); + + foreach (var assignment in allAssignments.Cast()) + { + VariableExpressionAst leftVarAst = null; + + // Handle direct variable assignment: $var = ... + if (assignment.Left is VariableExpressionAst leftVar) + { + leftVarAst = leftVar; + } + // Handle typed assignment: [type]$var = ... + else if (assignment.Left is ConvertExpressionAst convertAst && + convertAst.Child is VariableExpressionAst typedVar) + { + leftVarAst = typedVar; + } + + if (leftVarAst == null) + { + continue; + } + + var leftVarName = Helper.Instance.VariableNameWithoutScope(leftVarAst.VariablePath); + + if (string.Equals(leftVarName, variableName, StringComparison.OrdinalIgnoreCase)) + { + assignments.Add(assignment); + } + } + + return assignments; + } + + /// + /// Checks if a hashtable contains a ComObject key or TypeName key to determine if it's for COM object creation. + /// + /// The HashtableAst to examine + /// True if the hashtable contains a ComObject key, false if it contains TypeName or neither + private static bool CheckHashtableForComObjectKey(HashtableAst hashtableAst) + { + foreach (var keyValuePair in hashtableAst.KeyValuePairs) + { + if (keyValuePair.Item1 is StringConstantExpressionAst keyAst) + { + // If the key is ComObject, it's a COM object + if (string.Equals(keyAst.Value, _comObjectParameterName, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + // If the key is TypeName, it's not a COM object + else if (string.Equals(keyAst.Value, _typeNameParameterName, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + } + } + return false; + } + + /// + /// Extracts a hashtable from an assignment statement's right-hand side. + /// + /// The AssignmentStatementAst to analyze + /// The HashtableAst if found, null otherwise + private static HashtableAst GetHashtableFromAssignment(AssignmentStatementAst assignmentAst) + { + var rightHandSide = assignmentAst.Right; + + HashtableAst hashtable = ExtractHashtableFromStatement(rightHandSide); + + if (hashtable != null) + { + return hashtable; + } + + var hashtableNodes = rightHandSide.FindAll(ast => ast is HashtableAst, true); + return hashtableNodes.FirstOrDefault() as HashtableAst; + } + + /// + /// Extracts a hashtable from various statement types (PipelineAst, CommandExpressionAst). + /// + /// The StatementAst to analyze + /// The HashtableAst if found, null otherwise + private static HashtableAst ExtractHashtableFromStatement(StatementAst statement) + { + switch (statement) + { + case PipelineAst pipelineAst when pipelineAst.PipelineElements.Count >= 1: + if (pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr && + cmdExpr.Expression is HashtableAst hashtableFromPipeline) + { + return hashtableFromPipeline; + } + break; + + case CommandExpressionAst commandExpr when commandExpr.Expression is HashtableAst hashtableFromCommand: + return hashtableFromCommand; + } + + return null; + } + + /// + /// GetError: Retrieves the error message + /// + /// The error message + public string GetError() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectError); + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingNewObjectName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); + } + + /// + /// Method: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity:Retrieves the severity of the rule: error, warning of information. + /// + /// The severity of the rule + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// + /// The diagnostic severity of the rule + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Method: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + } +} \ No newline at end of file diff --git a/Rules/Strings.resx b/Rules/Strings.resx index 260214967..e53caff6c 100644 --- a/Rules/Strings.resx +++ b/Rules/Strings.resx @@ -135,6 +135,12 @@ Avoid Using Invoke-Expression + + For creating .NET objects, prefer using type literals (e.g., [Some.Type]::new()) over the New-Object cmdlet. Type literals offer better performance, compile-time validation, and clearer intent, especially for well-known types. + + + Avoid Using New-Object + Readability and clarity should be the goal of any script we expect to maintain over time. When calling a command that takes parameters, where possible consider using name parameters as opposed to positional parameters. To fix a violation of this rule, please use named parameters instead of positional parameters when calling a command. @@ -381,6 +387,9 @@ AvoidUsingInvokeExpression + + AvoidUsingNewObject + AvoidUsingPlainTextForPassword @@ -519,6 +528,9 @@ Invoke-Expression is used. Please remove Invoke-Expression from script and find other options instead. + + Consider replacing the 'New-Object' cmdlet with type literals (e.g., '[System.Object]::new()') for creating .NET objects. This approach is generally more performant and idiomatic. + Cmdlet '{0}' has positional parameter. Please use named parameters instead of positional parameters when calling a command. diff --git a/Tests/Rules/AvoidUsingNewObject.ps1 b/Tests/Rules/AvoidUsingNewObject.ps1 new file mode 100644 index 000000000..217cfeebf --- /dev/null +++ b/Tests/Rules/AvoidUsingNewObject.ps1 @@ -0,0 +1,90 @@ +$hashset1 = New-Object System.Collections.Generic.HashSet[String] # Issue #1 + +[Hashtable] $param = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' +} + +New-Object @param # Issue #2 + +function Test +{ + $hashset2 = New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #3 + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #4 + + [Hashtable] $param = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + New-Object @param # Issue #5 + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #6 + + [Hashtable] $param = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + New-Object @param # Issue #7 + } + + function Test2 + { + [Cmdletbinding()] + Param () + + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #8 + + [Hashtable] $param = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + New-Object @param # Issue #9 + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #10 + } + } +} + +class TestClass +{ + [System.Collections.Generic.HashSet[String]] $Set + [System.Collections.Generic.HashSet[String]] $Set2 = (New-Object -TypeName System.Collections.Generic.HashSet[String]) # Issue #11 + + TestClass () + { + $this.Set = (New-Object -TypeName System.Collections.Generic.HashSet[String]) # Issue #12 + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #13 + + } + + [Hashtable] $param = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + New-Object @param # Issue #14 + } +} + +[Hashtable] $script:params = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' +} + +function Test-Scope +{ + New-Object @script:params # Issue #15 + + $params2 = $script:params + New-Object @params2 # Issue #16 + + [Hashtable] $params3 = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object @using:params3 # Issue #17 + } +} + diff --git a/Tests/Rules/AvoidUsingNewObject.tests.ps1 b/Tests/Rules/AvoidUsingNewObject.tests.ps1 new file mode 100644 index 000000000..529448c51 --- /dev/null +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -0,0 +1,27 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + +BeforeAll { + $NewObjectMessage = "Consider replacing the 'New-Object' cmdlet with type literals" + $NewObjectName = "PSAvoidUsingNewObject" + $violations = Invoke-ScriptAnalyzer "$PSScriptRoot\AvoidUsingNewObject.ps1" | Where-Object { $_.RuleName -eq $NewObjectName } + $noViolations = Invoke-ScriptAnalyzer "$PSScriptRoot\AvoidUsingNewObjectNoViolations.ps1" | Where-Object { $_.RuleName -eq $NewObjectName } +} + +Describe "AvoidUsingNewObject" { + Context "When there are violations" { + It "has 17 New-Object violations" { + $violations.Count | Should -Be 17 + } + + It "has the correct description message for New-Object" { + $violations[0].Message | Should -Match $NewObjectMessage + } + } + + Context "When there are no violations" { + It "has 0 violations" { + $noViolations.Count | Should -Be 0 + } + } +} diff --git a/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 b/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 new file mode 100644 index 000000000..9736d62b6 --- /dev/null +++ b/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 @@ -0,0 +1,86 @@ +New-Object -ComObject "WScript.Shell" + +[Hashtable] $param = @{ + ComObject = "WScript.Shell" +} + +New-Object @param + +function Test +{ + New-Object -ComObject "WScript.Shell" + + [Hashtable] $param = @{ + ComObject = "WScript.Shell" + } + + New-Object @param + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -ComObject "WScript.Shell" + + [Hashtable] $param = @{ + ComObject = "WScript.Shell" + } + + New-Object @param + } + + function Test2 + { + [Cmdletbinding()] + Param () + + New-Object -ComObject "WScript.Shell" + + [Hashtable] $param = @{ + ComObject = "WScript.Shell" + } + + New-Object @param + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -ComObject "WScript.Shell" + } + } +} + +class TestClass +{ + TestClass () + { + New-Object -ComObject "WScript.Shell" + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object -ComObject "WScript.Shell" + + } + + [Hashtable] $param = @{ + ComObject = "WScript.Shell" + } + + New-Object @param + } +} + +[Hashtable] $script:params = @{ + ComObject = "WScript.Shell" +} + +function Test-Scope +{ + New-Object @script:params + + $params2 = $script:params + New-Object @params2 + + [Hashtable] $params3 = @{ + ComObject = "WScript.Shell" + } + + Invoke-Command -ComputerName "localhost" -ScriptBlock { + New-Object @using:params3 + } +} + From 4422b7e223a773ed3e902f80bd9f72acb9e84577 Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Sun, 15 Jun 2025 18:47:24 -0500 Subject: [PATCH 2/4] ref: Support variable resolution in hashtables and partial completions --- Rules/AvoidUsingNewObject.cs | 467 ++++++++++++------ Tests/Rules/AvoidUsingNewObject.ps1 | 17 + Tests/Rules/AvoidUsingNewObject.tests.ps1 | 4 +- .../Rules/AvoidUsingNewObjectNoViolations.ps1 | 26 +- 4 files changed, 354 insertions(+), 160 deletions(-) diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs index 965fd288d..6c1e7b27d 100644 --- a/Rules/AvoidUsingNewObject.cs +++ b/Rules/AvoidUsingNewObject.cs @@ -6,6 +6,7 @@ using System.Globalization; using System.Linq; using System.Management.Automation.Language; +using System.Text.RegularExpressions; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; #if !CORECLR @@ -22,12 +23,36 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules #endif public class AvoidUsingNewObject : IScriptRule { + #region Singleton members private const string _cmdletName = "New-Object"; - private const string _comObjectParameterName = "ComObject"; - private const string _typeNameParameterName = "TypeName"; + // We only want to check for the first letter to allow for parameter appreviations. + // For Example: -ComObject, -Com, -C. + private static Regex _comObjectParameterRegex = new Regex( + "^C", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + TimeSpan.FromSeconds(1) + ); + + // We only want to check for the first letter to allow for parameter appreviations. + // For Example: -TypeName, -Type, -T. + private static Regex _typeNameParameterRegex = new Regex( + "^T", + RegexOptions.Compiled | RegexOptions.IgnoreCase, + TimeSpan.FromSeconds(1) + ); + + #endregion + + #region Private members + + // Required to query New-Object Parameter Splats / Variable reassignments private Ast _rootAst; + #endregion + + #region Public methods + /// /// AnalyzeScript: Analyzes the given Ast and returns DiagnosticRecords based on the analysis. /// @@ -38,7 +63,6 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); - // Required to query New-Object Parameter Splats / Variable reassignments _rootAst = ast; IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); @@ -67,6 +91,80 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) } } + #endregion + + #region Private methods + + /// + /// GetError: Retrieves the error message + /// + /// The error message + public string GetError() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectError); + } + + /// + /// GetName: Retrieves the name of this rule. + /// + /// The name of this rule + public string GetName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingNewObjectName); + } + + /// + /// GetCommonName: Retrieves the common name of this rule. + /// + /// The common name of this rule + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); + } + + /// + /// GetDescription: Retrieves the description of this rule. + /// + /// The description of this rule + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); + } + + /// + /// Method: Retrieves the type of the rule: builtin, managed or module. + /// + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + /// + /// GetSeverity:Retrieves the severity of the rule: error, warning of information. + /// + /// The severity of the rule + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// + /// The diagnostic severity of the rule + public DiagnosticSeverity GetDiagnosticSeverity() + { + return DiagnosticSeverity.Warning; + } + + /// + /// Method: Retrieves the module/assembly name the rule is from. + /// + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + /// /// Determines if the New-Object command is creating a COM object. /// @@ -78,48 +176,40 @@ private bool IsComObject(CommandAst cmdAst) { if (element is CommandParameterAst cmdParameterAst) { - if (string.Equals(cmdParameterAst.ParameterName, _comObjectParameterName, StringComparison.OrdinalIgnoreCase)) - { + var paramName = cmdParameterAst.ParameterName; + if (string.IsNullOrEmpty(paramName)) + continue; + + if (_comObjectParameterRegex.IsMatch(paramName)) return true; - } - else if (string.Equals(cmdParameterAst.ParameterName, _typeNameParameterName, StringComparison.OrdinalIgnoreCase)) - { + + if (_typeNameParameterRegex.IsMatch(paramName)) return false; - } continue; } - else if (element is VariableExpressionAst splattedVarAst && splattedVarAst.Splatted) - { - return ProcessParameterSplat(splattedVarAst); - } - else if (element is UsingExpressionAst usingExprAst && usingExprAst.SubExpression is VariableExpressionAst usingVarAst) + + if (element is VariableExpressionAst splattedVarAst && splattedVarAst.Splatted) + return ProcessVariableExpression(splattedVarAst); + + if (element is ExpandableStringExpressionAst expandableStringAst) + return ProcessExpandableExpression(expandableStringAst); + + if (element is UsingExpressionAst usingExprAst) { + if (!(usingExprAst.SubExpression is VariableExpressionAst usingVarAst)) + continue; + if (!usingVarAst.Splatted) - { continue; - } - return ProcessParameterSplat(usingVarAst); + return ProcessVariableExpression(usingVarAst); } } return false; } - /// - /// Processes a Parameter Splat to determine if it contains COM object parameter. - /// - /// The VariableExpressionAst representing the splatted variable - /// True if the variable contains COM object parameter, false otherwise - private bool ProcessParameterSplat(VariableExpressionAst splattedVarAst) - { - var variableName = Helper.Instance.VariableNameWithoutScope(splattedVarAst.VariablePath); - var visitedVariables = new HashSet(StringComparer.OrdinalIgnoreCase); - - return ProcessVariable(variableName, visitedVariables); - } - /// /// Recursively processes variable assignments to trace back to the original hashtable definition. /// @@ -128,33 +218,36 @@ private bool ProcessParameterSplat(VariableExpressionAst splattedVarAst) /// True if the variable chain leads to a COM object parameter, false otherwise private bool ProcessVariable(string variableName, HashSet visitedVariables) { - // Prevent infinite recursion if (visitedVariables.Contains(variableName)) - { return false; - } visitedVariables.Add(variableName); - var assignments = FindAssignmentsInScope(variableName); + var assignments = FindAssignmentStatements(variableName); foreach (var assignment in assignments) { var hashtableAst = GetHashtableFromAssignment(assignment); if (hashtableAst != null) - { - return CheckHashtableForComObjectKey(hashtableAst); - } + return ContainsComObjectKey(hashtableAst); var sourceVariable = GetVariableFromAssignment(assignment); if (sourceVariable != null) { - var result = ProcessVariable(sourceVariable, visitedVariables); + if (ProcessVariable(sourceVariable, visitedVariables)) + return true; - if (result) - { + continue; + } + + var stringConstant = GetStringConstantFromAssignment(assignment); + if (stringConstant != null) + { + if (_comObjectParameterRegex.IsMatch(stringConstant)) return true; - } + + if (_typeNameParameterRegex.IsMatch(stringConstant)) + return false; } } @@ -162,69 +255,90 @@ private bool ProcessVariable(string variableName, HashSet visitedVariabl } /// - /// Extracts the source variable name from a variable-to-variable assignment. + /// Extracts a string constant from an assignment statement's right-hand side. /// /// The AssignmentStatementAst to analyze - /// The name of the source variable, or null if not a variable assignment - private static string GetVariableFromAssignment(AssignmentStatementAst assignmentAst) + /// The string constant value if found, null otherwise + private static string GetStringConstantFromAssignment(AssignmentStatementAst assignmentAst) { var rightHandSide = assignmentAst.Right; - if (rightHandSide is PipelineAst pipelineAst && - pipelineAst.PipelineElements.Count == 1 && - pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr && - cmdExpr.Expression is VariableExpressionAst varExpr) + if (rightHandSide is PipelineAst pipelineAst) { - return Helper.Instance.VariableNameWithoutScope(varExpr.VariablePath); + if (pipelineAst.PipelineElements.Count != 1) + return null; + + if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) + return null; + + if (!(cmdExpr.Expression is StringConstantExpressionAst stringExpr)) + return null; + + return stringExpr.Value; } - if (rightHandSide is CommandExpressionAst commandExpr && - commandExpr.Expression is VariableExpressionAst directVarExpr) + // Handle direct CommandExpressionAst -> StringConstantExpressionAst + if (rightHandSide is CommandExpressionAst commandExpr) { - return Helper.Instance.VariableNameWithoutScope(directVarExpr.VariablePath); + if (!(commandExpr.Expression is StringConstantExpressionAst directStringExpr)) + return null; + + return directStringExpr.Value; } return null; } + /// + /// Processes VariableExpressionAst to determine if the variable potentially matches 'ComObject' parameter. + /// + /// The VariableExpressionAst to process + /// True if the variable contains COM object parameter, false otherwise + private bool ProcessVariableExpression(VariableExpressionAst variableAst) + { + var variableName = GetVariableNameWithoutScope(variableAst); + var visitedVariables = new HashSet(StringComparer.OrdinalIgnoreCase); + + return ProcessVariable(variableName, visitedVariables); + } + + /// + /// Attempts to resolve expandable strings to its variable value. + /// + /// The ExpandableStringExpressionAst to resolve + /// The resolved string value, or null if it cannot be determined + private bool ProcessExpandableExpression(ExpandableStringExpressionAst expandableAst) + { + if (expandableAst.NestedExpressions.Count == 0) + return false; + + if (!(expandableAst.NestedExpressions[0] is VariableExpressionAst singleVar)) + return false; + + return ProcessVariableExpression(singleVar); + } + /// /// Finds all assignment statements in the current scope that assign to the specified variable. /// /// The name of the variable to search for /// A list of AssignmentStatementAst objects that assign to the variable - private List FindAssignmentsInScope(string variableName) + private List FindAssignmentStatements(string variableName) { var assignments = new List(); - var allAssignments = _rootAst.FindAll(ast => ast is AssignmentStatementAst, true); foreach (var assignment in allAssignments.Cast()) { - VariableExpressionAst leftVarAst = null; - - // Handle direct variable assignment: $var = ... - if (assignment.Left is VariableExpressionAst leftVar) - { - leftVarAst = leftVar; - } - // Handle typed assignment: [type]$var = ... - else if (assignment.Left is ConvertExpressionAst convertAst && - convertAst.Child is VariableExpressionAst typedVar) - { - leftVarAst = typedVar; - } - + VariableExpressionAst leftVarAst = GetLeftVariableFromAssignment(assignment); if (leftVarAst == null) - { continue; - } - var leftVarName = Helper.Instance.VariableNameWithoutScope(leftVarAst.VariablePath); + var leftVarName = GetVariableNameWithoutScope(leftVarAst); + if (!string.Equals(leftVarName, variableName, StringComparison.OrdinalIgnoreCase)) + continue; - if (string.Equals(leftVarName, variableName, StringComparison.OrdinalIgnoreCase)) - { - assignments.Add(assignment); - } + assignments.Add(assignment); } return assignments; @@ -235,27 +349,79 @@ private List FindAssignmentsInScope(string variableName) /// /// The HashtableAst to examine /// True if the hashtable contains a ComObject key, false if it contains TypeName or neither - private static bool CheckHashtableForComObjectKey(HashtableAst hashtableAst) + private bool ContainsComObjectKey(HashtableAst hashtableAst) { foreach (var keyValuePair in hashtableAst.KeyValuePairs) { - if (keyValuePair.Item1 is StringConstantExpressionAst keyAst) + if (keyValuePair.Item1 is StringConstantExpressionAst stringKey) { - // If the key is ComObject, it's a COM object - if (string.Equals(keyAst.Value, _comObjectParameterName, StringComparison.OrdinalIgnoreCase)) - { + string paramName = stringKey.Value; + if (string.IsNullOrEmpty(paramName)) + continue; + + if (_comObjectParameterRegex.IsMatch(paramName)) return true; - } - // If the key is TypeName, it's not a COM object - else if (string.Equals(keyAst.Value, _typeNameParameterName, StringComparison.OrdinalIgnoreCase)) - { + + if (_typeNameParameterRegex.IsMatch(paramName)) return false; - } + + continue; + } + + if (keyValuePair.Item1 is VariableExpressionAst varKey) + { + if (ProcessVariableExpression(varKey)) + return true; + + continue; + } + + if (keyValuePair.Item1 is ExpandableStringExpressionAst expandableKey) + { + if (ProcessExpandableExpression(expandableKey)) + return true; } } + return false; } + #endregion + + #region Static methods + + /// + /// Extracts a hashtable from various statement types (PipelineAst, CommandExpressionAst). + /// + /// The StatementAst to analyze + /// The HashtableAst if found, null otherwise + private static HashtableAst ExtractHashtableFromStatement(StatementAst statement) + { + if (statement is PipelineAst pipelineAst) + { + if (pipelineAst.PipelineElements.Count < 1) + return null; + + if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) + return null; + + if (!(cmdExpr.Expression is HashtableAst hashtableFromPipeline)) + return null; + + return hashtableFromPipeline; + } + + if (statement is CommandExpressionAst commandExpr) + { + if (!(commandExpr.Expression is HashtableAst hashtableFromCommand)) + return null; + + return hashtableFromCommand; + } + + return null; + } + /// /// Extracts a hashtable from an assignment statement's right-hand side. /// @@ -268,106 +434,99 @@ private static HashtableAst GetHashtableFromAssignment(AssignmentStatementAst as HashtableAst hashtable = ExtractHashtableFromStatement(rightHandSide); if (hashtable != null) - { return hashtable; - } var hashtableNodes = rightHandSide.FindAll(ast => ast is HashtableAst, true); return hashtableNodes.FirstOrDefault() as HashtableAst; } /// - /// Extracts a hashtable from various statement types (PipelineAst, CommandExpressionAst). + /// Extracts the left-hand variable from an assignment statement. /// - /// The StatementAst to analyze - /// The HashtableAst if found, null otherwise - private static HashtableAst ExtractHashtableFromStatement(StatementAst statement) + /// The assignment statement + /// The variable expression, or null if not found + private static VariableExpressionAst GetLeftVariableFromAssignment(AssignmentStatementAst assignment) { - switch (statement) - { - case PipelineAst pipelineAst when pipelineAst.PipelineElements.Count >= 1: - if (pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr && - cmdExpr.Expression is HashtableAst hashtableFromPipeline) - { - return hashtableFromPipeline; - } - break; - - case CommandExpressionAst commandExpr when commandExpr.Expression is HashtableAst hashtableFromCommand: - return hashtableFromCommand; - } + if (assignment.Left is VariableExpressionAst leftVar) + return leftVar; + + if (assignment.Left is ConvertExpressionAst convertAst && + convertAst.Child is VariableExpressionAst typedVar) + return typedVar; return null; } /// - /// GetError: Retrieves the error message + /// Extracts the source variable name from a variable-to-variable assignment. /// - /// The error message - public string GetError() + /// The AssignmentStatementAst to analyze + /// The name of the source variable, or null if not a variable assignment + private static string GetVariableFromAssignment(AssignmentStatementAst assignmentAst) { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectError); - } + var rightHandSide = assignmentAst.Right; - /// - /// GetName: Retrieves the name of this rule. - /// - /// The name of this rule - public string GetName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingNewObjectName); - } + if (rightHandSide is PipelineAst pipelineAst) + { + if (pipelineAst.PipelineElements.Count != 1) + return null; - /// - /// GetCommonName: Retrieves the common name of this rule. - /// - /// The common name of this rule - public string GetCommonName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); - } + if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) + return null; - /// - /// GetDescription: Retrieves the description of this rule. - /// - /// The description of this rule - public string GetDescription() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); - } + if (!(cmdExpr.Expression is VariableExpressionAst varExpr)) + return null; - /// - /// Method: Retrieves the type of the rule: builtin, managed or module. - /// - public SourceType GetSourceType() - { - return SourceType.Builtin; - } + return GetVariableNameWithoutScope(varExpr); + } - /// - /// GetSeverity:Retrieves the severity of the rule: error, warning of information. - /// - /// The severity of the rule - public RuleSeverity GetSeverity() - { - return RuleSeverity.Warning; + if (rightHandSide is CommandExpressionAst commandExpr) + { + if (!(commandExpr.Expression is VariableExpressionAst directVarExpr)) + return null; + + return GetVariableNameWithoutScope(directVarExpr); + } + + return null; } /// - /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// Extracts the variable name without scope from a VariableExpressionAst. + /// Additionally, trims quotes from the variable name (required for expandable strings). /// - /// The diagnostic severity of the rule - public DiagnosticSeverity GetDiagnosticSeverity() + /// The VariableExpressionAst to extract the variable name from + /// The variable name without scope + private static string GetVariableNameWithoutScope(VariableExpressionAst variableAst) { - return DiagnosticSeverity.Warning; + var variableName = Helper.Instance.VariableNameWithoutScope(variableAst.VariablePath); + return TrimQuotes(variableName); } /// - /// Method: Retrieves the module/assembly name the rule is from. + /// ExpandableStringExpressionAst's with quotes will not resolve to a non-quoted string. + /// It's necessary to trim the quotes from the input string in order to successfully lookup + /// the variable value. /// - public string GetSourceName() + /// The input string to trim + /// The trimmed string, or the original string if it doesn't contain quotes + private static string TrimQuotes(string input) { - return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + if (input.Length < 2) + return input; + + char first = input[0]; + char last = input[input.Length - 1]; + + if (first != last) + return input; + + if (first == '"' || first == '\'') + return input.Substring(1, input.Length - 2); + + return input; } + + #endregion } } \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingNewObject.ps1 b/Tests/Rules/AvoidUsingNewObject.ps1 index 217cfeebf..0ee714183 100644 --- a/Tests/Rules/AvoidUsingNewObject.ps1 +++ b/Tests/Rules/AvoidUsingNewObject.ps1 @@ -88,3 +88,20 @@ function Test-Scope } } +$scripbBlockTest = { + New-Object -TypeName System.Collections.Generic.HashSet[String] # Issue #18 + + [Hashtable] $params4 = @{ + TypeName = 'System.Collections.Generic.HashSet[String]' + } + + New-Object @params4 # Issue #19 +} + +$test = "co" +$value = "WScript.Shell" +[hashtable] $test1 = @{ + "$test" = $value +} + +New-Object @test1 # Issue #20 \ No newline at end of file diff --git a/Tests/Rules/AvoidUsingNewObject.tests.ps1 b/Tests/Rules/AvoidUsingNewObject.tests.ps1 index 529448c51..5c2f27c62 100644 --- a/Tests/Rules/AvoidUsingNewObject.tests.ps1 +++ b/Tests/Rules/AvoidUsingNewObject.tests.ps1 @@ -10,8 +10,8 @@ BeforeAll { Describe "AvoidUsingNewObject" { Context "When there are violations" { - It "has 17 New-Object violations" { - $violations.Count | Should -Be 17 + It "has 20 New-Object violations" { + $violations.Count | Should -Be 20 } It "has the correct description message for New-Object" { diff --git a/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 b/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 index 9736d62b6..ca0c86aa5 100644 --- a/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 +++ b/Tests/Rules/AvoidUsingNewObjectNoViolations.ps1 @@ -8,10 +8,11 @@ New-Object @param function Test { - New-Object -ComObject "WScript.Shell" + $partialParameter = "CO" + New-Object -"$partialParameter" "WScript.Shell" [Hashtable] $param = @{ - ComObject = "WScript.Shell" + ComO = "WScript.Shell" } New-Object @param @@ -20,7 +21,7 @@ function Test New-Object -ComObject "WScript.Shell" [Hashtable] $param = @{ - ComObject = "WScript.Shell" + CO = "WScript.Shell" } New-Object @param @@ -34,7 +35,7 @@ function Test New-Object -ComObject "WScript.Shell" [Hashtable] $param = @{ - ComObject = "WScript.Shell" + C = "WScript.Shell" } New-Object @param @@ -84,3 +85,20 @@ function Test-Scope } } +$scripbBlockTest = { + New-Object -ComObject "WScript.Shell" + + [Hashtable] $params4 = @{ + ComObject = 'WScript.Shell' + } + + New-Object @params4 +} + +$partialKey = "COMO" +$value = "WScript.Shell" +[hashtable] $partialKeyParams = @{ + "$partialKey" = $value +} + +New-Object @partialKeyParams \ No newline at end of file From da95630832712acc088929363ffaa2d40660279f Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Mon, 16 Jun 2025 13:23:19 -0500 Subject: [PATCH 3/4] fix: Add missing Documentation --- docs/Rules/AvoidUsingNewObject.md | 256 ++++++++++++++++++++++++++++++ 1 file changed, 256 insertions(+) create mode 100644 docs/Rules/AvoidUsingNewObject.md diff --git a/docs/Rules/AvoidUsingNewObject.md b/docs/Rules/AvoidUsingNewObject.md new file mode 100644 index 000000000..b3603ae1e --- /dev/null +++ b/docs/Rules/AvoidUsingNewObject.md @@ -0,0 +1,256 @@ +--- +description: Avoid Using New-Object +ms.date: 06/14/2025 +ms.topic: reference +title: AvoidUsingNewObject +--- +# AvoidUsingNewObject + +**Severity Level: Warning** + +## Description + +The `New-Object` cmdlet should be avoided in modern PowerShell code except when creating COM objects. PowerShell provides more efficient, readable, and idiomatic alternatives for object creation that offer better performance and cleaner syntax. + +This rule flags all uses of `New-Object` ***except*** when used with the `-ComObject` parameter, as COM object creation is one of the few legitimate remaining use cases for this cmdlet. + +## Why Avoid New-Object? + +### Performance Issues +`New-Object` uses reflection internally, which is significantly slower than direct type instantiation or accelerated type syntax. The performance difference becomes more pronounced in loops or frequently executed code. + +### Readability and Maintainability +Modern PowerShell syntax is more concise and easier to read than `New-Object` constructions, especially for common .NET types. + +### PowerShell Best Practices +The PowerShell community and Microsoft recommend using native PowerShell syntax over legacy cmdlets when better alternatives exist. + +## Examples + +### Wrong + +```powershell +# Creating .NET objects +$list = New-Object System.Collections.Generic.List[string] +$hashtable = New-Object System.Collections.Hashtable +$stringBuilder = New-Object System.Text.StringBuilder +$datetime = New-Object System.DateTime(2023, 12, 25) + +# Creating custom objects +$obj = New-Object PSObject -Property @{ + Name = "John" + Age = 30 +} +``` + +### Correct + +```powershell +# Use accelerated type syntax for .NET objects +$list = [System.Collections.Generic.List[string]]::new() +$hashtable = @{} # or [hashtable]@{} +$stringBuilder = [System.Text.StringBuilder]::new() +$datetime = [DateTime]::new(2023, 12, 25) + +# Use PSCustomObject for custom objects +$obj = [PSCustomObject]@{ + Name = "John" + Age = 30 +} + +# COM objects are still acceptable with New-Object +$excel = New-Object -ComObject Excel.Application +$word = New-Object -ComObject Word.Application +``` + +## Alternative Approaches + +### For .NET Types +- **Static `new()` method**: `[TypeName]::new(parameters)` +- **Type accelerators**: Use built-in shortcuts like `@{}` for hashtables +- **Cast operators**: `[TypeName]$value` for type conversion + +### For Custom Objects +- **PSCustomObject**: `[PSCustomObject]@{ Property = Value }` +- **Ordered dictionaries**: `[ordered]@{ Property = Value }` + +### For Collections +- **Array subexpression**: `@(items)` +- **Hashtable literal**: `@{ Key = Value }` +- **Generic collections**: `[System.Collections.Generic.List[Type]]::new()` + +## Performance Comparison + +```powershell +# Slow - uses reflection +Measure-Command { 1..1000 | ForEach-Object { New-Object System.Text.StringBuilder } } + +# Fast - direct instantiation +Measure-Command { 1..1000 | ForEach-Object { [System.Text.StringBuilder]::new() } } +``` + +The modern syntax provides a performance improvement over `New-Object` for most common scenarios. + +## Exceptions + +The rule allows `New-Object` when used with the `-ComObject` parameter because: +- COM object creation requires the `New-Object` cmdlet. +- No direct PowerShell alternative exists for COM instantiation. +- COM objects are external to the .NET type system. + +```powershell +# This is acceptable +$shell = New-Object -ComObject WScript.Shell +$ie = New-Object -ComObject InternetExplorer.Application +``` + +--- + +## Migration Guide + +| Old Syntax | New Syntax | +|------------|------------| +| **Creating a custom object**:
`New-Object PSObject -Property @{ Name = 'John'; Age = 30 }` | **Use PSCustomObject**:
`[PSCustomObject]@{ Name = 'John'; Age = 30 }` | +| **Creating a hashtable**:
`New-Object System.Collections.Hashtable` | **Use hashtable literal**:
`@{}`
**Or explicitly cast**:
`[hashtable]@{}` | +| **Creating a generic list**:
`New-Object 'System.Collections.Generic.List[string]'` | **Use static `new()` method**:
`[System.Collections.Generic.List[string]]::new()` | +| **Creating a DateTime object**:
`New-Object DateTime -ArgumentList 2023, 12, 25` | **Use static `new()` method**:
`[DateTime]::new(2023, 12, 25)` | +| **Creating a StringBuilder**:
`New-Object System.Text.StringBuilder` | **Use static `new()` method**:
`[System.Text.StringBuilder]::new()` | +| **Creating a process object**:
`New-Object System.Diagnostics.Process` | **Use static `new()` method**:
`[System.Diagnostics.Process]::new()` | +| **Creating a custom .NET object**:
`New-Object -TypeName 'Namespace.TypeName' -ArgumentList $args` | **Use static `new()` method**:
`[Namespace.TypeName]::new($args)` | + +--- + +### Detailed Examples + +#### Custom Object Creation + +**Old Syntax:** + +```powershell +$obj = New-Object PSObject -Property @{ + Name = 'John' + Age = 30 +} +``` + +**New Syntax:** + +```powershell +$obj = [PSCustomObject]@{ + Name = 'John' + Age = 30 +} +``` + +#### Hashtable Creation + +**Old Syntax:** + +```powershell +$hashtable = New-Object System.Collections.Hashtable +$hashtable.Add('Key', 'Value') +``` + +**New Syntax:** + +```powershell +$hashtable = @{ + Key = 'Value' +} +``` + +Or explicitly cast: + +```powershell +$hashtable = [hashtable]@{ + Key = 'Value' +} +``` + +#### Generic List Creation + +**Old Syntax:** + +```powershell +$list = New-Object 'System.Collections.Generic.List[string]' +$list.Add('Item1') +$list.Add('Item2') +``` + +**New Syntax:** + +```powershell +$list = [System.Collections.Generic.List[string]]::new() +$list.Add('Item1') +$list.Add('Item2') +``` + +#### DateTime Object Creation + +**Old Syntax:** + +```powershell +$date = New-Object DateTime -ArgumentList 2023, 12, 25 +``` + +**New Syntax:** + +```powershell +$date = [DateTime]::new(2023, 12, 25) +``` + +#### StringBuilder Creation + +**Old Syntax:** + +```powershell +$stringBuilder = New-Object System.Text.StringBuilder +$stringBuilder.Append('Hello') +``` + +**New Syntax:** + +```powershell +$stringBuilder = [System.Text.StringBuilder]::new() +$stringBuilder.Append('Hello') +``` + +#### Custom .NET Object Creation + +**Old Syntax:** + +```powershell +$customObject = New-Object -TypeName 'Namespace.TypeName' -ArgumentList $arg1, $arg2 +``` + +**New Syntax:** + +```powershell +$customObject = [Namespace.TypeName]::new($arg1, $arg2) +``` + +#### Process Object Creation + +**Old Syntax:** + +```powershell +$process = New-Object System.Diagnostics.Process +``` + +**New Syntax:** + +```powershell +$process = [System.Diagnostics.Process]::new() +``` + +--- +## Related Links + +- [New-Object][01] +- [PowerShell scripting performance considerations][02] +- [Creating .NET and COM objects][03] + + +[01]: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.utility/new-object +[02]: https://learn.microsoft.com/en-us/powershell/scripting/dev-cross-plat/performance/script-authoring-considerations +[03]: https://learn.microsoft.com/en-us/powershell/scripting/samples/creating-.net-and-com-objects--new-object- \ No newline at end of file From 26c779cdf5c42da4ac5e5edc4a09e7954d8f85ad Mon Sep 17 00:00:00 2001 From: "Dr. Skill Issue" Date: Tue, 17 Jun 2025 05:34:34 -0500 Subject: [PATCH 4/4] ref: final restructure --- Rules/AvoidUsingNewObject.cs | 744 ++++++++++++++++++++--------------- 1 file changed, 428 insertions(+), 316 deletions(-) diff --git a/Rules/AvoidUsingNewObject.cs b/Rules/AvoidUsingNewObject.cs index 6c1e7b27d..5cc35c34f 100644 --- a/Rules/AvoidUsingNewObject.cs +++ b/Rules/AvoidUsingNewObject.cs @@ -4,9 +4,7 @@ using System; using System.Collections.Generic; using System.Globalization; -using System.Linq; using System.Management.Automation.Language; -using System.Text.RegularExpressions; using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic; #if !CORECLR @@ -16,194 +14,159 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules { /// - /// AvoidUsingNewObject: Check to make sure that New-Object is not used. + /// Flags New-Object usage except when creating COM objects via -ComObject parameter. + /// Supports parameter abbreviation, splatting, variable resolution, and expandable strings. /// #if !CORECLR [Export(typeof(IScriptRule))] #endif public class AvoidUsingNewObject : IScriptRule { - #region Singleton members - private const string _cmdletName = "New-Object"; - - // We only want to check for the first letter to allow for parameter appreviations. - // For Example: -ComObject, -Com, -C. - private static Regex _comObjectParameterRegex = new Regex( - "^C", - RegexOptions.Compiled | RegexOptions.IgnoreCase, - TimeSpan.FromSeconds(1) - ); - - // We only want to check for the first letter to allow for parameter appreviations. - // For Example: -TypeName, -Type, -T. - private static Regex _typeNameParameterRegex = new Regex( - "^T", - RegexOptions.Compiled | RegexOptions.IgnoreCase, - TimeSpan.FromSeconds(1) - ); + #region Constants + + private const string CmdletName = "New-Object"; + private const string ComObjectParameterName = "ComObject"; + private const string TypeNameParameterName = "TypeName"; #endregion - #region Private members + #region Fields - // Required to query New-Object Parameter Splats / Variable reassignments + /// + /// Root AST for variable assignment tracking. + /// private Ast _rootAst; + /// + /// Lazy-loaded cache mapping variable names to their assignment statements. + /// Only initialized when splatted variables are encountered. + /// + private Lazy>> _assignmentCache; + #endregion - #region Public methods + #region Public Methods /// - /// AnalyzeScript: Analyzes the given Ast and returns DiagnosticRecords based on the analysis. + /// Analyzes PowerShell AST for New-Object usage that should be replaced with type literals. /// - /// The script's ast - /// The name of the script file being analyzed - /// The results of the analysis + /// The root AST to analyze + /// Source file name for diagnostic reporting + /// Enumerable of diagnostic records for non-COM New-Object usage + /// Thrown when ast parameter is null public IEnumerable AnalyzeScript(Ast ast, string fileName) { if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage); _rootAst = ast; - IEnumerable commandAsts = ast.FindAll(testAst => testAst is CommandAst, true); - - foreach (CommandAst cmdAst in commandAsts) + // Direct enumeration without intermediate collection + foreach (var node in ast.FindAll(testAst => testAst is CommandAst cmdAst && + string.Equals(cmdAst.GetCommandName(), CmdletName, StringComparison.OrdinalIgnoreCase), true)) { - if (cmdAst.GetCommandName() == null) continue; + var cmdAst = (CommandAst)node; - if (!string.Equals(cmdAst.GetCommandName(), _cmdletName, StringComparison.OrdinalIgnoreCase)) + if (!IsComObject(cmdAst)) { - continue; - } - - if (IsComObject(cmdAst)) - { - continue; - } - - yield return new DiagnosticRecord( - GetError(), + yield return new DiagnosticRecord( + string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectError), cmdAst.Extent, GetName(), - GetDiagnosticSeverity(), + DiagnosticSeverity.Warning, fileName, - cmdAst.GetCommandName()); + CmdletName); + } } } #endregion - #region Private methods - - /// - /// GetError: Retrieves the error message - /// - /// The error message - public string GetError() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectError); - } - - /// - /// GetName: Retrieves the name of this rule. - /// - /// The name of this rule - public string GetName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.NameSpaceFormat, GetSourceName(), Strings.AvoidUsingNewObjectName); - } + #region Private Instance Methods - Core Logic /// - /// GetCommonName: Retrieves the common name of this rule. + /// Determines if a New-Object command creates a COM object. /// - /// The common name of this rule - public string GetCommonName() + /// The New-Object command AST to analyze + /// True if command creates COM object via -ComObject parameter; false otherwise + private bool IsComObject(CommandAst commandAst) { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); - } + // Quick check for positional TypeName (non-splat expressions after New-Object) + if (commandAst.CommandElements.Count >= 2) + { + var firstArg = commandAst.CommandElements[1]; - /// - /// GetDescription: Retrieves the description of this rule. - /// - /// The description of this rule - public string GetDescription() - { - return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); - } + // Non-splat variable provided for first positional parameter. + if (firstArg is VariableExpressionAst varAst && !varAst.Splatted) + { + return false; + } - /// - /// Method: Retrieves the type of the rule: builtin, managed or module. - /// - public SourceType GetSourceType() - { - return SourceType.Builtin; - } + // Non-splat using expression provided for first positional parameter (e.g., $using:var) + if (firstArg is UsingExpressionAst usingAst && !(usingAst.SubExpression is VariableExpressionAst usingVar && usingVar.Splatted)) + { + return false; + } + } - /// - /// GetSeverity:Retrieves the severity of the rule: error, warning of information. - /// - /// The severity of the rule - public RuleSeverity GetSeverity() - { - return RuleSeverity.Warning; + // Parse named parameters with early TypeName exit + return HasComObjectParameter(commandAst); } /// - /// DiagnosticSeverity: Retrieves the severity of the rule of type DiagnosticSeverity: error, warning or information. + /// Parses command parameters to detect COM object usage with minimal allocations. + /// Implements early exit optimization for TypeName parameter detection. /// - /// The diagnostic severity of the rule - public DiagnosticSeverity GetDiagnosticSeverity() + /// The command AST to analyze for parameters + /// True if ComObject parameter is found; false if TypeName parameter is found or neither + private bool HasComObjectParameter(CommandAst commandAst) { - return DiagnosticSeverity.Warning; - } + var waitingForParamValue = false; + var elements = commandAst.CommandElements; + var elementCount = elements.Count; - /// - /// Method: Retrieves the module/assembly name the rule is from. - /// - public string GetSourceName() - { - return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); - } + // Fast path: insufficient elements + if (elementCount <= 1) return false; - /// - /// Determines if the New-Object command is creating a COM object. - /// - /// The CommandAst representing the New-Object command - /// True if the command is creating a COM object, false otherwise - private bool IsComObject(CommandAst cmdAst) - { - foreach (var element in cmdAst.CommandElements) + for (int i = 1; i < elementCount; i++) { - if (element is CommandParameterAst cmdParameterAst) - { - var paramName = cmdParameterAst.ParameterName; - if (string.IsNullOrEmpty(paramName)) - continue; - - if (_comObjectParameterRegex.IsMatch(paramName)) - return true; - - if (_typeNameParameterRegex.IsMatch(paramName)) - return false; + var element = elements[i]; + if (waitingForParamValue) + { + waitingForParamValue = false; continue; } - if (element is VariableExpressionAst splattedVarAst && splattedVarAst.Splatted) - return ProcessVariableExpression(splattedVarAst); - - if (element is ExpandableStringExpressionAst expandableStringAst) - return ProcessExpandableExpression(expandableStringAst); - - if (element is UsingExpressionAst usingExprAst) + switch (element) { - if (!(usingExprAst.SubExpression is VariableExpressionAst usingVarAst)) - continue; - - if (!usingVarAst.Splatted) - continue; - - return ProcessVariableExpression(usingVarAst); + case CommandParameterAst paramAst: + var paramName = paramAst.ParameterName; + + // Early exit: TypeName parameter means not COM + if (TypeNameParameterName.StartsWith(paramName, StringComparison.OrdinalIgnoreCase)) + return false; + + // Found ComObject parameter + if (ComObjectParameterName.StartsWith(paramName, StringComparison.OrdinalIgnoreCase)) + return true; + + waitingForParamValue = true; + break; + + case ExpandableStringExpressionAst expandableAst: + if (ProcessExpandableString(expandableAst, ref waitingForParamValue, out bool result)) + return result; + break; + + case VariableExpressionAst varAst when varAst.Splatted: + if (IsSplattedVariableComObject(varAst)) + return true; + break; + + case UsingExpressionAst usingAst when usingAst.SubExpression is VariableExpressionAst usingVar && usingVar.Splatted: + if (IsSplattedVariableComObject((VariableExpressionAst)usingAst.SubExpression)) + return true; + break; } } @@ -211,284 +174,370 @@ private bool IsComObject(CommandAst cmdAst) } /// - /// Recursively processes variable assignments to trace back to the original hashtable definition. + /// Processes expandable strings with minimal allocations. /// - /// The name of the variable to process - /// Set of already visited variables to prevent infinite recursion - /// True if the variable chain leads to a COM object parameter, false otherwise - private bool ProcessVariable(string variableName, HashSet visitedVariables) + private bool ProcessExpandableString( + ExpandableStringExpressionAst expandableAst, + ref bool waitingForParamValue, + out bool foundResult) { - if (visitedVariables.Contains(variableName)) - return false; - - visitedVariables.Add(variableName); + foundResult = false; + var expandedValues = Helper.Instance.GetStringsFromExpressionAst(expandableAst); - var assignments = FindAssignmentStatements(variableName); - - foreach (var assignment in assignments) + if (expandedValues is IList list && list.Count == 0 && expandableAst.NestedExpressions != null) { - var hashtableAst = GetHashtableFromAssignment(assignment); - if (hashtableAst != null) - return ContainsComObjectKey(hashtableAst); - - var sourceVariable = GetVariableFromAssignment(assignment); - if (sourceVariable != null) + // Defer cache initialization until actually needed + var resolvedText = TryResolveExpandableString(expandableAst); + if (resolvedText != null) { - if (ProcessVariable(sourceVariable, visitedVariables)) - return true; - - continue; + expandedValues = new[] { resolvedText }; } + } - var stringConstant = GetStringConstantFromAssignment(assignment); - if (stringConstant != null) + foreach (var expandedValue in expandedValues) + { + if (expandedValue.Length > 1 && expandedValue[0] == '-') { - if (_comObjectParameterRegex.IsMatch(stringConstant)) - return true; - - if (_typeNameParameterRegex.IsMatch(stringConstant)) - return false; + // Avoid substring allocation by using span slicing + var paramName = GetParameterNameFromExpandedValue(expandedValue); + + if (TypeNameParameterName.StartsWith(paramName, StringComparison.OrdinalIgnoreCase)) + { + foundResult = true; + return true; // TypeName found, not COM + } + + if (ComObjectParameterName.StartsWith(paramName, StringComparison.OrdinalIgnoreCase)) + { + foundResult = true; + return true; // ComObject found + } + + waitingForParamValue = true; } } return false; } + #endregion + + #region Private Instance Methods - Variable Resolution + /// - /// Extracts a string constant from an assignment statement's right-hand side. + /// Lazy resolution of expandable strings - only initialize cache when needed. /// - /// The AssignmentStatementAst to analyze - /// The string constant value if found, null otherwise - private static string GetStringConstantFromAssignment(AssignmentStatementAst assignmentAst) + private string TryResolveExpandableString(ExpandableStringExpressionAst expandableAst) { - var rightHandSide = assignmentAst.Right; + if (expandableAst.NestedExpressions?.Count != 1) + return null; - if (rightHandSide is PipelineAst pipelineAst) - { - if (pipelineAst.PipelineElements.Count != 1) - return null; + var nestedExpr = expandableAst.NestedExpressions[0]; - if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) - return null; + if (!(nestedExpr is VariableExpressionAst varAst)) + return null; - if (!(cmdExpr.Expression is StringConstantExpressionAst stringExpr)) - return null; + // Now we actually need the cache + EnsureAssignmentCacheInitialized(); - return stringExpr.Value; - } + var varName = GetVariableNameWithoutScope(varAst); + var varValues = ResolveVariableValues(varName); - // Handle direct CommandExpressionAst -> StringConstantExpressionAst - if (rightHandSide is CommandExpressionAst commandExpr) + if (varValues is IList list && list.Count > 0) { - if (!(commandExpr.Expression is StringConstantExpressionAst directStringExpr)) - return null; + var resolvedText = expandableAst.Extent.Text; + var userPath = varAst.VariablePath.UserPath; - return directStringExpr.Value; + // Optimize string replacement + var varPattern = string.Concat("${", userPath, "}"); + var index = resolvedText.IndexOf(varPattern, StringComparison.Ordinal); + + if (index >= 0) + { + return string.Concat( + resolvedText.Substring(0, index), + list[0], + resolvedText.Substring(index + varPattern.Length) + ); + } + + // Try without braces + varPattern = "$" + userPath; + index = resolvedText.IndexOf(varPattern, StringComparison.Ordinal); + + if (index >= 0) + { + return string.Concat( + resolvedText.Substring(0, index), + list[0], + resolvedText.Substring(index + varPattern.Length) + ); + } } return null; } /// - /// Processes VariableExpressionAst to determine if the variable potentially matches 'ComObject' parameter. + /// Analyzes splatted variable for COM object parameters by examining hashtable assignments. + /// Supports string literals, expandable strings, and variable key resolution. /// - /// The VariableExpressionAst to process - /// True if the variable contains COM object parameter, false otherwise - private bool ProcessVariableExpression(VariableExpressionAst variableAst) + /// Splatted variable expression to analyze + /// True if hashtable contains ComObject key or abbreviation + private bool IsSplattedVariableComObject(VariableExpressionAst splattedVar) { - var variableName = GetVariableNameWithoutScope(variableAst); - var visitedVariables = new HashSet(StringComparer.OrdinalIgnoreCase); + EnsureAssignmentCacheInitialized(); + + var variableName = GetVariableNameWithoutScope(splattedVar); - return ProcessVariable(variableName, visitedVariables); + return IsSplattedVariableComObjectRecursive(variableName, new HashSet(StringComparer.OrdinalIgnoreCase)); } - /// - /// Attempts to resolve expandable strings to its variable value. - /// - /// The ExpandableStringExpressionAst to resolve - /// The resolved string value, or null if it cannot be determined - private bool ProcessExpandableExpression(ExpandableStringExpressionAst expandableAst) + private bool IsSplattedVariableComObjectRecursive(string variableName, HashSet visited) { - if (expandableAst.NestedExpressions.Count == 0) + if (visited.Contains(variableName)) + { return false; + } - if (!(expandableAst.NestedExpressions[0] is VariableExpressionAst singleVar)) - return false; + visited.Add(variableName); + + var assignments = FindHashtableAssignments(variableName); + + foreach (var assignment in assignments) + { + if (assignment.Right is CommandExpressionAst cmdExpr) + { + if (cmdExpr.Expression is HashtableAst hashtableAst) + { + var hasComKey = HasComObjectKey(hashtableAst); + + if (hasComKey) + { + return true; + } + } + else if (cmdExpr.Expression is VariableExpressionAst referencedVar) + { + // Follow variable chain: $params2 = $script:params + var referencedName = GetVariableNameWithoutScope(referencedVar); + + if (IsSplattedVariableComObjectRecursive(referencedName, visited)) + { + return true; + } + } + } + } - return ProcessVariableExpression(singleVar); + return false; } + #endregion + + #region Private Instance Methods - Cache Management + /// - /// Finds all assignment statements in the current scope that assign to the specified variable. + /// Ensures the assignment cache is initialized before use. /// - /// The name of the variable to search for - /// A list of AssignmentStatementAst objects that assign to the variable - private List FindAssignmentStatements(string variableName) + private void EnsureAssignmentCacheInitialized() { - var assignments = new List(); - var allAssignments = _rootAst.FindAll(ast => ast is AssignmentStatementAst, true); - - foreach (var assignment in allAssignments.Cast()) + if (_assignmentCache == null) { - VariableExpressionAst leftVarAst = GetLeftVariableFromAssignment(assignment); - if (leftVarAst == null) - continue; - - var leftVarName = GetVariableNameWithoutScope(leftVarAst); - if (!string.Equals(leftVarName, variableName, StringComparison.OrdinalIgnoreCase)) - continue; - - assignments.Add(assignment); + _assignmentCache = new Lazy>>(BuildAssignmentCache); } - - return assignments; } /// - /// Checks if a hashtable contains a ComObject key or TypeName key to determine if it's for COM object creation. + /// Builds case-insensitive cache mapping variable names to their assignment statements. + /// Single AST traversal shared across all variable lookups. /// - /// The HashtableAst to examine - /// True if the hashtable contains a ComObject key, false if it contains TypeName or neither - private bool ContainsComObjectKey(HashtableAst hashtableAst) + /// Dictionary of variable assignments indexed by variable name + private Dictionary> BuildAssignmentCache() { - foreach (var keyValuePair in hashtableAst.KeyValuePairs) - { - if (keyValuePair.Item1 is StringConstantExpressionAst stringKey) - { - string paramName = stringKey.Value; - if (string.IsNullOrEmpty(paramName)) - continue; - - if (_comObjectParameterRegex.IsMatch(paramName)) - return true; + var cache = new Dictionary>(StringComparer.OrdinalIgnoreCase); - if (_typeNameParameterRegex.IsMatch(paramName)) - return false; + foreach (var ast in _rootAst.FindAll(ast => ast is AssignmentStatementAst, true)) + { + var assignment = (AssignmentStatementAst)ast; + VariableExpressionAst leftVar = null; - continue; + switch (assignment.Left) + { + case VariableExpressionAst varAst: + leftVar = varAst; + break; + case ConvertExpressionAst convertExpr when convertExpr.Child is VariableExpressionAst convertedVar: + leftVar = convertedVar; + break; } - if (keyValuePair.Item1 is VariableExpressionAst varKey) + if (leftVar != null) { - if (ProcessVariableExpression(varKey)) - return true; + var variableName = GetVariableNameWithoutScope(leftVar); - continue; - } + if (!cache.TryGetValue(variableName, out var list)) + { + list = new List(4); + cache[variableName] = list; + } - if (keyValuePair.Item1 is ExpandableStringExpressionAst expandableKey) - { - if (ProcessExpandableExpression(expandableKey)) - return true; + list.Add(assignment); } } - return false; + return cache; } - #endregion - - #region Static methods - /// - /// Extracts a hashtable from various statement types (PipelineAst, CommandExpressionAst). + /// Retrieves cached assignment statements for the specified variable. /// - /// The StatementAst to analyze - /// The HashtableAst if found, null otherwise - private static HashtableAst ExtractHashtableFromStatement(StatementAst statement) + /// Variable name to look up + /// List of assignment statements or empty list if none found + private List FindHashtableAssignments(string variableName) { - if (statement is PipelineAst pipelineAst) - { - if (pipelineAst.PipelineElements.Count < 1) - return null; - - if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) - return null; + var cache = _assignmentCache.Value; + return cache.TryGetValue(variableName, out var assignments) + ? assignments + : new List(); + } - if (!(cmdExpr.Expression is HashtableAst hashtableFromPipeline)) - return null; + #endregion - return hashtableFromPipeline; - } + #region Private Instance Methods - Hashtable Analysis - if (statement is CommandExpressionAst commandExpr) + /// + /// Checks hashtable for ComObject keys, supporting parameter abbreviation. + /// Handles string literals, expandable strings, and variable keys. + /// + /// Hashtable AST to examine + /// True if any key matches ComObject parameter name or abbreviation + private bool HasComObjectKey(HashtableAst hashtableAst) + { + foreach (var keyValuePair in hashtableAst.KeyValuePairs) { - if (!(commandExpr.Expression is HashtableAst hashtableFromCommand)) - return null; + var keyStrings = GetKeyStrings(keyValuePair.Item1); - return hashtableFromCommand; + foreach (var keyString in keyStrings) + { + // Require minimum 3 characters for COM parameter abbreviation to avoid false positives + if (keyString.Length >= 3) + { + if (ComObjectParameterName.StartsWith(keyString, StringComparison.OrdinalIgnoreCase)) + { + return true; + } + } + } } - return null; + return false; } /// - /// Extracts a hashtable from an assignment statement's right-hand side. + /// Extracts string values from hashtable key expressions. + /// Supports literals, expandable strings, and variable resolution. /// - /// The AssignmentStatementAst to analyze - /// The HashtableAst if found, null otherwise - private static HashtableAst GetHashtableFromAssignment(AssignmentStatementAst assignmentAst) + /// Key expression from hashtable + /// List of resolved string values for the key + private List GetKeyStrings(ExpressionAst keyExpression) { - var rightHandSide = assignmentAst.Right; - - HashtableAst hashtable = ExtractHashtableFromStatement(rightHandSide); + switch (keyExpression) + { + case StringConstantExpressionAst stringAst: + return new List(1) { stringAst.Value }; - if (hashtable != null) - return hashtable; + case ExpandableStringExpressionAst expandableAst: + var expandedStrings = Helper.Instance.GetStringsFromExpressionAst(expandableAst); - var hashtableNodes = rightHandSide.FindAll(ast => ast is HashtableAst, true); - return hashtableNodes.FirstOrDefault() as HashtableAst; - } + if (expandedStrings is IList list && list.Count > 0) + return new List(list); - /// - /// Extracts the left-hand variable from an assignment statement. - /// - /// The assignment statement - /// The variable expression, or null if not found - private static VariableExpressionAst GetLeftVariableFromAssignment(AssignmentStatementAst assignment) - { - if (assignment.Left is VariableExpressionAst leftVar) - return leftVar; + // Fallback for single variable case + if (expandableAst.NestedExpressions?.Count == 1 && + expandableAst.NestedExpressions[0] is VariableExpressionAst varAst) + { + return ResolveVariableValues(varAst.VariablePath.UserPath); + } + break; - if (assignment.Left is ConvertExpressionAst convertAst && - convertAst.Child is VariableExpressionAst typedVar) - return typedVar; + case VariableExpressionAst variableAst: + return ResolveVariableValues(variableAst.VariablePath.UserPath); + } - return null; + return new List(0); } /// - /// Extracts the source variable name from a variable-to-variable assignment. + /// Resolves variable values by analyzing string assignments using cached lookups. /// - /// The AssignmentStatementAst to analyze - /// The name of the source variable, or null if not a variable assignment - private static string GetVariableFromAssignment(AssignmentStatementAst assignmentAst) + /// Variable name to resolve + /// List of possible string values assigned to the variable + private List ResolveVariableValues(string variableName) { - var rightHandSide = assignmentAst.Right; + var values = new List(); - if (rightHandSide is PipelineAst pipelineAst) + // Optimize scope removal + var colonIndex = variableName.IndexOf(':'); + var normalizedName = colonIndex >= 0 + ? TrimQuotes(variableName.Substring(colonIndex + 1)) + : TrimQuotes(variableName); + + if (_assignmentCache.Value.TryGetValue(normalizedName, out var variableAssignments)) { - if (pipelineAst.PipelineElements.Count != 1) - return null; + foreach (var assignment in variableAssignments) + { + if (assignment.Right is CommandExpressionAst cmdExpr) + { + var extractedValues = Helper.Instance.GetStringsFromExpressionAst(cmdExpr.Expression); + + // Avoid AddRange if possible + if (extractedValues is IList list) + { + for (int i = 0; i < list.Count; i++) + values.Add(list[i]); + } + else + { + values.AddRange(extractedValues); + } + } + } + } + + return values; + } - if (!(pipelineAst.PipelineElements[0] is CommandExpressionAst cmdExpr)) - return null; + #endregion - if (!(cmdExpr.Expression is VariableExpressionAst varExpr)) - return null; + #region Private Static Methods - return GetVariableNameWithoutScope(varExpr); - } + /// + /// Extracts parameter name without allocating substrings. + /// + private static string GetParameterNameFromExpandedValue(string expandedValue) + { + // Skip the '-' prefix + var startIndex = 1; + var length = expandedValue.Length - 1; - if (rightHandSide is CommandExpressionAst commandExpr) + // Check for quotes and adjust + if (length >= 2) { - if (!(commandExpr.Expression is VariableExpressionAst directVarExpr)) - return null; + var firstChar = expandedValue[startIndex]; + var lastChar = expandedValue[expandedValue.Length - 1]; - return GetVariableNameWithoutScope(directVarExpr); + if ((firstChar == '"' || firstChar == '\'') && firstChar == lastChar) + { + startIndex++; + length -= 2; + } } - return null; + // Only allocate if we need to trim + return startIndex == 1 && length == expandedValue.Length - 1 + ? expandedValue.Substring(1) + : expandedValue.Substring(startIndex, length); } /// @@ -528,5 +577,68 @@ private static string TrimQuotes(string input) } #endregion + + #region IScriptRule Implementation + + /// + /// Gets the fully qualified name of this rule. + /// + /// Rule name in format "SourceName\RuleName" + public string GetName() + { + return string.Format( + CultureInfo.CurrentCulture, + Strings.NameSpaceFormat, + GetSourceName(), + Strings.AvoidUsingNewObjectName + ); + } + + /// + /// Gets the user-friendly common name of this rule. + /// + /// Localized common name + public string GetCommonName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName); + } + + /// + /// Gets the detailed description of what this rule checks. + /// + /// Localized rule description + public string GetDescription() + { + return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription); + } + + /// + /// Gets the severity level of violations detected by this rule. + /// + /// Warning severity level + public RuleSeverity GetSeverity() + { + return RuleSeverity.Warning; + } + + /// + /// Gets the source name for this rule. + /// + /// PSScriptAnalyzer source name + public string GetSourceName() + { + return string.Format(CultureInfo.CurrentCulture, Strings.SourceName); + } + + /// + /// Gets the source type indicating this is a built-in rule. + /// + /// Builtin source type + public SourceType GetSourceType() + { + return SourceType.Builtin; + } + + #endregion } } \ No newline at end of file