Merge pull request #493 from PowerShell/FixAvoidUsernameAndPasswordParamsExtent

Fix extent of AvoidUsernameAndPasswordParams rule

Author: Kapil Borle

Rules/AvoidUserNameAndPasswordParams.cs

@@ -50,6 +50,9 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
                 // Finds all ParamAsts.
                 IEnumerable<Ast> paramAsts = funcAst.FindAll(testAst => testAst is ParameterAst, true);
+
+                ParameterAst usernameAst = null;
+                ParameterAst passwordAst = null;
                 // Iterates all ParamAsts and check if their names are on the list.
                 foreach (ParameterAst paramAst in paramAsts)
                 {
@@ -67,7 +70,6 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                         || paramAttribute.TypeName.GetReflectionType() == typeof(System.Security.SecureString));
 
                     String paramName = paramAst.Name.VariablePath.ToString();                    
-
                     foreach (String password in passwords)
                     {
                         if (paramName.IndexOf(password, StringComparison.OrdinalIgnoreCase) != -1)
@@ -79,6 +81,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                             }
 
                             hasPwd = true;
+                            passwordAst = paramAst;
                             break;
                         }
                     }
@@ -88,6 +91,7 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                         if (paramName.IndexOf(username, StringComparison.OrdinalIgnoreCase) != -1)
                         {
                             hasUserName = true;
+                            usernameAst = paramAst;
                             break;
                         }
                     }
@@ -97,11 +101,49 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 {
                     yield return new DiagnosticRecord(
                         String.Format(CultureInfo.CurrentCulture, Strings.AvoidUsernameAndPasswordParamsError, funcAst.Name),
-                        funcAst.Extent, GetName(), DiagnosticSeverity.Error, fileName);
+                        GetExtent(usernameAst, passwordAst, ast), GetName(), DiagnosticSeverity.Error, fileName);
                 }
             }
         }
 
+        /// <summary>
+        /// Returns script extent of username and password parameters
+        /// </summary>
+        /// <param name="usernameAst"></param>
+        /// <param name="passwordAst"></param>
+        /// <returns>IScriptExtent</returns>
+        private IScriptExtent GetExtent(ParameterAst usernameAst, ParameterAst passwordAst, Ast scriptAst)
+        {
+            var usrExt = usernameAst.Extent;
+            var pwdExt = passwordAst.Extent;
+            IScriptExtent startExt, endExt;
+            var usrBeforePwd 
+                = (usrExt.StartLineNumber == pwdExt.StartLineNumber
+                    && usrExt.StartColumnNumber < pwdExt.StartColumnNumber)
+                    || usrExt.StartLineNumber < pwdExt.StartLineNumber;
+            if (usrBeforePwd)
+            {
+                startExt = usrExt;
+                endExt = pwdExt;
+            }
+            else
+            {
+                startExt = pwdExt;
+                endExt = usrExt;
+            }
+            var startPos = new ScriptPosition(
+                startExt.File,
+                startExt.StartLineNumber,
+                startExt.StartColumnNumber,
+                startExt.StartScriptPosition.Line);
+            var endPos = new ScriptPosition(
+                endExt.File,
+                endExt.EndLineNumber,
+                endExt.EndColumnNumber,
+                endExt.EndScriptPosition.Line);
+            return new ScriptExtent(startPos, endPos);
+        }
+
         /// <summary>
         /// GetName: Retrieves the name of this rule.
         /// </summary>

Tests/Rules/AvoidUserNameAndPasswordParams.ps1

@@ -53,7 +53,8 @@ function MyFunction3
     )
 }
 
-function TestFunction($password, [PSCredential[]]$passwords, $username){
+function TestFunction1($password, $username, [PSCredential[]]$passwords){
 }
 
-
+function TestFunction2($username, $password, [PSCredential[]]$passwords){
+}

Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

@@ -1,6 +1,6 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = "Function 'TestFunction' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute."
+$violationMessage = "Function 'TestFunction1' has both Username and Password parameters. Either set the type of the Password parameter to SecureString or replace the Username and Password parameters with a Credential parameter of type PSCredential. If using a Credential parameter in PowerShell 4.0 or earlier, please define a credential transformation attribute after the PSCredential type attribute."
 $violationName = "PSAvoidUsingUserNameAndPasswordParams"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName}
@@ -9,17 +9,25 @@ $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParamsN
 Describe "AvoidUserNameAndPasswordParams" {
     Context "When there are violations" {
         It "has 1 avoid username and password parameter violations" {
-            $violations.Count | Should Be 1
+            $violations.Count | Should Be 2
         }
 
         It "has the correct violation message" {
             $violations[0].Message | Should Be $violationMessage
         }
+
+	It "has correct extent" {
+	   $expectedExtent = '$password, $username'
+	   $violations[0].Extent.Text | Should Be $expectedExtent
+
+	   $expectedExtent = '$username, $password'
+	   $violations[1].Extent.Text | Should Be $expectedExtent
+	}
     }
 
     Context "When there are no violations" {
         It "returns no violations" {
             $noViolations.Count | Should Be 0
         }
     }
-}
+}