Merge pull request #45 from PowerShell/suppression

Fix bugs from bugbash

Author: quoctruong

Engine/Generic/AvoidParameterGeneric.cs

@@ -35,7 +35,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                     {
                         if (ParameterCondition(cmdAst, ceAst))
                         {
-                            yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
+                            yield return new DiagnosticRecord(GetError(fileName, cmdAst), cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, cmdAst.GetCommandName());
                         }
                     }
                 }

Engine/Generic/RuleSuppression.cs

@@ -26,6 +26,15 @@ public class RuleSuppression
     {
         private string _ruleName;
 
+        /// <summary>
+        /// The start offset of the rule suppression attribute (not where it starts to apply)
+        /// </summary>
+        public int StartAttributeLine
+        {
+            get;
+            set;
+        }
+
         /// <summary>
         /// The start offset of the rule suppression
         /// </summary>
@@ -57,7 +66,9 @@ public string RuleName
             set
             {
                 _ruleName = value;
-                if ((ScriptAnalyzer.Instance.ScriptRules != null
+
+                if (!String.IsNullOrWhiteSpace(_ruleName)
+                    && (ScriptAnalyzer.Instance.ScriptRules != null
                         && ScriptAnalyzer.Instance.ScriptRules.Count(item => String.Equals(item.GetName(), _ruleName, StringComparison.OrdinalIgnoreCase)) == 0)
                     && (ScriptAnalyzer.Instance.TokenRules != null
                         && ScriptAnalyzer.Instance.TokenRules.Count(item => String.Equals(item.GetName(), _ruleName, StringComparison.OrdinalIgnoreCase)) == 0)
@@ -131,6 +142,7 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
 
             if (attrAst != null)
             {
+                StartAttributeLine = attrAst.Extent.StartLineNumber;
                 var positionalArguments = attrAst.PositionalArguments;
                 var namedArguments = attrAst.NamedArguments;
 
@@ -236,6 +248,18 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
                     }
                 }
 
+                if (!String.IsNullOrWhiteSpace(Error))
+                {
+                    // May be cases where the rulename is null because we didn't look at the rulename after
+                    // we found out there is an error
+                    RuleName = String.Empty;
+                }
+                else if (String.IsNullOrWhiteSpace(RuleName))
+                {
+                    RuleName = String.Empty;
+                    Error = Strings.NullRuleNameError;
+                }
+                
                 // Must have scope and target together
                 if (String.IsNullOrWhiteSpace(Scope) && !String.IsNullOrWhiteSpace(Target))
                 {
@@ -248,24 +272,26 @@ public RuleSuppression(AttributeAst attrAst, int start, int end)
 
             if (!String.IsNullOrWhiteSpace(Error))
             {
-                Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, attrAst.Extent.StartLineNumber,
+                Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, StartAttributeLine,
                     System.IO.Path.GetFileName(attrAst.Extent.File), Error);
             }
         }
 
         /// <summary>
-        /// Constructs rule expression from rule name, id, start and end
+        /// Constructs rule expression from rule name, id, start, end and startAttributeLine
         /// </summary>
         /// <param name="ruleName"></param>
         /// <param name="ruleSuppressionID"></param>
         /// <param name="start"></param>
         /// <param name="end"></param>
-        public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end)
+        /// <param name="startAttributeLine"></param>
+        public RuleSuppression(string ruleName, string ruleSuppressionID, int start, int end, int startAttributeLine)
         {
             RuleName = ruleName;
             RuleSuppressionID = ruleSuppressionID;
             StartOffset = start;
             EndOffset = end;
+            StartAttributeLine = startAttributeLine;
         }
 
         /// <summary>
@@ -320,16 +346,24 @@ public static List<RuleSuppression> GetSuppressions(IEnumerable<AttributeAst> at
 
                     if (targetAsts != null)
                     {
+                        if (targetAsts.Count() == 0)
+                        {
+                            ruleSupp.Error = String.Format(CultureInfo.CurrentCulture, Strings.RuleSuppressionErrorFormat, ruleSupp.StartAttributeLine,
+                                System.IO.Path.GetFileName(scopeAst.Extent.File), String.Format(Strings.TargetCannotBeFoundError, ruleSupp.Target, ruleSupp.Scope));
+                            result.Add(ruleSupp);
+                            continue;
+                        }
+
                         foreach (Ast targetAst in targetAsts)
                         {
-                            result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset, targetAst.Extent.EndOffset));
+                            result.Add(new RuleSuppression(ruleSupp.RuleName, ruleSupp.RuleSuppressionID, targetAst.Extent.StartOffset, targetAst.Extent.EndOffset, attributeAst.Extent.StartLineNumber));
                         }
                     }
 
                 }
                 else
                 {
-                    // this may add rule suppression that contains erro but we will check for this in the engine to throw out error
+                    // this may add rule suppression that contains error but we will check for this in the engine to throw out error
                     result.Add(ruleSupp);
                 }
             }

Engine/Helper.cs

@@ -5,6 +5,7 @@
 using System.Management.Automation;
 using System.Management.Automation.Language;
 using System.Management.Automation.Runspaces;
+using System.Globalization;
 using Microsoft.Windows.Powershell.ScriptAnalyzer.Generic;
 
 namespace Microsoft.Windows.Powershell.ScriptAnalyzer
@@ -740,9 +741,25 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
             int ruleSuppressionIndex = 0;
             DiagnosticRecord record = diagnostics.First();
             RuleSuppression ruleSuppression = ruleSuppressions.First();
+            int suppressionCount = 0;
 
             while (recordIndex < diagnostics.Count)
             {
+                if (!String.IsNullOrWhiteSpace(ruleSuppression.Error))
+                {
+                    ruleSuppressionIndex += 1;
+
+                    if (ruleSuppressionIndex == ruleSuppressions.Count)
+                    {
+                        break;
+                    }
+
+                    ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
+                    suppressionCount = 0;
+
+                    continue;
+                }
+
                 // the record precedes the rule suppression so don't apply the suppression
                 if (record.Extent.StartOffset < ruleSuppression.StartOffset)
                 {
@@ -753,12 +770,21 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
                 {
                     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)
+                    {
+                        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));
+                    }
+
                     if (ruleSuppressionIndex == ruleSuppressions.Count)
                     {
                         break;
                     }
 
                     ruleSuppression = ruleSuppressions[ruleSuppressionIndex];
+                    suppressionCount = 0;
 
                     continue;
                 }
@@ -771,21 +797,38 @@ public List<DiagnosticRecord> SuppressRule(string ruleName, Dictionary<string, L
                         && !string.Equals(ruleSuppression.RuleSuppressionID, record.RuleSuppressionID, StringComparison.OrdinalIgnoreCase))
                     {
                         results.Add(record);
+                        suppressionCount -= 1;
                     }
                     // 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;
+                suppressionCount += 1;
 
                 if (recordIndex == diagnostics.Count)
                 {
+                    // 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));
+                    }
+
                     break;
                 }
 
                 record = diagnostics[recordIndex];
             }
 
+            // Add all unprocessed records to results
+            while (recordIndex < diagnostics.Count)
+            {
+                results.Add(diagnostics[recordIndex]);
+                recordIndex += 1;
+            }
+
             return results;
         }
 

Engine/Strings.Designer.cs

@@ -183,4 +183,13 @@
   <data name="WrongScopeArgumentSuppressionAttributeError" xml:space="preserve">
     <value>Scope can only be either function or class.</value>
   </data>
+  <data name="NullRuleNameError" xml:space="preserve">
+    <value>RuleName must not be null.</value>
+  </data>
+  <data name="TargetCannotBeFoundError" xml:space="preserve">
+    <value>Cannot find any Targets {0} that match the Scope {1} to apply the SuppressMessageAttribute.</value>
+  </data>
+  <data name="RuleSuppressionIDError" xml:space="preserve">
+    <value>Cannot find any DiagnosticRecord with the Rule Suppression ID {0}.</value>
+  </data>
 </root>

Engine/Strings.resx

@@ -35,17 +35,18 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
             foreach (Ast foundAst in foundAsts)
             {
                 CommandAst cmdAst = (CommandAst)foundAst;
+                string aliasName = cmdAst.GetCommandName();
                 // Handles the exception caused by commands like, {& $PLINK $args 2> $TempErrorFile}.
                 // You can also review the remark section in following document,
                 // MSDN: CommandAst.GetCommandName Method
-                if (cmdAst.GetCommandName() == null) continue;
+                if (aliasName == null) continue;
 
-                string cmdletName = Microsoft.Windows.Powershell.ScriptAnalyzer.Helper.Instance.GetCmdletNameFromAlias(cmdAst.GetCommandName());
+                string cmdletName = Microsoft.Windows.Powershell.ScriptAnalyzer.Helper.Instance.GetCmdletNameFromAlias(aliasName);
 
                 if (!String.IsNullOrEmpty(cmdletName))
                 {
-                    yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, cmdAst.GetCommandName(), cmdletName),
-                        cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
+                    yield return new DiagnosticRecord(string.Format(CultureInfo.CurrentCulture, Strings.AvoidUsingCmdletAliasesError, aliasName, cmdletName),
+                        cmdAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, aliasName);
                 }
             }
         }

Rules/AvoidAlias.cs

@@ -40,7 +40,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                         yield return
                             new DiagnosticRecord(
                                 string.Format(CultureInfo.CurrentCulture, Strings.AvoidGlobalVarsError,
-                                    varAst.VariablePath.UserPath), varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName);
+                                    varAst.VariablePath.UserPath), varAst.Extent, GetName(), DiagnosticSeverity.Warning, fileName, varAst.VariablePath.UserPath);
                     }
                 }
             }