Format suppressed messages output

quoctruong · quoctruong · commit f1086f5cb094 · 2015-04-22T11:10:57.000-07:00
diff --git a/Engine/Commands/InvokeScriptAnalyzerCommand.cs b/Engine/Commands/InvokeScriptAnalyzerCommand.cs
@@ -104,12 +104,12 @@ public SwitchParameter Recurse
         /// </summary>
         [Parameter(Mandatory = false)]
         [SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]
-        public SwitchParameter ShowSuppressed
+        public SwitchParameter SuppressedOnly
         {
-            get { return showSuppressed; }
-            set { showSuppressed = value; }
+            get { return suppressedOnly; }
+            set { suppressedOnly = value; }
         }
-        private bool showSuppressed;
+        private bool suppressedOnly;
 
         #endregion Parameters
 
@@ -268,7 +268,7 @@ private void AnalyzeFile(string filePath)
             Token[] tokens = null;
             ParseError[] errors = null;
             List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();
-            List<DiagnosticRecord> suppressed = 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>>();            
@@ -341,10 +341,9 @@ private void AnalyzeFile(string filePath)
                         // We want the Engine to continue functioning even if one or more Rules throws an exception
                         try
                         {
-                            var records = scriptRule.AnalyzeScript(ast, filePath).ToList();
-                            Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, records);
-                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
-                            suppressed.AddRange(records.Where(record => record.Suppression != null));
+                            var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());
+                            diagnostics.AddRange(records.Item2);
+                            suppressed.AddRange(records.Item1);
                         }
                         catch (Exception scriptRuleException)
                         {
@@ -372,10 +371,9 @@ private void AnalyzeFile(string filePath)
                         // We want the Engine to continue functioning even if one or more Rules throws an exception
                         try
                         {
-                            var records = tokenRule.AnalyzeTokens(tokens, fileName).ToList();
-                            Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, records);
-                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
-                            suppressed.AddRange(records.Where(record => record.Suppression != null));
+                            var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());
+                            diagnostics.AddRange(records.Item2);
+                            suppressed.AddRange(records.Item1);
                         }
                         catch (Exception tokenRuleException)
                         {
@@ -403,10 +401,9 @@ private void AnalyzeFile(string filePath)
                         // We want the Engine to continue functioning even if one or more Rules throws an exception
                         try
                         {
-                            var records = dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList();
-                            Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);
-                            diagnostics.AddRange(records.Where(record => record.Suppression == null));
-                            suppressed.AddRange(records.Where(record => record.Suppression != null));
+                            var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());
+                            diagnostics.AddRange(records.Item2);
+                            suppressed.AddRange(records.Item1);
                         }
                         catch (Exception dscResourceRuleException)
                         {
@@ -448,10 +445,9 @@ private void AnalyzeFile(string filePath)
                                             // We want the Engine to continue functioning even if one or more Rules throws an exception
                                             try
                                             {
-                                                var records = dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList();
-                                                Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);
-                                                diagnostics.AddRange(records.Where(record => record.Suppression == null));
-                                                suppressed.AddRange(records.Where(record => record.Suppression != null));
+                                                var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());
+                                                diagnostics.AddRange(records.Item2);
+                                                suppressed.AddRange(records.Item1);
                                             }
                                             catch (Exception dscResourceRuleException)
                                             {
@@ -509,7 +505,7 @@ private void AnalyzeFile(string filePath)
             //Output through loggers
             foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers)
             {
-                if (ShowSuppressed)
+                if (SuppressedOnly)
                 {
                     foreach (DiagnosticRecord suppressRecord in suppressed)
                     {
diff --git a/Engine/Generic/DiagnosticRecord.cs b/Engine/Generic/DiagnosticRecord.cs
@@ -19,7 +19,6 @@ public class DiagnosticRecord
         private DiagnosticSeverity severity;
         private string scriptName;
         private string ruleSuppressionId;
-        private RuleSuppression suppression;
 
         /// <summary>
         /// Represents a string from the rule about why this diagnostic was created.
@@ -30,22 +29,6 @@ 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 { return suppression; }
-            set
-            {
-                suppression = value;
-                if (suppression != null)
-                {
-                    Message = suppression.Justification;
-                }
-            }
-        }
-
         /// <summary>
         /// Represents a span of text in a script.
         /// </summary>
diff --git a/Engine/Generic/SuppressedRecord.cs b/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;
+            }
+        }
+    }
+}
diff --git a/Engine/Helper.cs b/Engine/Helper.cs
@@ -716,23 +716,28 @@ internal List<RuleSuppression> GetSuppressionsClass(TypeDefinitionAst typeAst)
         }
 
         /// <summary>
-        /// Suppress the rules from the diagnostic records list by attaching rule suppression to the record that is suppressed
+        /// 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 void 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<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;
+                return result;
             }
 
             List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];
 
             if (ruleSuppressions.Count == 0)
             {
-                return;
+                return result;
             }
 
             int recordIndex = 0;
@@ -794,14 +799,19 @@ public void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppressio
                             && !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))
                         {
                             suppressionCount -= 1;
+                            unSuppressedRecords.Add(record);
                         }
                         // otherwise, we suppress the record, move on to the next.
                         else
                         {
-                            record.Suppression = ruleSuppression;
+                            suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression));
                         }
                     }
                 }
+                else
+                {
+                    unSuppressedRecords.Add(record);
+                }
 
                 // important assumption: this point is reached only if we want to move to the next record
                 recordIndex += 1;
@@ -822,6 +832,14 @@ public void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppressio
 
                 record = diagnostics[recordIndex];
             }
+
+            while (recordIndex < diagnostics.Count)
+            {
+                unSuppressedRecords.Add(diagnostics[recordIndex]);
+                recordIndex += 1;
+            }
+
+            return result;
         }
 
         #endregion
@@ -910,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
diff --git a/Engine/ScriptAnalyzerEngine.csproj b/Engine/ScriptAnalyzerEngine.csproj
@@ -52,6 +52,7 @@
     <Compile Include="Commands\InvokeScriptAnalyzerCommand.cs" />
     <Compile Include="Generic\AvoidCmdletGeneric.cs" />
     <Compile Include="Generic\AvoidParameterGeneric.cs" />
+    <Compile Include="Generic\SuppressedRecord.cs" />
     <Compile Include="Generic\DiagnosticRecord.cs" />
     <Compile Include="Generic\ExternalRule.cs" />
     <Compile Include="Generic\IDSCResourceRule.cs" />
@@ -99,4 +100,4 @@ copy /y "$(ProjectDir)\*.ps1xml" "$(SolutionDir)$(SolutionName)"
 copy /y "$(ProjectDir)\*.psd1" "$(SolutionDir)$(SolutionName)"
 copy /y "$(TargetPath)" "$(SolutionDir)$(SolutionName)"</PostBuildEvent>
   </PropertyGroup>
-</Project>
+</Project>

Original file line number	Diff line number	Diff line change
`@@ -104,12 +104,12 @@ public SwitchParameter Recurse`
`104`	`104`	`/// </summary>`
`105`	`105`	`[Parameter(Mandatory = false)]`
`106`	`106`	`[SuppressMessage("Microsoft.Performance", "CA1819:PropertiesShouldNotReturnArrays")]`
`107`		`- public SwitchParameter ShowSuppressed`
	`107`	`+ public SwitchParameter SuppressedOnly`
`108`	`108`	`{`
`109`		`- get { return showSuppressed; }`
`110`		`- set { showSuppressed = value; }`
	`109`	`+ get { return suppressedOnly; }`
	`110`	`+ set { suppressedOnly = value; }`
`111`	`111`	`}`
`112`		`- private bool showSuppressed;`
	`112`	`+ private bool suppressedOnly;`
`113`	`113`
`114`	`114`	`#endregion Parameters`
`115`	`115`
`@@ -268,7 +268,7 @@ private void AnalyzeFile(string filePath)`
`268`	`268`	`Token[] tokens = null;`
`269`	`269`	`ParseError[] errors = null;`
`270`	`270`	`List<DiagnosticRecord> diagnostics = new List<DiagnosticRecord>();`
`271`		`- List<DiagnosticRecord> suppressed = new List<DiagnosticRecord>();`
	`271`	`+ List<SuppressedRecord> suppressed = new List<SuppressedRecord>();`
`272`	`272`
`273`	`273`	`// Use a List of KVP rather than dictionary, since for a script containing inline functions with same signature, keys clash`
`274`	`274`	`List<KeyValuePair<CommandInfo, IScriptExtent>> cmdInfoTable = new List<KeyValuePair<CommandInfo, IScriptExtent>>();`
`@@ -341,10 +341,9 @@ private void AnalyzeFile(string filePath)`
`341`	`341`	`// We want the Engine to continue functioning even if one or more Rules throws an exception`
`342`	`342`	`try`
`343`	`343`	`{`
`344`		`- var records = scriptRule.AnalyzeScript(ast, filePath).ToList();`
`345`		`- Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, records);`
`346`		`- diagnostics.AddRange(records.Where(record => record.Suppression == null));`
`347`		`- suppressed.AddRange(records.Where(record => record.Suppression != null));`
	`344`	`+ var records = Helper.Instance.SuppressRule(scriptRule.GetName(), ruleSuppressions, scriptRule.AnalyzeScript(ast, filePath).ToList());`
	`345`	`+ diagnostics.AddRange(records.Item2);`
	`346`	`+ suppressed.AddRange(records.Item1);`
`348`	`347`	`}`
`349`	`348`	`catch (Exception scriptRuleException)`
`350`	`349`	`{`
`@@ -372,10 +371,9 @@ private void AnalyzeFile(string filePath)`
`372`	`371`	`// We want the Engine to continue functioning even if one or more Rules throws an exception`
`373`	`372`	`try`
`374`	`373`	`{`
`375`		`- var records = tokenRule.AnalyzeTokens(tokens, fileName).ToList();`
`376`		`- Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, records);`
`377`		`- diagnostics.AddRange(records.Where(record => record.Suppression == null));`
`378`		`- suppressed.AddRange(records.Where(record => record.Suppression != null));`
	`374`	`+ var records = Helper.Instance.SuppressRule(tokenRule.GetName(), ruleSuppressions, tokenRule.AnalyzeTokens(tokens, filePath).ToList());`
	`375`	`+ diagnostics.AddRange(records.Item2);`
	`376`	`+ suppressed.AddRange(records.Item1);`
`379`	`377`	`}`
`380`	`378`	`catch (Exception tokenRuleException)`
`381`	`379`	`{`
`@@ -403,10 +401,9 @@ private void AnalyzeFile(string filePath)`
`403`	`401`	`// We want the Engine to continue functioning even if one or more Rules throws an exception`
`404`	`402`	`try`
`405`	`403`	`{`
`406`		`- var records = dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList();`
`407`		`- Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);`
`408`		`- diagnostics.AddRange(records.Where(record => record.Suppression == null));`
`409`		`- suppressed.AddRange(records.Where(record => record.Suppression != null));`
	`404`	`+ var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCClass(ast, filePath).ToList());`
	`405`	`+ diagnostics.AddRange(records.Item2);`
	`406`	`+ suppressed.AddRange(records.Item1);`
`410`	`407`	`}`
`411`	`408`	`catch (Exception dscResourceRuleException)`
`412`	`409`	`{`
`@@ -448,10 +445,9 @@ private void AnalyzeFile(string filePath)`
`448`	`445`	`// We want the Engine to continue functioning even if one or more Rules throws an exception`
`449`	`446`	`try`
`450`	`447`	`{`
`451`		`- var records = dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList();`
`452`		`- Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, records);`
`453`		`- diagnostics.AddRange(records.Where(record => record.Suppression == null));`
`454`		`- suppressed.AddRange(records.Where(record => record.Suppression != null));`
	`448`	`+ var records = Helper.Instance.SuppressRule(dscResourceRule.GetName(), ruleSuppressions, dscResourceRule.AnalyzeDSCResource(ast, filePath).ToList());`
	`449`	`+ diagnostics.AddRange(records.Item2);`
	`450`	`+ suppressed.AddRange(records.Item1);`
`455`	`451`	`}`
`456`	`452`	`catch (Exception dscResourceRuleException)`
`457`	`453`	`{`
`@@ -509,7 +505,7 @@ private void AnalyzeFile(string filePath)`
`509`	`505`	`//Output through loggers`
`510`	`506`	`foreach (ILogger logger in ScriptAnalyzer.Instance.Loggers)`
`511`	`507`	`{`
`512`		`- if (ShowSuppressed)`
	`508`	`+ if (SuppressedOnly)`
`513`	`509`	`{`
`514`	`510`	`foreach (DiagnosticRecord suppressRecord in suppressed)`
`515`	`511`	`{`
Original file line number	Diff line number	Diff line change
`@@ -716,23 +716,28 @@ internal List<RuleSuppression> GetSuppressionsClass(TypeDefinitionAst typeAst)`
`716`	`716`	`}`
`717`	`717`
`718`	`718`	`/// <summary>`
`719`		`- /// Suppress the rules from the diagnostic records list by attaching rule suppression to the record that is suppressed`
	`719`	`+ /// Suppress the rules from the diagnostic records list.`
	`720`	`+ /// Returns a list of suppressed records as well as the ones that are not suppressed`
`720`	`721`	`/// </summary>`
`721`	`722`	`/// <param name="ruleSuppressions"></param>`
`722`	`723`	`/// <param name="diagnostics"></param>`
`723`		`- public void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)`
	`724`	`+ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(string ruleName, Dictionary<string, List<RuleSuppression>> ruleSuppressionsDict, List<DiagnosticRecord> diagnostics)`
`724`	`725`	`{`
	`726`	`+ List<SuppressedRecord> suppressedRecords = new List<SuppressedRecord>();`
	`727`	`+ List<DiagnosticRecord> unSuppressedRecords = new List<DiagnosticRecord>();`
	`728`	`+ Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> result = Tuple.Create(suppressedRecords, unSuppressedRecords);`
	`729`	`+`
`725`	`730`	`if (ruleSuppressionsDict == null \|\| !ruleSuppressionsDict.ContainsKey(ruleName)`
`726`	`731`	`\|\| diagnostics == null \|\| diagnostics.Count == 0)`
`727`	`732`	`{`
`728`		`- return;`
	`733`	`+ return result;`
`729`	`734`	`}`
`730`	`735`
`731`	`736`	`List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];`
`732`	`737`
`733`	`738`	`if (ruleSuppressions.Count == 0)`
`734`	`739`	`{`
`735`		`- return;`
	`740`	`+ return result;`
`736`	`741`	`}`
`737`	`742`
`738`	`743`	`int recordIndex = 0;`
`@@ -794,14 +799,19 @@ public void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppressio`
`794`	`799`	`&& !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))`
`795`	`800`	`{`
`796`	`801`	`suppressionCount -= 1;`
	`802`	`+ unSuppressedRecords.Add(record);`
`797`	`803`	`}`
`798`	`804`	`// otherwise, we suppress the record, move on to the next.`
`799`	`805`	`else`
`800`	`806`	`{`
`801`		`- record.Suppression = ruleSuppression;`
	`807`	`+ suppressedRecords.Add(new SuppressedRecord(record, ruleSuppression));`
`802`	`808`	`}`
`803`	`809`	`}`
`804`	`810`	`}`
	`811`	`+ else`
	`812`	`+ {`
	`813`	`+ unSuppressedRecords.Add(record);`
	`814`	`+ }`
`805`	`815`
`806`	`816`	`// important assumption: this point is reached only if we want to move to the next record`
`807`	`817`	`recordIndex += 1;`
`@@ -822,6 +832,14 @@ public void SuppressRule(string ruleName, Dictionary<string, List<RuleSuppressio`
`822`	`832`
`823`	`833`	`record = diagnostics[recordIndex];`
`824`	`834`	`}`
	`835`	`+`
	`836`	`+ while (recordIndex < diagnostics.Count)`
	`837`	`+ {`
	`838`	`+ unSuppressedRecords.Add(diagnostics[recordIndex]);`
	`839`	`+ recordIndex += 1;`
	`840`	`+ }`
	`841`	`+`
	`842`	`+ return result;`
`825`	`843`	`}`
`826`	`844`
`827`	`845`	`#endregion`
`@@ -910,15 +928,6 @@ public object VisitScriptBlock(ScriptBlockAst scriptBlockAst)`
`910`	`928`	`return null;`
`911`	`929`	`}`
`912`	`930`
`913`		`- /// <summary>`
`914`		`- /// Do nothing`
`915`		`- /// </summary>`
`916`		`- /// <param name="baseCtorInvokeMemberExpressionAst"></param>`
`917`		`- /// <returns></returns>`
`918`		`- public object VisitBaseCtorInvokeMemberExpression(BaseCtorInvokeMemberExpressionAst baseCtorInvokeMemberExpressionAst)`
`919`		`- {`
`920`		`- return null;`
`921`		`- }`
`922`	`931`
`923`	`932`	`/// <summary>`
`924`	`933`	`/// Do nothing`