diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs index 50742ce63..7e9124bdf 100644 --- a/Engine/Commands/InvokeScriptAnalyzerCommand.cs +++ b/Engine/Commands/InvokeScriptAnalyzerCommand.cs @@ -257,6 +257,7 @@ private void AnalyzeFile(string filePath) Token[] tokens = null; ParseError[] errors = null; List diagnostics = new List(); + IEnumerable funcDefAsts; // Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash @@ -291,6 +292,8 @@ private void AnalyzeFile(string filePath) return; } + Dictionary> ruleSuppressions = Helper.Instance.GetRuleSuppression(ast); + #region Run VariableAnalysis try { @@ -317,11 +320,11 @@ private void AnalyzeFile(string filePath) // We want the Engine to continue functioning even if one or more Rules throws an exception try { - diagnostics.AddRange(scriptRule.AnalyzeScript(ast, filePath)); + diagnostics.AddRange(Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList())); } catch (Exception scriptRuleException) { - WriteError(new ErrorRecord(scriptRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, filePath)); + WriteError(new ErrorRecord(scriptRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); continue; } } @@ -382,7 +385,7 @@ private void AnalyzeFile(string filePath) } catch (Exception commandRuleException) { - WriteError(new ErrorRecord(commandRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, fileName)); + WriteError(new ErrorRecord(commandRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, fileName)); continue; } } @@ -411,7 +414,7 @@ private void AnalyzeFile(string filePath) } catch (Exception tokenRuleException) { - WriteError(new ErrorRecord(tokenRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, fileName)); + WriteError(new ErrorRecord(tokenRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, fileName)); continue; } } @@ -439,7 +442,7 @@ private void AnalyzeFile(string filePath) } catch (Exception dscResourceRuleException) { - WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, filePath)); + WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); continue; } } @@ -481,7 +484,7 @@ private void AnalyzeFile(string filePath) } catch (Exception dscResourceRuleException) { - WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, filePath)); + WriteError(new ErrorRecord(dscResourceRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, filePath)); continue; } } @@ -516,7 +519,7 @@ private void AnalyzeFile(string filePath) } catch (Exception externalRuleException) { - WriteError(new ErrorRecord(externalRuleException, Strings.RuleError, ErrorCategory.InvalidOperation, fileName)); + WriteError(new ErrorRecord(externalRuleException, Strings.RuleErrorMessage, ErrorCategory.InvalidOperation, fileName)); } } } diff --git a/Engine/Generic/DiagnosticRecord.cs b/Engine/Generic/DiagnosticRecord.cs index 732777025..ca50c9032 100644 --- a/Engine/Generic/DiagnosticRecord.cs +++ b/Engine/Generic/DiagnosticRecord.cs @@ -18,6 +18,7 @@ public class DiagnosticRecord private string ruleName; private DiagnosticSeverity severity; private string scriptName; + private string ruleSuppressionId; /// /// Represents a string from the rule about why this diagnostic was created. @@ -62,7 +63,16 @@ public string ScriptName { get { return scriptName; } //Trim down to the leaf element of the filePath and pass it to Diagnostic Record - set { scriptName = System.IO.Path.GetFileName(value); ; } + set { scriptName = System.IO.Path.GetFileName(value); } + } + + /// + /// Returns the rule id for this record + /// + public string RuleSuppressionID + { + get { return ruleSuppressionId; } + set { ruleSuppressionId = value; } } /// @@ -81,13 +91,14 @@ public DiagnosticRecord() /// The name of the rule that created this diagnostic /// The severity of this diagnostic /// The name of the script file being analyzed - public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, DiagnosticSeverity severity, string scriptName) + public DiagnosticRecord(string message, IScriptExtent extent, string ruleName, DiagnosticSeverity severity, string scriptName, string ruleId = null) { Message = string.IsNullOrEmpty(message) ? string.Empty : message; RuleName = string.IsNullOrEmpty(ruleName) ? string.Empty : ruleName; Extent = extent; Severity = severity; ScriptName = string.IsNullOrEmpty(scriptName) ? string.Empty : scriptName; + ruleSuppressionId = ruleId; } } diff --git a/Engine/Generic/RuleSuppression.cs b/Engine/Generic/RuleSuppression.cs new file mode 100644 index 000000000..135d3b6a8 --- /dev/null +++ b/Engine/Generic/RuleSuppression.cs @@ -0,0 +1,181 @@ +using System; +using System.Linq; +using System.Management.Automation.Language; +using System.Collections.Generic; + +namespace Microsoft.Windows.Powershell.ScriptAnalyzer.Generic +{ + /// + /// + /// + public class RuleSuppression + { + /// + /// The start offset of the rule suppression + /// + public int StartOffset + { + get; + set; + } + + /// + /// The end offset of the rule suppression + /// + public int EndOffset + { + get; + set; + } + + /// + /// Name of the rule being suppressed + /// + public string RuleName + { + get; + set; + } + + /// + /// ID of the violation instance + /// + public string RuleSuppressionID + { + get; + set; + } + + /// + /// Returns error occurred in trying to parse the attribute + /// + public string Error + { + get; + set; + } + + /// + /// Returns rule suppression from an attribute ast that has the type suppressmessageattribute + /// + /// + /// + /// + public RuleSuppression(AttributeAst attrAst, int start, int end) + { + Error = String.Empty; + + if (attrAst != null) + { + var positionalArguments = attrAst.PositionalArguments; + var namedArguments = attrAst.NamedArguments; + + int lastPositionalArgumentsOffset = -1; + + if (positionalArguments != null && positionalArguments.Count != 0) + { + int count = positionalArguments.Count; + lastPositionalArgumentsOffset = positionalArguments[positionalArguments.Count - 1].Extent.StartOffset; + + if (positionalArguments.Any(item => !(item is StringConstantExpressionAst))) + { + Error = Strings.StringConstantArgumentsSuppressionAttributeError; + } + else + { + switch (count) + { + case 2: + RuleSuppressionID = (positionalArguments[1] as StringConstantExpressionAst).Value; + goto case 1; + + case 1: + RuleName = (positionalArguments[0] as StringConstantExpressionAst).Value; + goto default; + + default: + break; + } + } + } + + if (namedArguments != null && namedArguments.Count != 0) + { + foreach (var name in namedArguments) + { + if (name.Extent.StartOffset < lastPositionalArgumentsOffset) + { + Error = Strings.NamedArgumentsBeforePositionalError; + break; + } + else if (!(name.Argument is StringConstantExpressionAst)) + { + Error = Strings.StringConstantArgumentsSuppressionAttributeError; + break; + } + + switch (name.ArgumentName.ToLower()) + { + case "rulename": + if (!String.IsNullOrWhiteSpace(RuleName)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } + + RuleName = (name.Argument as StringConstantExpressionAst).Value; + goto default; + + case "rulesuppressionid": + if (!String.IsNullOrWhiteSpace(RuleSuppressionID)) + { + Error = String.Format(Strings.NamedAndPositionalArgumentsConflictError, name); + } + + RuleSuppressionID = (name.Argument as StringConstantExpressionAst).Value; + goto default; + + default: + break; + } + } + } + } + + StartOffset = start; + EndOffset = end; + } + + /// + /// Given a list of attribute asts, return a list of rule suppression + /// with startoffset at start and endoffset at end + /// + /// + /// + /// + /// + public static List GetSuppressions(IEnumerable attrAsts, int start, int end) + { + List result = new List(); + + if (attrAsts == null) + { + return result; + } + + IEnumerable suppressionAttribute = attrAsts.Where( + item => item.TypeName.GetReflectionType() == typeof(System.Diagnostics.CodeAnalysis.SuppressMessageAttribute)); + + foreach (var attributeAst in suppressionAttribute) + { + RuleSuppression ruleSupp = new RuleSuppression(attributeAst, start, end); + + if (string.IsNullOrWhiteSpace(ruleSupp.Error)) + { + result.Add(ruleSupp); + } + } + + return result; + } + } +} diff --git a/Engine/Helper.cs b/Engine/Helper.cs index f66c9b33f..14c4fd0cc 100644 --- a/Engine/Helper.cs +++ b/Engine/Helper.cs @@ -5,6 +5,7 @@ using System.Management.Automation; using System.Management.Automation.Language; using System.Management.Automation.Runspaces; +using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic; namespace Microsoft.Windows.Powershell.ScriptAnalyzer { @@ -58,7 +59,7 @@ public static Helper Instance /// public PSCmdlet MyCmdlet { get; set; } - private TupleComparer tupleComparer = new TupleComparer(); + internal TupleComparer tupleComparer = new TupleComparer(); /// /// My Tokens @@ -604,6 +605,190 @@ public bool HasSpecialVars(string varName) return false; } + /// + /// Returns a dictionary of rule suppression from the ast. + /// Key of the dictionary is rule name. + /// Value is a list of tuple of integers that represents the interval to apply the rule + /// + /// + /// + public Dictionary> GetRuleSuppression(Ast ast) + { + List ruleSuppressionList = new List(); + Dictionary> results = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + if (ast == null) + { + return results; + } + + ScriptBlockAst sbAst = ast as ScriptBlockAst; + + // Get rule suppression from the ast itself if it is scriptblockast + if (sbAst != null && sbAst.ParamBlock != null && sbAst.ParamBlock.Attributes != null) + { + ruleSuppressionList.AddRange(RuleSuppression.GetSuppressions(sbAst.ParamBlock.Attributes, sbAst.Extent.StartOffset, sbAst.Extent.EndOffset)); + } + + // Get rule suppression from functions + IEnumerable funcAsts = ast.FindAll(item => item is FunctionDefinitionAst, true).Cast(); + + foreach (var funcAst in funcAsts) + { + ruleSuppressionList.AddRange(GetSuppressionsFunction(funcAst)); + } + + // Get rule suppression from classes + IEnumerable typeAsts = ast.FindAll(item => item is TypeDefinitionAst, true).Cast(); + + foreach (var typeAst in typeAsts) + { + ruleSuppressionList.AddRange(GetSuppressionsClass(typeAst)); + } + + ruleSuppressionList.Sort((item, item2) => item.StartOffset.CompareTo(item2.StartOffset)); + + foreach (RuleSuppression ruleSuppression in ruleSuppressionList) + { + if (!results.ContainsKey(ruleSuppression.RuleName)) + { + List ruleSuppressions = new List(); + results.Add(ruleSuppression.RuleName, ruleSuppressions); + } + + results[ruleSuppression.RuleName].Add(ruleSuppression); + } + + return results; + } + + /// + /// Returns a list of rule suppressions from the function + /// + /// + /// + internal List GetSuppressionsFunction(FunctionDefinitionAst funcAst) + { + List result = new List(); + + if (funcAst != null && funcAst.Body != null + && funcAst.Body.ParamBlock != null && funcAst.Body.ParamBlock.Attributes != null) + { + result.AddRange(RuleSuppression.GetSuppressions(funcAst.Body.ParamBlock.Attributes, funcAst.Extent.StartOffset, funcAst.Extent.EndOffset)); + } + + return result; + } + + /// + /// Returns a list of rule suppression from the class + /// + /// + /// + internal List GetSuppressionsClass(TypeDefinitionAst typeAst) + { + List result = new List(); + + if (typeAst != null && typeAst.Attributes != null && typeAst.Attributes.Count != 0) + { + result.AddRange(RuleSuppression.GetSuppressions(typeAst.Attributes, typeAst.Extent.StartOffset, typeAst.Extent.EndOffset)); + } + + if (typeAst.Members == null) + { + return result; + } + + foreach (var member in typeAst.Members) + { + FunctionMemberAst funcMemb = member as FunctionMemberAst; + + if (funcMemb == null) + { + continue; + } + + result.AddRange(RuleSuppression.GetSuppressions(funcMemb.Attributes, funcMemb.Extent.StartOffset, funcMemb.Extent.EndOffset)); + } + + return result; + } + + /// + /// Suppress the rules from the diagnostic records list and return the result + /// + /// + /// + public List SuppressRule(string ruleName, Dictionary> ruleSuppressionsDict, List diagnostics) + { + List results = new List(); + + if (ruleSuppressionsDict == null || !ruleSuppressionsDict.ContainsKey(ruleName) + || diagnostics == null || diagnostics.Count == 0) + { + return diagnostics; + } + + List ruleSuppressions = ruleSuppressionsDict[ruleName]; + + if (ruleSuppressions.Count == 0) + { + return diagnostics; + } + + int recordIndex = 0; + int ruleSuppressionIndex = 0; + DiagnosticRecord record = diagnostics.First(); + RuleSuppression ruleSuppression = ruleSuppressions.First(); + + while (recordIndex < diagnostics.Count) + { + // 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) + { + ruleSuppressionIndex += 1; + + if (ruleSuppressionIndex == ruleSuppressions.Count) + { + break; + } + + ruleSuppression = ruleSuppressions[ruleSuppressionIndex]; + + 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)) + { + results.Add(record); + } + // otherwise, we ignore the record, move on to the next. + } + + // important assumption: this point is reached only if we want to move to the next record + recordIndex += 1; + + if (recordIndex == diagnostics.Count) + { + break; + } + + record = diagnostics[recordIndex]; + } + + return results; + } + #endregion } diff --git a/Engine/ScriptAnalyzerEngine.csproj b/Engine/ScriptAnalyzerEngine.csproj index 990354497..dcca0b969 100644 --- a/Engine/ScriptAnalyzerEngine.csproj +++ b/Engine/ScriptAnalyzerEngine.csproj @@ -1,4 +1,4 @@ - + Debug @@ -63,6 +63,7 @@ + @@ -70,7 +71,7 @@ - + True True Strings.resx @@ -86,7 +87,7 @@ ResXFileCodeGenerator - Strings.cs + Strings.Designer.cs diff --git a/Engine/Strings.cs b/Engine/Strings.Designer.cs similarity index 84% rename from Engine/Strings.cs rename to Engine/Strings.Designer.cs index 9dfec8b3b..ee875d5f6 100644 --- a/Engine/Strings.cs +++ b/Engine/Strings.Designer.cs @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ // // This code was generated by a tool. -// Runtime Version:4.0.30319.35317 +// Runtime Version:4.0.30319.34014 // // Changes to this file may cause incorrect behavior and will be lost if // the code is regenerated. @@ -141,6 +141,24 @@ internal static string MissingRuleExtension { } } + /// + /// Looks up a localized string similar to {0} cannot be set by both positional and named arguments.. + /// + internal static string NamedAndPositionalArgumentsConflictError { + get { + return ResourceManager.GetString("NamedAndPositionalArgumentsConflictError", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Named arguments must always come after positional arguments.. + /// + internal static string NamedArgumentsBeforePositionalError { + get { + return ResourceManager.GetString("NamedArgumentsBeforePositionalError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Parse error in file {0}: {1} at line {2} column {3}.. /// @@ -159,6 +177,15 @@ internal static string ParserErrorMessage { } } + /// + /// Looks up a localized string similar to RULE_ERROR. + /// + internal static string RuleErrorMessage { + get { + return ResourceManager.GetString("RuleErrorMessage", resourceCulture); + } + } + /// /// Looks up a localized string similar to Cannot find analyzer rules.. /// @@ -168,6 +195,15 @@ internal static string RulesNotFound { } } + /// + /// Looks up a localized string similar to All the arguments of the suppression message attribute should be string constants.. + /// + internal static string StringConstantArgumentsSuppressionAttributeError { + get { + return ResourceManager.GetString("StringConstantArgumentsSuppressionAttributeError", resourceCulture); + } + } + /// /// Looks up a localized string similar to Analyzing file: {0}. /// @@ -185,16 +221,5 @@ internal static string VerboseRunningMessage { return ResourceManager.GetString("VerboseRunningMessage", resourceCulture); } } - - /// - /// Looks up a localized string similar to Rule Error '{0}'.. - /// - internal static string RuleError - { - get - { - return ResourceManager.GetString("RuleErrorMessage", resourceCulture); - } - } } } diff --git a/Engine/Strings.resx b/Engine/Strings.resx index 9181adefc..7dc6afd36 100644 --- a/Engine/Strings.resx +++ b/Engine/Strings.resx @@ -144,6 +144,12 @@ Cannot find rule extension '{0}'. + + {0} cannot be set by both positional and named arguments. + + + Named arguments must always come after positional arguments. + Parse error in file {0}: {1} at line {2} column {3}. @@ -156,6 +162,9 @@ Cannot find analyzer rules. + + All the arguments of the suppression message attribute should be string constants. + Analyzing file: {0} diff --git a/Rules/AvoidUnitializedVariable.cs b/Rules/AvoidUnitializedVariable.cs index fa5dab4a7..d39b489bc 100644 --- a/Rules/AvoidUnitializedVariable.cs +++ b/Rules/AvoidUnitializedVariable.cs @@ -42,7 +42,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (Helper.Instance.IsUninitialized(varAst, ast)) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUninitializedVariableError, varAst.VariablePath.UserPath), - varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, string.Format(CultureInfo.CurrentCulture, "{0}{1}", GetName(), varAst.VariablePath.UserPath)); } } @@ -59,7 +59,7 @@ public IEnumerable AnalyzeScript(Ast ast, string fileName) if (Helper.Instance.IsUninitialized(varAst, funcAst)) { yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUninitializedVariableError, varAst.VariablePath.UserPath), - varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName); + varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, string.Format(CultureInfo.CurrentCulture, "{0}{1}", GetName(), varAst.VariablePath.UserPath)); } } }