Skip to content

Add -SuppressedOnly switch to ScriptAnalyzer #56

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 22, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 36 additions & 6 deletions Engine/Commands/InvokeScriptAnalyzerCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,17 @@ public SwitchParameter Recurse
}
private bool recurse;

/// <summary>
/// ShowSuppressed: Show the suppressed message
/// </summary>
[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
public SwitchParameter SuppressedOnly
{
get { return suppressedOnly; }
set { suppressedOnly = value; }
}
private bool suppressedOnly;

#endregion Parameters

#region Private Members
Expand Down Expand Up @@ -256,6 +267,7 @@ private void AnalyzeFile(string filePath)
Token[] tokens = null;
ParseError[] errors = null;
List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
List<SuppressedRecord> suppressed = new List<SuppressedRecord>();

// Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash
List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();
Expand Down Expand Up @@ -328,7 +340,9 @@ private void AnalyzeFile(string filePath)
// We want the Engine to continue functioning even if one or more Rules throws an exception
try
{
diagnostics.AddRange(Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList()));
var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception scriptRuleException)
{
Expand Down Expand Up @@ -356,7 +370,9 @@ private void AnalyzeFile(string filePath)
// We want the Engine to continue functioning even if one or more Rules throws an exception
try
{
diagnostics.AddRange(Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, fileName).ToList()));
var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception tokenRuleException)
{
Expand Down Expand Up @@ -384,7 +400,9 @@ private void AnalyzeFile(string filePath)
// We want the Engine to continue functioning even if one or more Rules throws an exception
try
{
diagnostics.AddRange(Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList()));
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception dscResourceRuleException)
{
Expand Down Expand Up @@ -426,7 +444,9 @@ private void AnalyzeFile(string filePath)
// We want the Engine to continue functioning even if one or more Rules throws an exception
try
{
diagnostics.AddRange(Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList()));
var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
diagnostics.AddRange(records.Item2);
suppressed.AddRange(records.Item1);
}
catch (Exception dscResourceRuleException)
{
Expand Down Expand Up @@ -484,9 +504,19 @@ private void AnalyzeFile(string filePath)
//Output through loggers
foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers)
{
foreach (DiagnosticRecord diagnostic in diagnostics)
if (SuppressedOnly)
{
logger.LogMessage(diagnostic, this);
foreach (DiagnosticRecord suppressRecord in suppressed)
{
logger.LogObject(suppressRecord, this);
}
}
else
{
foreach (DiagnosticRecord diagnostic in diagnostics)
{
logger.LogObject(diagnostic, this);
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions Engine/Generic/ILogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ public interface ILogger
/// <summary>
/// LogMessage: Logs the given diagnostic, using the command for Write methods if needed.
/// </summary>
/// <param name="diagnostic">The DiagnosticRecord to be logged.</param>
/// <param name="obj">The object to be logged.</param>
/// <param name="command">The InvokePSScriptAnalyzerCommand that this logger is running off of.</param>
void LogMessage(DiagnosticRecord diagnostic, InvokeScriptAnalyzerCommand command);
void LogObject(Object obj, InvokeScriptAnalyzerCommand command);

/// <summary>
/// GetName: Retrieves the name of the logger.
Expand Down
33 changes: 29 additions & 4 deletions Engine/Generic/RuleSuppression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@ public string Error
set;
}

/// <summary>
/// Returns the justification for the suppression
/// </summary>
public string Justification
{
get;
set;
}

private static HashSet<string> scopeSet;

/// <summary>
Expand Down Expand Up @@ -161,6 +170,10 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
{
switch (count)
{
case 5:
Justification = (positionalArguments[4] as StringConstantExpressionAst).Value;
goto case 4;

case 4:
Target = (positionalArguments[3] as StringConstantExpressionAst).Value;
goto case 3;
Expand Down Expand Up @@ -242,6 +255,15 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
Target = (name.Argument as StringConstantExpressionAst).Value;
goto default;

case "justification":
if (!String.IsNullOrWhiteSpace(Justification))
{
Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name);
}

Justification = (name.Argument as StringConstantExpressionAst).Value;
goto default;

default:
break;
}
Expand Down Expand Up @@ -278,20 +300,22 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
}

/// <summary>
/// Constructs rule expression from rule name, id, start, end and startAttributeLine
/// Constructs rule expression from rule name, id, start, end, startAttributeLine and justification
/// </summary>
/// <param name="ruleName"></param>
/// <param name="ruleSuppressionID"></param>
/// <param name="start"></param>
/// <param name="end"></param>
/// <param name="startAttributeLine"></param>
public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end, int startAttributeLine)
/// <param name="justification"></param>
public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end, int startAttributeLine, string justification)
{
RuleName = ruleName;
RuleSuppressionID = ruleSuppressionID;
StartOffset = start;
EndOffset = end;
StartAttributeLine = startAttributeLine;
Justification = justification;
}

/// <summary>
Expand Down Expand Up @@ -327,7 +351,7 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at
}

// regex for wild card *
Regex reg = new Regex(String.Format("^{0}$", Regex.Escape(ruleSupp.Target).Replace(@"\*", ".*")));
Regex reg = new Regex(String.Format("^{0}$", Regex.Escape(ruleSupp.Target).Replace(@"\*", ".*")), RegexOptions.IgnoreCase);
IEnumerable<Ast> targetAsts = null;

switch (ruleSupp.Scope.ToLower())
Expand Down Expand Up @@ -356,7 +380,8 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at

foreach (Ast targetAst in targetAsts)
{
result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset, targetAst.Extent.EndOffset, attributeAst.Extent.StartLineNumber));
result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset,
targetAst.Extent.EndOffset, attributeAst.Extent.StartLineNumber, ruleSupp.Justification));
}
}

Expand Down
42 changes: 42 additions & 0 deletions Engine/Generic/SuppressedRecord.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Microsoft.Windows.Powershell.ScriptAnalyzer.Generic
{
/// <summary>
/// Represents a suppressed diagnostic record
/// </summary>
public class SuppressedRecord : DiagnosticRecord
{
/// <summary>
/// The rule suppression of this record
/// </summary>
public RuleSuppression Suppression
{
get;
set;
}

/// <summary>
/// Creates a suppressed record based on a diagnostic record and the rule suppression
/// </summary>
/// <param name="record"></param>
/// <param name="Suppression"></param>
public SuppressedRecord(DiagnosticRecord record, RuleSuppression suppression)
{
Suppression = suppression;
if (record != null)
{
RuleName = record.RuleName;
Message = record.Message;
Extent = record.Extent;
Severity = record.Severity;
ScriptName = record.ScriptName;
RuleSuppressionID = record.RuleSuppressionID;
}
}
}
}
99 changes: 50 additions & 49 deletions Engine/Helper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -716,25 +716,28 @@ internal List<RuleSuppression> GetSuppressionsClass(TypeDefinitionAst typeAst)
}

/// <summary>
/// Suppress the rules from the diagnostic records list and return the result
/// Suppress the rules from the diagnostic records list.
/// Returns a list of suppressed records as well as the ones that are not suppressed
/// </summary>
/// <param name="ruleSuppressions"></param>
/// <param name="diagnostics"></param>
public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)
public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)
{
List<DiagnosticRecord> results = new List<DiagnosticRecord>();
List<SuppressedRecord> suppressedRecords = new List<SuppressedRecord>();
List<DiagnosticRecord> unSuppressedRecords = new List<DiagnosticRecord>();
Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> result = Tuple.Create(suppressedRecords, unSuppressedRecords);

if (ruleSuppressionsDict == null || !ruleSuppressionsDict.ContainsKey(ruleName)
|| diagnostics == null || diagnostics.Count == 0)
{
return diagnostics;
return result;
}

List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];

if (ruleSuppressions.Count == 0)
{
return diagnostics;
return result;
}

int recordIndex = 0;
Expand All @@ -760,46 +763,54 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
continue;
}

// the record precedes the rule suppression so don't apply the suppression
if (record.Extent.StartOffset < ruleSuppression.StartOffset)
{
results.Add(record);
}
// end of the rule suppression is less than the record start offset so move on to next rule suppression
else if (ruleSuppression.EndOffset < record.Extent.StartOffset)
// if the record precedes the rule suppression then we don't apply the suppression
// so we check that start of record is greater than start of suppression
if (record.Extent.StartOffset >= ruleSuppression.StartOffset)
{
ruleSuppressionIndex += 1;

// If we cannot found any error but the rulesuppression has a rulesuppressionid then it must be used wrongly
if (!String.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && suppressionCount == 0)
// end of the rule suppression is less than the record start offset so move on to next rule suppression
if (ruleSuppression.EndOffset < record.Extent.StartOffset)
{
ruleSuppression.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSuppression.StartAttributeLine,
System.IO.Path.GetFileName(record.Extent.File), String.Format(Strings.RuleSuppressionIDError, ruleSuppression.RuleSuppressionID));
Helper.Instance.MyCmdlet.WriteError(new ErrorRecord(new ArgumentException(ruleSuppression.Error), ruleSuppression.Error, ErrorCategory.InvalidArgument, ruleSuppression));
}
ruleSuppressionIndex += 1;

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

ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
suppressionCount = 0;
if (ruleSuppressionIndex == ruleSuppressions.Count)
{
break;
}

continue;
ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
suppressionCount = 0;

continue;
}
// at this point, the record is inside the interval
else
{
// if the rule suppression id from the rule suppression is not null and the one from diagnostic record is not null
// and they are they are not the same then we cannot ignore the record
if (!string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && !string.IsNullOrWhiteSpace(record.RuleSuppressionID)
&& !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))
{
suppressionCount -= 1;
unSuppressedRecords.Add(record);
}
// otherwise, we suppress the record, move on to the next.
else
{
suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression));
}
}
}
// at this point, the record is inside the interval
else
{
// if the rule suppression id from the rule suppression is not null and the one from diagnostic record is not null
// and they are they are not the same then we cannot ignore the record
if (!string.IsNullOrWhiteSpace(ruleSuppression.RuleSuppressionID) && !string.IsNullOrWhiteSpace(record.RuleSuppressionID)
&& !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))
{
results.Add(record);
suppressionCount -= 1;
}
// otherwise, we ignore the record, move on to the next.
unSuppressedRecords.Add(record);
}

// important assumption: this point is reached only if we want to move to the next record
Expand All @@ -822,14 +833,13 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
record = diagnostics[recordIndex];
}

// Add all unprocessed records to results
while (recordIndex < diagnostics.Count)
{
results.Add(diagnostics[recordIndex]);
unSuppressedRecords.Add(diagnostics[recordIndex]);
recordIndex += 1;
}

return results;
return result;
}

#endregion
Expand Down Expand Up @@ -918,15 +928,6 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst)
return null;
}

/// <summary>
/// Do nothing
/// </summary>
/// <param name="baseCtorInvokeMemberExpressionAst"></param>
/// <returns></returns>
public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst)
{
return null;
}

/// <summary>
/// Do nothing
Expand Down
Loading