Change the RuleSuppression infrastructure to support getting suppressed message

Author: Quoc Truong

Engine/Commands/InvokeScriptAnalyzerCommand.cs

@@ -328,7 +328,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 = scriptRule.AnalyzeScript(ast, filePath).ToList();
+                            Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, records);
+                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
                         }
                         catch (Exception scriptRuleException)
                         {
@@ -356,7 +358,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 = tokenRule.AnalyzeTokens(tokens, fileName).ToList();
+                            Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, records);
+                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
                         }
                         catch (Exception tokenRuleException)
                         {
@@ -384,7 +388,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 = dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList();
+                            Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);
+                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
                         }
                         catch (Exception dscResourceRuleException)
                         {
@@ -426,7 +432,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 = dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList();
+                                                Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);
+                                                diagnostics.AddRange(records.Where(record => record.Suppression == null));
                                             }
                                             catch (Exception dscResourceRuleException)
                                             {

Engine/Generic/DiagnosticRecord.cs

@@ -29,6 +29,15 @@ public string Message
             set { message = value; }
         }
 
+        /// <summary>
+        /// The Rule Suppression that applies to this record (null if none is applied)
+        /// </summary>
+        public RuleSuppression Suppression
+        {
+            get;
+            set;
+        }
+
         /// <summary>
         /// Represents a span of text in a script.
         /// </summary>

Engine/Generic/RuleSuppression.cs

@@ -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>
@@ -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;
@@ -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;
                         }
@@ -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>
@@ -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));
                         }
                     }
 

Engine/Helper.cs

@@ -716,25 +716,23 @@ 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 by attaching rule suppression to the record that is 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 void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)
         {
-            List<DiagnosticRecord> results = new List<DiagnosticRecord>();
-
             if (ruleSuppressionsDict == null || !ruleSuppressionsDict.ContainsKey(ruleName)
                 || diagnostics == null || diagnostics.Count == 0)
             {
-                return diagnostics;
+                return;
             }
 
             List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];
 
             if (ruleSuppressions.Count == 0)
             {
-                return diagnostics;
+                return;
             }
 
             int recordIndex = 0;
@@ -760,46 +758,49 @@ 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)
+                // 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)
                 {
-                    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 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;
-                }
-                // 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))
+                        ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
+                        suppressionCount = 0;
+
+                        continue;
+                    }
+                    // at this point, the record is inside the interval
+                    else
                     {
-                        results.Add(record);
-                        suppressionCount -= 1;
+                        // 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;
+                        }
+                        // otherwise, we suppress the record, move on to the next.
+                        else
+                        {
+                            record.Suppression = ruleSuppression;
+                        }
                     }
-                    // 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
@@ -821,15 +822,6 @@ 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]);
-                recordIndex += 1;
-            }
-
-            return results;
         }
 
         #endregion