Merge pull request #566 from PowerShell/kapilmb/FixRuleSuppressionOffsets

Fix rule suppression bug caused by modified extents

Author: Kapil Borle

Engine/Helper.cs

@@ -1310,15 +1310,17 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
             }
 
             List<RuleSuppression> ruleSuppressions = ruleSuppressionsDict[ruleName];
-
+            var offsetArr = GetOffsetArray(diagnostics);
             int recordIndex = 0;
             int startRecord = 0;
             bool[] suppressed = new bool[diagnostics.Count];
-
             foreach (RuleSuppression ruleSuppression in ruleSuppressions)
             {
                 int suppressionCount = 0;
-                while (startRecord < diagnostics.Count && diagnostics[startRecord].Extent.StartOffset < ruleSuppression.StartOffset)
+                while (startRecord < diagnostics.Count
+                    // && diagnostics[startRecord].Extent.StartOffset < ruleSuppression.StartOffset)
+                    // && diagnostics[startRecord].Extent.StartLineNumber < ruleSuppression.st)
+                    && offsetArr[startRecord].Item1 < ruleSuppression.StartOffset)
                 {
                     startRecord += 1;
                 }
@@ -1329,8 +1331,10 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
                 while (recordIndex < diagnostics.Count)
                 {
                     DiagnosticRecord record = diagnostics[recordIndex];
+                    var curOffset = offsetArr[recordIndex];
 
-                    if (record.Extent.EndOffset > ruleSuppression.EndOffset)
+                    //if (record.Extent.EndOffset > ruleSuppression.EndOffset)
+                    if (curOffset.Item2 > ruleSuppression.EndOffset)
                     {
                         break;
                     }
@@ -1378,6 +1382,57 @@ public Tuple<List<SuppressedRecord>, List<DiagnosticRecord>> SuppressRule(
             return result;
         }
 
+        private Tuple<int,int>[] GetOffsetArray(List<DiagnosticRecord> diagnostics)
+        {
+            Func<int,int,Tuple<int,int>> GetTuple = (x, y) => new Tuple<int, int>(x, y);
+            Func<Tuple<int, int>> GetDefaultTuple = () => GetTuple(0, 0);
+            var offsets = new Tuple<int, int>[diagnostics.Count];
+            for (int k = 0; k < diagnostics.Count; k++)
+            {
+                var ext = diagnostics[k].Extent;
+                if (ext.StartOffset == 0 && ext.EndOffset == 0)
+                {
+                    // check if line and column number correspond to 0 offsets
+                    if (ext.StartLineNumber == 1
+                        && ext.StartColumnNumber == 1
+                        && ext.EndLineNumber == 1
+                        && ext.EndColumnNumber == 1)
+                    {
+                        offsets[k] = GetDefaultTuple();
+                        continue;
+                    }
+                    // created using the ScriptExtent constructor, which sets
+                    // StartOffset and EndOffset to 0
+                    // find the token the corresponding start line and column number
+                    var startToken = Tokens.Where(x
+                        => x.Extent.StartLineNumber == ext.StartLineNumber
+                        && x.Extent.StartColumnNumber == ext.StartColumnNumber)
+                        .FirstOrDefault();
+                    if (startToken == null)
+                    {
+                        offsets[k] = GetDefaultTuple();
+                        continue;
+                    }
+                    var endToken = Tokens.Where(x
+                        => x.Extent.EndLineNumber == ext.EndLineNumber
+                        && x.Extent.EndColumnNumber == ext.EndColumnNumber)
+                        .FirstOrDefault();
+                    if (endToken == null)
+                    {
+                        offsets[k] = GetDefaultTuple();
+                        continue;
+                    }
+                    offsets[k] = GetTuple(startToken.Extent.StartOffset, endToken.Extent.EndOffset);
+                }
+                else
+                {
+                    // Extent has valid offsets
+                    offsets[k] = GetTuple(ext.StartOffset, ext.EndOffset);
+                }
+            }
+            return offsets;
+        }
+
         public static string[] ProcessCustomRulePaths(string[] rulePaths, SessionState sessionState, bool recurse = false)
         {
             //if directory is given, list all the psd1 files

Tests/Engine/RuleSuppression.tests.ps1

@@ -30,6 +30,21 @@ $securePassword = ConvertTo-SecureString $decryptedPassword -AsPlainText -Force
 }
 '@
 
+# If function doesn't starts at offset 0, then the test case fails before commit b551211
+$ruleSuppressionAvoidUsernameAndPassword = @'
+
+function SuppressUserAndPwdRule()
+{
+    [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidUsingUserNameAndPassWordParams", "")]
+    [CmdletBinding()]
+    param
+    (
+        [System.String] $username,
+        [System.Boolean] $password
+    )
+}
+'@
+
 Describe "RuleSuppressionWithoutScope" {
     Context "Function" {
         It "Does not raise violations" {
@@ -38,6 +53,16 @@ Describe "RuleSuppressionWithoutScope" {
             $suppression = $violationsUsingScriptDefinition | Where-Object { $_.RuleName -eq "PSProvideCommentHelp" }
             $suppression.Count | Should Be 0
         }
+
+        It "Suppresses rule with extent created using ScriptExtent constructor" {
+            Invoke-ScriptAnalyzer `
+              -ScriptDefinition $ruleSuppressionAvoidUsernameAndPassword `
+              -IncludeRule "PSAvoidUsingUserNameAndPassWordParams" `
+              -OutVariable ruleViolations `
+              -SuppressedOnly
+            $ruleViolations.Count | Should Be 1
+	    }
+
     }
 
     Context "Script" {