Skip to content

Commit 68caca9

Browse files
author
quoctruong
committed
Merge pull request #45 from PowerShell/suppression
Fix bugs from bugbash
2 parents 228b727 + cd8176e commit 68caca9

11 files changed

+132
-112
lines changed

‎Engine/Generic/AvoidParameterGeneric.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3535
{
3636
if (ParameterCondition(cmdAst, ceAst))
3737
{
38-
yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
38+
yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName());
3939
}
4040
}
4141
}

‎Engine/Generic/RuleSuppression.cs

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,15 @@ public class RuleSuppression
2626
{
2727
private string _ruleName;
2828

29+
/// <summary>
30+
/// The start offset of the rule suppression attribute (not where it starts to apply)
31+
/// </summary>
32+
public int StartAttributeLine
33+
{
34+
get;
35+
set;
36+
}
37+
2938
/// <summary>
3039
/// The start offset of the rule suppression
3140
/// </summary>
@@ -57,7 +66,9 @@ public string RuleName
5766
set
5867
{
5968
_ruleName = value;
60-
if ((ScriptAnalyzer.Instance.ScriptRules != null
69+
70+
if (!String.IsNullOrWhiteSpace(_ruleName)
71+
&& (ScriptAnalyzer.Instance.ScriptRules != null
6172
&& ScriptAnalyzer.Instance.ScriptRules.Count(item => String.Equals(item.GetName(), _ruleName, StringComparison.OrdinalIgnoreCase)) == 0)
6273
&& (ScriptAnalyzer.Instance.TokenRules != null
6374
&& ScriptAnalyzer.Instance.TokenRules.Count(item => String.Equals(item.GetName(), _ruleName, StringComparison.OrdinalIgnoreCase)) == 0)
@@ -131,6 +142,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
131142

132143
if (attrAst != null)
133144
{
145+
StartAttributeLine = attrAst.Extent.StartLineNumber;
134146
var positionalArguments = attrAst.PositionalArguments;
135147
var namedArguments = attrAst.NamedArguments;
136148

@@ -236,6 +248,18 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
236248
}
237249
}
238250

251+
if (!String.IsNullOrWhiteSpace(Error))
252+
{
253+
// May be cases where the rulename is null because we didn't look at the rulename after
254+
// we found out there is an error
255+
RuleName = String.Empty;
256+
}
257+
else if (String.IsNullOrWhiteSpace(RuleName))
258+
{
259+
RuleName = String.Empty;
260+
Error = Strings.NullRuleNameError;
261+
}
262+
239263
// Must have scope and target together
240264
if (String.IsNullOrWhiteSpace(Scope) && !String.IsNullOrWhiteSpace(Target))
241265
{
@@ -248,24 +272,26 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
248272

249273
if (!String.IsNullOrWhiteSpace(Error))
250274
{
251-
Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, attrAst.Extent.StartLineNumber,
275+
Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, StartAttributeLine,
252276
System.IO.Path.GetFileName(attrAst.Extent.File), Error);
253277
}
254278
}
255279

256280
/// <summary>
257-
/// Constructs rule expression from rule name, id, start and end
281+
/// Constructs rule expression from rule name, id, start, end and startAttributeLine
258282
/// </summary>
259283
/// <param name="ruleName"></param>
260284
/// <param name="ruleSuppressionID"></param>
261285
/// <param name="start"></param>
262286
/// <param name="end"></param>
263-
public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end)
287+
/// <param name="startAttributeLine"></param>
288+
public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end, int startAttributeLine)
264289
{
265290
RuleName = ruleName;
266291
RuleSuppressionID = ruleSuppressionID;
267292
StartOffset = start;
268293
EndOffset = end;
294+
StartAttributeLine = startAttributeLine;
269295
}
270296

271297
/// <summary>
@@ -320,16 +346,24 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at
320346

321347
if (targetAsts != null)
322348
{
349+
if (targetAsts.Count() == 0)
350+
{
351+
ruleSupp.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSupp.StartAttributeLine,
352+
System.IO.Path.GetFileName(scopeAst.Extent.File), String.Format(Strings.TargetCannotBeFoundError, ruleSupp.Target, ruleSupp.Scope));
353+
result.Add(ruleSupp);
354+
continue;
355+
}
356+
323357
foreach (Ast targetAst in targetAsts)
324358
{
325-
result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset, targetAst.Extent.EndOffset));
359+
result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset, targetAst.Extent.EndOffset, attributeAst.Extent.StartLineNumber));
326360
}
327361
}
328362

329363
}
330364
else
331365
{
332-
// this may add rule suppression that contains erro but we will check for this in the engine to throw out error
366+
// this may add rule suppression that contains error but we will check for this in the engine to throw out error
333367
result.Add(ruleSupp);
334368
}
335369
}

‎Engine/Helper.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Management.Automation;
66
using System.Management.Automation.Language;
77
using System.Management.Automation.Runspaces;
8+
using System.Globalization;
89
using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
910

1011
namespace Microsoft.Windows.Powershell.ScriptAnalyzer
@@ -740,9 +741,25 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
740741
int ruleSuppressionIndex = 0;
741742
DiagnosticRecord record = diagnostics.First();
742743
RuleSuppression ruleSuppression = ruleSuppressions.First();
744+
int suppressionCount = 0;
743745

744746
while (recordIndex < diagnostics.Count)
745747
{
748+
if (!String.IsNullOrWhiteSpace(ruleSuppression.Error))
749+
{
750+
ruleSuppressionIndex += 1;
751+
752+
if (ruleSuppressionIndex == ruleSuppressions.Count)
753+
{
754+
break;
755+
}
756+
757+
ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
758+
suppressionCount = 0;
759+
760+
continue;
761+
}
762+
746763
// the record precedes the rule suppression so don't apply the suppression
747764
if (record.Extent.StartOffset < ruleSuppression.StartOffset)
748765
{
@@ -753,12 +770,21 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
753770
{
754771
ruleSuppressionIndex += 1;
755772

773+
// If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly
774+
if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0)
775+
{
776+
ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSuppression.StartAttributeLine,
777+
System.IO.Path.GetFileName(record.Extent.File), String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID));
778+
Helper.Instance.MyCmdlet.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
779+
}
780+
756781
if (ruleSuppressionIndex == ruleSuppressions.Count)
757782
{
758783
break;
759784
}
760785

761786
ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
787+
suppressionCount = 0;
762788

763789
continue;
764790
}
@@ -771,21 +797,38 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
771797
&& !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))
772798
{
773799
results.Add(record);
800+
suppressionCount -= 1;
774801
}
775802
// otherwise, we ignore the record, move on to the next.
776803
}
777804

778805
// important assumption: this point is reached only if we want to move to the next record
779806
recordIndex += 1;
807+
suppressionCount += 1;
780808

781809
if (recordIndex == diagnostics.Count)
782810
{
811+
// If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly
812+
if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0)
813+
{
814+
ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSuppression.StartAttributeLine,
815+
System.IO.Path.GetFileName(record.Extent.File), String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID));
816+
Helper.Instance.MyCmdlet.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
817+
}
818+
783819
break;
784820
}
785821

786822
record = diagnostics[recordIndex];
787823
}
788824

825+
// Add all unprocessed records to results
826+
while (recordIndex < diagnostics.Count)
827+
{
828+
results.Add(diagnostics[recordIndex]);
829+
recordIndex += 1;
830+
}
831+
789832
return results;
790833
}
791834

‎Engine/Strings.Designer.cs

Lines changed: 28 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Engine/Strings.resx

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,4 +183,13 @@
183183
<data name="WrongScopeArgumentSuppressionAttributeError" xml:space="preserve">
184184
<value>Scope can only be either function or class.</value>
185185
</data>
186+
<data name="NullRuleNameError" xml:space="preserve">
187+
<value>RuleName must not be null.</value>
188+
</data>
189+
<data name="TargetCannotBeFoundError" xml:space="preserve">
190+
<value>Cannot find any Targets {0} that match the Scope {1} to apply the SuppressMessageAttribute.</value>
191+
</data>
192+
<data name="RuleSuppressionIDError" xml:space="preserve">
193+
<value>Cannot find any DiagnosticRecord with the Rule Suppression ID {0}.</value>
194+
</data>
186195
</root>

‎Rules/AvoidAlias.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,17 +35,18 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
3535
foreach (Ast foundAst in foundAsts)
3636
{
3737
CommandAst cmdAst = (CommandAst)foundAst;
38+
string aliasName = cmdAst.GetCommandName();
3839
// Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}.
3940
// You can also review the remark section in following document,
4041
// MSDN: CommandAst.GetCommandName Method
41-
if (cmdAst.GetCommandName() == null) continue;
42+
if (aliasName == null) continue;
4243

43-
string cmdletName = Microsoft.Windows.Powershell.ScriptAnalyzer.Helper.Instance.GetCmdletNameFromAlias(cmdAst.GetCommandName());
44+
string cmdletName = Microsoft.Windows.Powershell.ScriptAnalyzer.Helper.Instance.GetCmdletNameFromAlias(aliasName);
4445

4546
if (!String.IsNullOrEmpty(cmdletName))
4647
{
47-
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, cmdAst.GetCommandName(), cmdletName),
48-
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
48+
yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, aliasName, cmdletName),
49+
cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, aliasName);
4950
}
5051
}
5152
}

‎Rules/AvoidGlobalVars.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
4040
yield return
4141
new DiagnosticRecord(
4242
string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalVarsError,
43-
varAst.VariablePath.UserPath), varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
43+
varAst.VariablePath.UserPath), varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, varAst.VariablePath.UserPath);
4444
}
4545
}
4646
}

0 commit comments

Comments
 (0)