PSReviewUnusedParameter generates a warning parameter if we use $MyInvocation.MyCommand #1575

Activity

1. ghost added

   Needs: Triage 🔍

   on Aug 25, 2020
2. SydneyhSmith commented on Aug 25, 2020

   

   Sydney Smith (SydneyhSmith)

   on Aug 25, 2020

   Collaborator

   More actions

   Thanks RomainTiennot (@aikiox) looks like you are using a paren expression with .parameters rather than .boundparameters, it you are actually not using the values of the parameters, just the name of the parameters...the rule is written for boundparameters (see #1520), additionally the use of parens complicates things in an unexpected way for PSSA.
   In other words, the rule is behaving as expected because the value of the parameters is not being used.

   With the .parameters you will be passing along all of the parameters, with .boundparameters you will pass just those that you are using (so you will not need the exclude list).

   Let us know if we maybe missed something in your script that explains this use case. Thanks!
3. Sydney Smith (SydneyhSmith)

   added

   Resolution - Answered

   and removed

   Needs: Triage 🔍

   on Aug 25, 2020
4. rjmholt commented on Aug 25, 2020

   

   Rob Holt (rjmholt)

   on Aug 25, 2020

   Contributor

   More actions

   additionally the use of parens complicates things in an unexpected way for PSSA

   To be clear, I think there's a case to be made that not identifying BoundParameters through parens is a bug.

   With that said, PSScriptAnalyzer is a static analyser, and just cannot pick up all possible runtime occurrences. In fact there are plenty of cases where it's impossible to detect ahead of time whether behaviour is correct or not. In those situations, it's up to the user to say "I know what I'm doing" and suppress the relevant diagnostic.

   The important parts here I think that make this something the rule can't be expected to check are:

   • The use of $MyInvocation.MyCommand.Parameters, which is a static list of all parameters, rather than just the bound ones
   • The use of Get-Variable, which is a runtime/dynamic variable discovery mechanism
   • The dependency on dynamic scope with Get-Variable to look up the call stack for the right parameter value. There's no way to know in general what variables will be defined from a function's perspective due to dynamic scope, and PSScriptAnalyzer's rule just try to encourage less dependency on that (because it's hard to reason about).

   In the case of this script, I think it's fair that diagnostics are issued; generally that's a warning that your script is hard to reason about. I think an appropriate refactor would use $PSBoundParameters, which this rule is specifically implemented to recognise.
5. aikiox commented on Aug 25, 2020

   

   RomainTiennot (aikiox)

   on Aug 25, 2020

   Author

   More actions

   Hello Sydney Smith (@SydneyhSmith) and Rob Holt (@rjmholt),,
   Thank you for your very enlightening answers. Indeed, I suspect that PSScriptAnalyzer does not handle all cases and my outcome was more like "Hey, I meet case, have you thought about it or do you have another way of doing it?"
   And you answered my question :)
   I have reviewed the code and it no longer returns a warning. I did not know the existence of $PSBoundParameters which perfectly meets my need and much more as mentioned Sydney Smith (@SydneyhSmith) (the exclusion list).

   Thanks for your help and sorry for the inconvenience.
   You are doing a great job!
   Here is the modified code:

   function Format-ConditionQuery ($parameters) {
       $Condition = $null
       foreach ($Params in $parameters.GetEnumerator()) {
           if ($Params.Value) {
               $Value = $Params.Value.replace("'", "''")
               $operator = ' = '
               $conditionOperator = " where "
               if ($condition) {
                   $conditionOperator = " and "
               }
               $Condition += (" {0} [{1}] {2} '{3}' " -f $conditionOperator, $Params.key, $operator, $Value )
           }
       }
       return $condition
   }
   Function foo {
       Param
       (
           [Parameter(Position = 0)]
           $id,
           [Parameter(Position = 1)]
           $name
       )
   
       Begin {
       }
       Process {
           $Condition = ''
           $Condition = Format-ConditionQuery -parameters $PSBoundParameters
           if ($Condition) {
               $Condition
           }
           else {
               Write-Error -Message "No condition generate"
           }
       }
       End {
   
       }
   }
   
   foo -name "ok" -id 'aa'

   Thanks !
6. RomainTiennot (aikiox)

   closed this as completedon Aug 25, 2020