Skip to content

Commit d9e0e6b

Browse files
committed
Implemented more CoPilot suggestions
1 parent 0aad726 commit d9e0e6b

3 files changed

Lines changed: 28 additions & 10 deletions

File tree

Rules/AvoidUsingNewObject.cs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
1919
#endif
2020

2121
/// <summary>
22-
/// Rule that reports an warning when the New-Object cmdlet is used in a script.
22+
/// Rule that reports a warning when the New-Object cmdlet is used in a script.
2323
/// The rule implements a correction that suggests using type-casting or type constructor.
2424
///
2525
/// Note:
26-
/// In most cases if there isn't an automatic correction isn't available,
26+
/// In most cases if there isn't an automatic correction available,
2727
/// the rule won't report any violation either.
2828
/// This is because if there isn't an automatic correction available, it generally means
2929
/// that there isn't a simple type-casting or type constructor that can be used that would
@@ -63,8 +63,8 @@ public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string file
6363

6464
IEnumerable<CommandAst> newObjectAsts = ast.FindAll(testAst =>
6565
testAst is CommandAst cmdAst &&
66-
cmdAst.GetCommandName() != null &&
67-
cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase),
66+
(cmdAst.GetCommandName() as string) is string commandName &&
67+
commandName.Equals("New-Object", StringComparison.OrdinalIgnoreCase),
6868
true
6969
).Cast<CommandAst>();
7070

@@ -77,19 +77,24 @@ testAst is CommandAst cmdAst &&
7777
// But not both, as that would mean there isn't a simple
7878
// type initializer available as a replacement.
7979
if (
80-
bindingResult.BoundParameters.Count == 2 &&
80+
bindingResult.BoundParameters.Count <= 2 &&
8181
bindingResult.BoundParameters.TryGetValue("TypeName", out ParameterBindingResult asTypeName) &&
8282
asTypeName.ConstantValue is string typeName
8383
) {
8484
Boolean isProperty = bindingResult.BoundParameters.TryGetValue("Property", out ParameterBindingResult boundResult);
8585
if (!isProperty)
8686
{
8787
Boolean isArgument = bindingResult.BoundParameters.TryGetValue("ArgumentList", out ParameterBindingResult argumentResult);
88-
if (isArgument) { boundResult = argumentResult; } else { continue; }
88+
if (isArgument) { boundResult = argumentResult; }
8989
}
9090

9191
string correction = null;
92-
if (isProperty)
92+
if (boundResult == null)
93+
{
94+
// No `-Property` or `-ArgumentList` parameter was used, so we suggest a parameterless constructor call.
95+
correction = "[" + typeName + "]::new()";
96+
}
97+
else if (isProperty)
9398
{
9499
correction = "[" + typeName + "]" + boundResult.Value.Extent.Text;
95100
}

Tests/Rules/AvoidUsingNewObject.tests.ps1

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,19 @@ Describe "AvoidUsingNewObject" {
144144

145145
Context 'Construct' {
146146

147+
It 'Empty string' {
148+
$scriptDefinition = { $String = New-Object String }.ToString()
149+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
150+
$violations.Count | Should -Be 1
151+
$violations.Severity | Should -Be Warning
152+
$violations.Extent.Text | Should -Be 'New-Object String'
153+
$violations.Message | Should -Be ($ruleMessage -f 'String')
154+
$violations.RuleSuppressionID | Should -Be 'String'
155+
$violations.SuggestedCorrections.Text | Should -Be '[String]::new()'
156+
$violations.SuggestedCorrections.Text | Should -Parse
157+
$violations.SuggestedCorrections.Description | Should -Be ($correctionDescription -f 'String')
158+
}
159+
147160
It 'String ArgumentList ($Test)' {
148161
$scriptDefinition = { $String = New-Object -TypeName String -ArgumentList ($Test) }.ToString()
149162
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
@@ -202,7 +215,7 @@ Describe "AvoidUsingNewObject" {
202215
$scriptDefinition = {
203216
Write-Host New-Object String 123
204217
}.ToString()
205-
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
218+
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
206219
$violations | Should -BeNullOrEmpty
207220
}
208221

@@ -267,7 +280,7 @@ Describe "AvoidUsingNewObject" {
267280
$violations.SuggestedCorrections.Text | Should -Parse
268281
}
269282

270-
It 'Double quoted string argument' {
283+
It 'Double quoted string argument' {
271284
$scriptDefinition = { New-Object MyClass "Test" }.ToString()
272285
$violations = Invoke-ScriptAnalyzer -ScriptDefinition $scriptDefinition -Settings $Settings
273286
$violations.SuggestedCorrections.Text | Should -Be '[MyClass]"Test"'

docs/Rules/AvoidUsingNewObject.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ title: AvoidUsingNewObject
1515
## Description
1616

1717
Avoid using the `New-Object` cmdlet to create objects as it might perform poorly.
18-
Instead, use type initializer to construct or cast the intended object.
18+
Instead, use a type initializer to construct or cast the intended object.
1919

2020
## Example
2121

0 commit comments

Comments
 (0)