Skip to content

Commit 95d0894

Browse files
author
Kapil Borle
committed
Add more checks for UseLiteralInitializerForHashtable rule
1 parent a585a55 commit 95d0894

2 files changed

Lines changed: 224 additions & 56 deletions

File tree

‎Engine/Helper.cs‎

Lines changed: 47 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,52 @@ public static bool IsMissingManifestMemberException(ErrorRecord errorRecord)
342342
&& string.Equals("MissingMemberException", errorRecord.CategoryInfo.Reason, StringComparison.OrdinalIgnoreCase);
343343
}
344344

345+
public IEnumerable<string> GetStringsFromExpressionAst(ExpressionAst exprAst)
346+
{
347+
if (exprAst == null)
348+
{
349+
throw new ArgumentNullException("exprAst");
350+
}
351+
352+
var result = new List<string>();
353+
if (exprAst is StringConstantExpressionAst)
354+
{
355+
result.Add((exprAst as StringConstantExpressionAst).Value);
356+
}
357+
// Array of the form "v-n", "v-n1"
358+
else if (exprAst is ArrayLiteralAst)
359+
{
360+
result.AddRange(Helper.Instance.GetStringsFromArrayLiteral(exprAst as ArrayLiteralAst));
361+
}
362+
// Array of the form @("v-n", "v-n1")
363+
else if (exprAst is ArrayExpressionAst)
364+
{
365+
ArrayExpressionAst arrExAst = exprAst as ArrayExpressionAst;
366+
if (arrExAst.SubExpression != null && arrExAst.SubExpression.Statements != null)
367+
{
368+
foreach (StatementAst stAst in arrExAst.SubExpression.Statements)
369+
{
370+
if (stAst is PipelineAst)
371+
{
372+
PipelineAst pipeAst = stAst as PipelineAst;
373+
if (pipeAst.PipelineElements != null)
374+
{
375+
foreach (CommandBaseAst cmdBaseAst in pipeAst.PipelineElements)
376+
{
377+
if (cmdBaseAst is CommandExpressionAst)
378+
{
379+
result.AddRange(Helper.Instance.GetStringsFromArrayLiteral((cmdBaseAst as CommandExpressionAst).Expression as ArrayLiteralAst));
380+
}
381+
}
382+
}
383+
}
384+
}
385+
}
386+
}
387+
388+
return result;
389+
}
390+
345391
/// <summary>
346392
/// Get the list of exported function by analyzing the ast
347393
/// </summary>
@@ -433,41 +479,7 @@ public HashSet<string> GetExportedFunction(Ast ast)
433479

434480
if (exprAst != null)
435481
{
436-
// One string so just add this to the list
437-
if (exprAst is StringConstantExpressionAst)
438-
{
439-
exportedFunctions.Add((exprAst as StringConstantExpressionAst).Value);
440-
}
441-
// Array of the form "v-n", "v-n1"
442-
else if (exprAst is ArrayLiteralAst)
443-
{
444-
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral(exprAst as ArrayLiteralAst));
445-
}
446-
// Array of the form @("v-n", "v-n1")
447-
else if (exprAst is ArrayExpressionAst)
448-
{
449-
ArrayExpressionAst arrExAst = exprAst as ArrayExpressionAst;
450-
if (arrExAst.SubExpression != null && arrExAst.SubExpression.Statements != null)
451-
{
452-
foreach (StatementAst stAst in arrExAst.SubExpression.Statements)
453-
{
454-
if (stAst is PipelineAst)
455-
{
456-
PipelineAst pipeAst = stAst as PipelineAst;
457-
if (pipeAst.PipelineElements != null)
458-
{
459-
foreach (CommandBaseAst cmdBaseAst in pipeAst.PipelineElements)
460-
{
461-
if (cmdBaseAst is CommandExpressionAst)
462-
{
463-
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromArrayLiteral((cmdBaseAst as CommandExpressionAst).Expression as ArrayLiteralAst));
464-
}
465-
}
466-
}
467-
}
468-
}
469-
}
470-
}
482+
exportedFunctions.UnionWith(Helper.Instance.GetStringsFromExpressionAst(exprAst));
471483
}
472484

473485
i += 1;

‎Rules/UseLiteralInitializerForHashtable.cs‎

Lines changed: 177 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@
1212

1313
using System;
1414
using System.Collections.Generic;
15+
using System.Linq;
1516
using System.Management.Automation.Language;
1617
#if !CORECLR
1718
using System.ComponentModel.Composition;
1819
#endif
1920
using System.Globalization;
2021
using Microsoft.Windows.PowerShell.ScriptAnalyzer.Generic;
22+
using System.Collections.Specialized;
23+
using System.Collections.ObjectModel;
2124

2225
namespace Microsoft.Windows.PowerShell.ScriptAnalyzer.BuiltinRules
2326
{
@@ -35,7 +38,6 @@ public UseLiteralInitializerForHashtable()
3538
var presetTypeNames = new string[]
3639
{
3740
"system.collection.hashtable",
38-
"collection.hashtable",
3941
"hashtable"
4042
};
4143
presetTypeNameSet = new HashSet<string>(presetTypeNames, StringComparer.OrdinalIgnoreCase);
@@ -62,7 +64,9 @@ public override AstVisitAction VisitCommand(CommandAst commandAst)
6264
return AstVisitAction.SkipChildren;
6365
}
6466

65-
if (!commandAst.GetCommandName().Equals("new-object", StringComparison.OrdinalIgnoreCase))
67+
var commandName = commandAst.GetCommandName();
68+
if (commandName == null
69+
|| !commandName.Equals("new-object", StringComparison.OrdinalIgnoreCase))
6670
{
6771
return AstVisitAction.Continue;
6872
}
@@ -72,23 +76,133 @@ public override AstVisitAction VisitCommand(CommandAst commandAst)
7276

7377
private void AnalyzeNewObjectCommand(CommandAst commandAst)
7478
{
75-
//new-object hashtable
76-
var typeNameAst = commandAst.CommandElements[1] as StringConstantExpressionAst;
77-
if (typeNameAst != null)
79+
string typeName;
80+
List<string> argumentList;
81+
GetParametersFromCommandAst(commandAst, out typeName, out argumentList);
82+
if (typeName == null
83+
|| !presetTypeNameSet.Contains(typeName))
7884
{
79-
if (presetTypeNameSet.Contains(typeNameAst.Value))
85+
return;
86+
}
87+
88+
if (argumentList != null)
89+
{
90+
if (argumentList.Any(arg => arg != null && arg.EndsWith("ignorecase", StringComparison.OrdinalIgnoreCase)))
91+
{
92+
return;
93+
}
94+
}
95+
var dr = new DiagnosticRecord(
96+
Strings.UseLiteralInitilializerForHashtableDescription,
97+
commandAst.Extent,
98+
GetName(),
99+
GetDiagnosticSeverity(),
100+
fileName,
101+
ruleId: null,
102+
suggestedCorrections: GetSuggestedCorrections(commandAst, this.fileName));
103+
diagnosticRecords.Add(dr);
104+
}
105+
106+
private void GetParametersFromCommandAst(CommandAst commandAst, out string typeName, out List<string> argumentList)
107+
{
108+
// This should read the command in all the following form
109+
// new-object hashtable
110+
// new-object -Typename hashtable
111+
// new-object hashtable -ArgumentList comparer
112+
// new-object -Typename hashtable -ArgumentList blah1,blah2
113+
// new-object -ArgumentList blah1,blah2 -typename hashtable
114+
115+
argumentList = null;
116+
typeName = null;
117+
var namedArguments = new OrderedDictionary(StringComparer.OrdinalIgnoreCase);
118+
namedArguments.Add("typename", null);
119+
namedArguments.Add("argumentlist", null);
120+
var positinalElems = GetNamedArguments(commandAst.CommandElements, ref namedArguments);
121+
GetPositionalArguments(new ReadOnlyCollection<CommandElementAst> (positinalElems), ref namedArguments);
122+
if (namedArguments["typename"] == null)
123+
{
124+
return;
125+
}
126+
127+
var typenameAst = namedArguments["typename"] as StringConstantExpressionAst;
128+
if (typenameAst == null)
129+
{
130+
return;
131+
}
132+
133+
typeName = typenameAst.Value;
134+
var argumentListAst = namedArguments["argumentlist"] as ExpressionAst;
135+
if (argumentListAst == null)
136+
{
137+
return;
138+
}
139+
140+
argumentList = new List<string>(Helper.Instance.GetStringsFromExpressionAst(argumentListAst));
141+
}
142+
143+
private int GetFirstEmptyIndex(OrderedDictionary namedArguments)
144+
{
145+
for (int k = 0; k < namedArguments.Count; k++)
146+
{
147+
if (namedArguments[k] == null)
80148
{
81-
var dr = new DiagnosticRecord(
82-
Strings.UseLiteralInitilializerForHashtableDescription,
83-
commandAst.Extent,
84-
GetName(),
85-
GetDiagnosticSeverity(),
86-
fileName,
87-
ruleId: null,
88-
suggestedCorrections: null);
89-
diagnosticRecords.Add(dr);
149+
return k;
90150
}
91151
}
152+
return -1;
153+
}
154+
155+
private void GetPositionalArguments(ReadOnlyCollection<CommandElementAst> positinalArguments, ref OrderedDictionary namedArguments)
156+
{
157+
for (int k = 0; k < positinalArguments.Count; k++)
158+
{
159+
int firstEmptyIndex = GetFirstEmptyIndex(namedArguments);
160+
if (firstEmptyIndex == -1)
161+
{
162+
return;
163+
}
164+
var elem = positinalArguments[k];
165+
namedArguments[firstEmptyIndex] = elem as ExpressionAst;
166+
}
167+
}
168+
169+
private List<CommandElementAst> GetNamedArguments(ReadOnlyCollection<CommandElementAst> commandElements, ref OrderedDictionary namedArguments)
170+
{
171+
bool paramFound = false;
172+
string paramName = null;
173+
var remainingCommandElements = new List<CommandElementAst>();
174+
for (int k = 1; k < commandElements.Count; k++)
175+
{
176+
if (paramFound)
177+
{
178+
paramFound = false;
179+
var argAst = commandElements[k] as ExpressionAst;
180+
if (argAst != null)
181+
{
182+
namedArguments[paramName] = argAst;
183+
continue;
184+
}
185+
}
186+
var paramAst = commandElements[k] as CommandParameterAst;
187+
if (paramAst != null)
188+
{
189+
foreach (var key in namedArguments.Keys)
190+
{
191+
var keyStr = key as string;
192+
if (keyStr.Equals(paramAst.ParameterName, StringComparison.OrdinalIgnoreCase))
193+
{
194+
paramFound = true;
195+
paramName = paramAst.ParameterName;
196+
break;
197+
}
198+
}
199+
}
200+
else
201+
{
202+
remainingCommandElements.Add(commandElements[k]);
203+
}
204+
}
205+
return remainingCommandElements;
92206
}
93207

94208
public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressionAst methodCallAst)
@@ -112,8 +226,10 @@ public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressio
112226
return AstVisitAction.Continue;
113227
}
114228

115-
// no arguments provided to new
116-
if (methodCallAst.Arguments == null)
229+
// no arguments provided to new OR one of the argument ends with ignorecase
230+
// (heuristics find to something like [system.stringcomparer]::ordinalignorecase)
231+
if (methodCallAst.Arguments == null
232+
|| !HasIgnoreCaseComparerArg(methodCallAst.Arguments))
117233
{
118234

119235
var dr = new DiagnosticRecord(
@@ -123,26 +239,66 @@ public override AstVisitAction VisitInvokeMemberExpression(InvokeMemberExpressio
123239
GetDiagnosticSeverity(),
124240
fileName,
125241
ruleId: null,
126-
suggestedCorrections: null);
242+
suggestedCorrections: GetSuggestedCorrections(methodCallAst, this.fileName));
127243
diagnosticRecords.Add(dr);
128244
}
129245

130246
return AstVisitAction.Continue;
131247
}
132248

249+
private bool HasIgnoreCaseComparerArg(ReadOnlyCollection<ExpressionAst> arguments)
250+
{
251+
foreach (var arg in arguments)
252+
{
253+
var memberExprAst = arg as MemberExpressionAst;
254+
if (memberExprAst == null)
255+
{
256+
continue;
257+
}
258+
var strConstExprAst = memberExprAst.Member as StringConstantExpressionAst;
259+
if (strConstExprAst == null)
260+
{
261+
continue;
262+
}
263+
if (strConstExprAst.Value.EndsWith("ignorecase"))
264+
{
265+
return true;
266+
}
267+
}
268+
return false;
269+
}
270+
271+
private List<CorrectionExtent> GetSuggestedCorrections(Ast violation, string filename)
272+
{
273+
var correctionExtents = new List<CorrectionExtent>();
274+
correctionExtents.Add(new CorrectionExtent(
275+
violation.Extent.StartLineNumber,
276+
violation.Extent.EndLineNumber,
277+
violation.Extent.StartColumnNumber,
278+
violation.Extent.EndColumnNumber,
279+
"@{}",
280+
filename,
281+
GetDescription()));
282+
return correctionExtents;
283+
}
284+
133285
public string GetCommonName()
134286
{
135-
return Strings.UseLiteralInitilializerForHashtableCommonName;
287+
return string.Format(CultureInfo.CurrentCulture, Strings.UseLiteralInitilializerForHashtableCommonName);
136288
}
137289

138290
public string GetDescription()
139291
{
140-
return Strings.UseLiteralInitilializerForHashtableDescription;
292+
return string.Format(CultureInfo.CurrentCulture, Strings.UseLiteralInitilializerForHashtableDescription);
141293
}
142294

143295
public string GetName()
144296
{
145-
return Strings.UseLiteralInitilializerForHashtableName;
297+
return string.Format(
298+
CultureInfo.CurrentCulture,
299+
Strings.NameSpaceFormat,
300+
GetSourceName(),
301+
Strings.UseLiteralInitilializerForHashtableName);
146302
}
147303

148304
public RuleSeverity GetSeverity()

0 commit comments

Comments
 (0)