Skip to content

Commit e14adb2

Browse files
iRon7iRon7
committed
Add new AvoidUsingNewObject rule to flag usage of New-Object cmdlet and suggest type initializer as a fix.
1 parent a143b9f commit e14adb2

5 files changed

Lines changed: 793 additions & 0 deletions

File tree

‎Rules/AvoidUsingNewObject.cs‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Globalization;
8+
using System.Management.Automation.Language;
9+
using System.Linq;
10+
11+
#if !CORECLR
12+
using System.ComponentModel.Composition;
13+
#endif
14+
15+
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
16+
{
17+
#if !CORECLR
18+
[Export(typeof(IScriptRule))]
19+
#endif
20+
21+
/// <summary>
22+
/// Rule that reports an warning when the New-Object cmdlet is used in a script.
23+
/// The rule implements a correction that suggests using type-casting or type constructor.
24+
///
25+
/// Note:
26+
/// In case a automatic correction isn't available, the rule won't report any violation either.
27+
/// This is because if there isn't an automatic correction available, it means that the there
28+
/// isn't a simple type-casting or type constructor that can be used as a replacement that
29+
/// would be more efficient or has a better syntax than using New-Object.
30+
/// In other words, if the `-ComObject` parameter is used, or both the parameters
31+
/// `-ArgumentList` and `-Property` are used, there won't be a simple type initializer
32+
/// available and the rule won't report any violation for the `New-Object` cmdlet.
33+
/// </summary>
34+
public class AvoidUsingNewObject : ConfigurableRule
35+
{
36+
37+
/// <summary>
38+
/// Construct an object of AvoidUsingNewObject type.
39+
/// </summary>
40+
public AvoidUsingNewObject() {
41+
Enable = false;
42+
}
43+
44+
/// <summary>
45+
/// Analyzes the given ast to find the [violation]
46+
/// </summary>
47+
/// <param name="ast">AST to be analyzed. This should be non-null</param>
48+
/// <param name="fileName">Name of file that corresponds to the input AST.</param>
49+
/// <returns>A an enumerable type containing the violations</returns>
50+
public override IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
51+
{
52+
if (ast == null) throw new ArgumentNullException(Strings.NullAstErrorMessage);
53+
54+
IEnumerable<CommandAst> newObjectAsts = ast.FindAll(testAst =>
55+
testAst is CommandAst cmdAst &&
56+
cmdAst.GetCommandName() != null &&
57+
cmdAst.GetCommandName().Equals("New-Object", StringComparison.OrdinalIgnoreCase),
58+
true
59+
).Cast<CommandAst>();
60+
61+
foreach (CommandAst cmdAst in newObjectAsts)
62+
{
63+
// Use StaticParameterBinder to reliably get parameter values
64+
var bindingResult = StaticParameterBinder.BindCommand(cmdAst, true);
65+
66+
// Check for `-TypeName` and either a `-ArgumentList` or `-Property`.
67+
// But not both, as that would mean there isn't a simple
68+
// type initializer available as a replacement.
69+
if (
70+
bindingResult.BoundParameters.Count == 2 &&
71+
bindingResult.BoundParameters.TryGetValue("TypeName", out ParameterBindingResult asTypeName) &&
72+
asTypeName.ConstantValue is string typeName
73+
) {
74+
Boolean isProperty = bindingResult.BoundParameters.TryGetValue("Property", out ParameterBindingResult boundResult);
75+
if (!isProperty)
76+
{
77+
Boolean isArgument = bindingResult.BoundParameters.TryGetValue("ArgumentList", out ParameterBindingResult argumentResult);
78+
if (isArgument) { boundResult = argumentResult; } else { continue; }
79+
}
80+
81+
string correction = null;
82+
if (isProperty)
83+
{
84+
correction = "[" + typeName + "]" + boundResult.Value.Extent.Text;
85+
}
86+
else if (boundResult.ConstantValue != null)
87+
{
88+
string valueText = boundResult.Value.Extent.Text;
89+
if (
90+
boundResult.Value is StringConstantExpressionAst stringConstant &&
91+
stringConstant.StringConstantType == StringConstantType.BareWord
92+
)
93+
{
94+
valueText = '"' + valueText.Replace("\"", string.Empty) + '"'; // Test""123 --> "Test123"
95+
}
96+
correction = "[" + typeName + "]" + valueText;
97+
}
98+
else if (
99+
boundResult.Value is ParenExpressionAst parenExpressionAst &&
100+
!parenExpressionAst.Pipeline.Extent.Text.StartsWith(",")
101+
)
102+
{
103+
correction = "[" +typeName + "]::new" + parenExpressionAst.Extent.Text;
104+
}
105+
else if (
106+
boundResult.Value is ArrayExpressionAst arrayExpressionAst &&
107+
!arrayExpressionAst.SubExpression.Extent.Text.StartsWith(",")
108+
)
109+
{
110+
correction = "[" + typeName + "]::new(" + arrayExpressionAst.SubExpression.Extent.Text + ")";
111+
}
112+
else if (
113+
boundResult.Value is SubExpressionAst subExpressionAst &&
114+
!subExpressionAst.SubExpression.Extent.Text.StartsWith(",")
115+
)
116+
{
117+
correction = "[" + typeName + "]::new(" + subExpressionAst.SubExpression.Extent.Text + ")";
118+
}
119+
else if (boundResult.Value is VariableExpressionAst)
120+
{
121+
// correction == $null
122+
123+
// The correction is inconclusive, as we can't be sure if the variable contains
124+
// a single value that can be used in a type initializer,
125+
// or if it contains multiple values that would require splatting,
126+
// which can't be automatically determined from the AST.
127+
}
128+
else
129+
{
130+
correction = "[" + typeName + "]::new(" + boundResult.Value.Extent.Text + ")";
131+
}
132+
133+
DiagnosticRecord diagnosticRecord = new DiagnosticRecord(
134+
string.Format(
135+
CultureInfo.CurrentCulture,
136+
Strings.AvoidUsingNewObjectError,
137+
typeName
138+
),
139+
cmdAst.Extent,
140+
GetName(),
141+
DiagnosticSeverity.Warning,
142+
fileName,
143+
typeName
144+
);
145+
146+
if (correction != null)
147+
{
148+
diagnosticRecord.SuggestedCorrections = new List<CorrectionExtent> {
149+
new CorrectionExtent(
150+
cmdAst.Extent.StartLineNumber,
151+
cmdAst.Extent.EndLineNumber,
152+
cmdAst.Extent.StartColumnNumber,
153+
cmdAst.Extent.EndColumnNumber,
154+
correction,
155+
fileName,
156+
string.Format(
157+
CultureInfo.CurrentCulture,
158+
Strings.AvoidUsingNewObjectCorrectionDescription,
159+
typeName
160+
)
161+
)
162+
};
163+
}
164+
165+
yield return diagnosticRecord;
166+
}
167+
}
168+
}
169+
170+
/// <summary>
171+
/// Retrieves the common name of this rule.
172+
/// </summary>
173+
public override string GetCommonName()
174+
{
175+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectCommonName);
176+
}
177+
178+
/// <summary>
179+
/// Retrieves the description of this rule.
180+
/// </summary>
181+
public override string GetDescription()
182+
{
183+
return string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingNewObjectDescription);
184+
}
185+
186+
/// <summary>
187+
/// Retrieves the name of this rule.
188+
/// </summary>
189+
public override string GetName()
190+
{
191+
return string.Format(
192+
CultureInfo.CurrentCulture,
193+
Strings.NameSpaceFormat,
194+
GetSourceName(),
195+
Strings.AvoidUsingNewObjectName);
196+
}
197+
198+
/// <summary>
199+
/// Retrieves the severity of the rule: error, warning or information.
200+
/// </summary>
201+
public override RuleSeverity GetSeverity()
202+
{
203+
return RuleSeverity.Warning;
204+
}
205+
206+
/// <summary>
207+
/// Gets the severity of the returned diagnostic record: error, warning, or information.
208+
/// </summary>
209+
/// <returns></returns>
210+
public DiagnosticSeverity GetDiagnosticSeverity()
211+
{
212+
return DiagnosticSeverity.Warning;
213+
}
214+
215+
/// <summary>
216+
/// Retrieves the name of the module/assembly the rule is from.
217+
/// </summary>
218+
public override string GetSourceName()
219+
{
220+
return string.Format(CultureInfo.CurrentCulture, Strings.SourceName);
221+
}
222+
223+
/// <summary>
224+
/// Retrieves the type of the rule, Builtin, Managed or Module.
225+
/// </summary>
226+
public override SourceType GetSourceType()
227+
{
228+
return SourceType.Builtin;
229+
}
230+
}
231+
}
232+

‎Rules/Strings.resx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,21 @@
336336
<data name="AvoidUsingConsoleWriteError" xml:space="preserve">
337337
<value>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.</value>
338338
</data>
339+
<data name="AvoidUsingNewObjectCommonName" xml:space="preserve">
340+
<value>Avoid Using New-Object</value>
341+
</data>
342+
<data name="AvoidUsingNewObjectDescription" xml:space="preserve">
343+
<value>Avoid using the 'New-Object' cmdlet to create objects as it might perform poorly.</value>
344+
</data>
345+
<data name="AvoidUsingNewObjectName" xml:space="preserve">
346+
<value>AvoidUsingNewObject</value>
347+
</data>
348+
<data name="AvoidUsingNewObjectError" xml:space="preserve">
349+
<value>Avoid using the 'New-Object' cmdlet to create objects of type '{0}' as it might perform poorly.</value>
350+
</data>
351+
<data name="AvoidUsingNewObjectCorrectionDescription" xml:space="preserve">
352+
<value>Use the type initializer '[{0}]' to construct or cast the intended object.</value>
353+
</data>
339354
<data name="AvoidUsingWriteHostDescription" xml:space="preserve">
340355
<value>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.</value>
341356
</data>

0 commit comments

Comments
 (0)