Merge pull request #56 from PowerShell/justificationprototype

Add -SuppressedOnly switch to ScriptAnalyzer

Author: Quoc Truong

Engine/Commands/InvokeScriptAnalyzerCommand.cs

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

Engine/Generic/ILogger.cs

@@ -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.

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>
@@ -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())
@@ -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/Generic/SuppressedRecord.cs

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

Engine/Helper.cs

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