Enforce that pscredential attribute must come before credentialattribute

Rules/AvoidUserNameAndPasswordParams.cs

@@ -53,12 +53,16 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
                 // Iterrates all ParamAsts and check if their names are on the list.
                 foreach (ParameterAst paramAst in paramAsts)
                 {
-                    TypeInfo paramType = (TypeInfo)paramAst.StaticType;
+                    var psCredentialType = paramAst.Attributes.FirstOrDefault(paramAttribute =>
+                        (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
+                        || paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
+
+                    var credentialAttribute = paramAst.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
+
                     String paramName = paramAst.Name.VariablePath.ToString();
 
-                    // if this is pscredential type with credential attribute, skip
-                    if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof (PSCredential)))
-                        && paramAst.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)))
+                    // if this is pscredential type with credential attribute where pscredential type comes first
+                    if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset)
                     {
                         continue;
                     }

Rules/Strings.resx

@@ -217,13 +217,13 @@
     <value>One Char</value>
   </data>
   <data name="UsePSCredentialTypeDescription" xml:space="preserve">
-    <value>Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute. This comes from the PowerShell teams best practices.</value>
+    <value>Checks that cmdlets that have a Credential parameter accept PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.. This comes from the PowerShell teams best practices.</value>
   </data>
   <data name="UsePSCredentialTypeError" xml:space="preserve">
-    <value>The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute.</value>
+    <value>The Credential parameter in '{0}' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.</value>
   </data>
   <data name="UsePSCredentialTypeErrorSB" xml:space="preserve">
-    <value>The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute.</value>
+    <value>The Credential parameter in a found script block must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute.</value>
   </data>
   <data name="UsePSCredentialTypeCommonName" xml:space="preserve">
     <value>PSCredential</value>
@@ -511,10 +511,10 @@
     <value>Avoid Using Username and Password Parameters</value>
   </data>
   <data name="AvoidUsernameAndPasswordParamsDescription" xml:space="preserve">
-    <value>Functions should only take in a credential parameter of type PSCredential with CredentialAttribute instead of username and password parameters.</value>
+    <value>Functions should only take in a credential parameter of type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute instead of username and password parameters.</value>
   </data>
   <data name="AvoidUsernameAndPasswordParamsError" xml:space="preserve">
-    <value>Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used.</value>
+    <value>Function '{0}' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used.</value>
   </data>
   <data name="AvoidUsernameAndPasswordParamsName" xml:space="preserve">
     <value>AvoidUsingUserNameAndPassWordParams</value>

Rules/UsePSCredentialType.cs

@@ -73,6 +73,12 @@ public IEnumerable<DiagnosticRecord> AnalyzeScript(Ast ast, string fileName)
 
             foreach (ScriptBlockAst scriptBlockAst in scriptBlockAsts)
             {
+                // check for the case where it's parent is function, in that case we already processed above
+                if (scriptBlockAst.Parent != null && scriptBlockAst.Parent is FunctionDefinitionAst)
+                {
+                    continue;
+                }
+
                 if (scriptBlockAst.ParamBlock != null && scriptBlockAst.ParamBlock.Parameters != null)
                 {
                     foreach (ParameterAst parameter in scriptBlockAst.ParamBlock.Parameters)
@@ -90,10 +96,13 @@ private bool WrongCredentialUsage(ParameterAst parameter)
         {
             if (parameter.Name.VariablePath.UserPath.Equals("Credential", StringComparison.OrdinalIgnoreCase))
             {
-                TypeInfo paramType = (TypeInfo)parameter.StaticType;
+                var psCredentialType = parameter.Attributes.FirstOrDefault(paramAttribute => (paramAttribute.TypeName.IsArray && (paramAttribute.TypeName as ArrayTypeName).ElementType.GetReflectionType() == typeof(PSCredential))
+                    || paramAttribute.TypeName.GetReflectionType() == typeof(PSCredential));
+
+                var credentialAttribute = parameter.Attributes.FirstOrDefault(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute));
 
-                if ((paramType == typeof(PSCredential) || (paramType.IsArray && paramType.GetElementType() == typeof(PSCredential)))
-                    && parameter.Attributes.Any(paramAttribute => paramAttribute.TypeName.GetReflectionType() == typeof(CredentialAttribute)))
+                // check that both exists and pscredentialtype comes before credential attribute
+                if (psCredentialType != null && credentialAttribute != null && psCredentialType.Extent.EndOffset < credentialAttribute.Extent.StartOffset)
                 {
                     return false;
                 }

Tests/Rules/AvoidUserNameAndPasswordParams.ps1

@@ -31,5 +31,29 @@
     }
 }
 
+function MyFunction3
+{
+    [CmdletBinding()]
+    [Alias()]
+    [OutputType([int])]
+    Param
+    (
+        # Param1 help description
+        [Parameter(Mandatory=$true,
+                   ValueFromPipelineByPropertyName=$true,
+                   Position=0)]
+        [System.Management.Automation.CredentialAttribute()]
+        [pscredential]
+        $UserName,
+
+        # Param2 help description
+        [System.Management.Automation.CredentialAttribute()]
+        [pscredential]
+        $Password
+    )
+}
+
 function TestFunction($password, [PSCredential[]]$passwords, $username){
-}
+}
+
+

Tests/Rules/AvoidUserNameAndPasswordParams.tests.ps1

@@ -1,19 +1,19 @@
 Import-Module PSScriptAnalyzer
 
-$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute should be used."
+$violationMessage = "Function 'Verb-Noun' has both username and password parameters. A credential parameter of type PSCredential with a CredentialAttribute where PSCredential comes before CredentialAttribute should be used."
 $violationName = "PSAvoidUsingUserNameAndPasswordParams"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParams.ps1 | Where-Object {$_.RuleName -eq $violationName}
 $noViolations = Invoke-ScriptAnalyzer $directory\AvoidUserNameAndPasswordParamsNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
 
 Describe "AvoidUserNameAndPasswordParams" {
     Context "When there are violations" {
-        It "has 2 avoid username and password parameter violations" {
-            $violations.Count | Should Be 2
+        It "has 3 avoid username and password parameter violations" {
+            $violations.Count | Should Be 3
         }
 
         It "has the correct violation message" {
-            $violations[1].Message | Should Match $violationMessage
+            $violations[2].Message | Should Match $violationMessage
         }
     }
 

Tests/Rules/AvoidUserNameAndPasswordParamsNoViolations.ps1

@@ -19,8 +19,8 @@ function MyFunction3
         [Parameter(Mandatory=$true,
                    ValueFromPipelineByPropertyName=$true,
                    Position=0)]
-        [System.Management.Automation.CredentialAttribute()]
         [pscredential]
+        [System.Management.Automation.CredentialAttribute()]
         $UserName,
 
         # Param2 help description

Tests/Rules/PSCredentialType.ps1

@@ -1,3 +1,21 @@
 function Credential([string]$credential) {
 
+}
+
+# this one is wrong because pscredential should come first
+function Credential2
+{
+    [CmdletBinding()]
+    [Alias()]
+    [OutputType([int])]
+    Param
+    (
+        # Param1 help description
+        [Parameter(Mandatory=$true,
+                   ValueFromPipelineByPropertyName=$true,
+                   Position=0)]
+        [System.Management.Automation.CredentialAttribute()]
+        [pscredential]
+        $Credential
+    )
 }

Tests/Rules/PSCredentialType.tests.ps1

@@ -1,18 +1,18 @@
 Import-Module PSScriptAnalyzer 
-$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute."
+$violationMessage = "The Credential parameter in 'Credential' must be of the type PSCredential with CredentialAttribute where PSCredential comes before CredentialAttribute."
 $violationName = "PSUsePSCredentialType"
 $directory = Split-Path -Parent $MyInvocation.MyCommand.Path
 $violations = Invoke-ScriptAnalyzer $directory\PSCredentialType.ps1 | Where-Object {$_.RuleName -eq $violationName}
 $noViolations = Invoke-ScriptAnalyzer $directory\PSCredentialTypeNoViolations.ps1 | Where-Object {$_.RuleName -eq $violationName}
 
 Describe "PSCredentialType" {
     Context "When there are violations" {
-        It "has 1 PSCredential type violation" {
-            $violations.Count | Should Be 1
+        It "has 2 PSCredential type violation" {
+            $violations.Count | Should Be 2
         }
 
         It "has the correct description message" {
-            $violations.Message | Should Match $violationMessage
+            $violations[1].Message | Should Match $violationMessage
         }
     }
 

Tests/Rules/PSCredentialTypeNoViolations.ps1

@@ -9,8 +9,8 @@
         [Parameter(Mandatory=$true,
                    ValueFromPipelineByPropertyName=$true,
                    Position=0)]
-        [System.Management.Automation.CredentialAttribute()]
         [pscredential]
+        [System.Management.Automation.CredentialAttribute()]
         $Credential
     )
 }