Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AvoidDefaultValueForMandatoryParameter: The documentation is wrong #876

Closed
LaurentDardenne opened this issue Feb 7, 2018 · 4 comments · Fixed by #907
Closed

AvoidDefaultValueForMandatoryParameter: The documentation is wrong #876

LaurentDardenne opened this issue Feb 7, 2018 · 4 comments · Fixed by #907

Comments

@LaurentDardenne
Copy link

The documentation does not match the implementation, it appears to be the documentation of an old rule (PSProvideDefaultParameterValue) that has been removed and this one is always quoted in the documentation

See also this discussion around this rule.

I do not understand why the presence of the cmdletbinding attribute influences the rule.
Setting a default value to a mandatory parameter does not make sense in all cases.
From this test :

badfunc

cmdlet BadFunc at command pipeline position 1
Supply values for the following parameters:
Param1:
BadFunc : Cannot validate argument on parameter 'Param1'. The argument is null or empty. Provide an argument that is not null or empty, and then try the command again.

#---
goodfunc2

cmdlet GoodFunc2 at command pipeline position 1
Supply values for the following parameters:
Param1:
GoodFunc2 : Cannot validate argument on parameter 'Param1'. The argument is null or empty. Provide an argument that is not null or empty, and then try the command again.

Or this:

$sb={
 param (
  [Parameter(Mandatory=$true)]
  $Param1="String"
 )
 
 $param1
}
&$sb
cmdlet  at command pipeline position 1
Supply values for the following parameters:
Param1:
Cannot validate argument on parameter 'Param1'. The argument is null or empty. Provide an argument that is not null or empty, and then try the command again.

Note that the error message is about cmdlet ...

Maybe the presence of the cmdletbinding attribute is another recommendation: Always Start With CmdletBinding

@LaurentDardenne
Copy link
Author

In the mentioned test file, for the following function :

function BadFunc
{
    [CmdletBinding()]
    param(
        # this one has default value
        [Parameter(Mandatory=$true)]
        [ValidateNotNullOrEmpty()]    
        [string]
        $Param1="String",
        # this parameter has no default value
        [Parameter(Mandatory=$false)]
        [ValidateNotNullOrEmpty()]    
        [string]
        $Param2
    )
    $Param1
    $Param1 = "test"
}

the parameter Param2 :

  # this parameter has no default value
  [Parameter(Mandatory=$false)]
  [ValidateNotNullOrEmpty()]    
  [string]
  $Param2

should be it seems to me like this:

  # this parameter has default value but it is not mandatory  
  [Parameter(Mandatory=$false)]
  [ValidateNotNullOrEmpty()]    
  [string]
  $Param2='Test'

And add a third use case:

  # this parameter has no default value and it is mandatory  
  [Parameter(Mandatory=$true)]
  [ValidateNotNullOrEmpty()]    
  [string]
  $Param3

@bergmeister
Copy link
Collaborator

@LaurentDardenne Thanks for the investigation and detailed report. Feel free to open a PR with a fix. I think we can also improve the implementation, I don't understand either why the author decided to trigger it only when using cmdletbinding.

@LaurentDardenne
Copy link
Author

I think we can also improve the implementation,
For my part, I think it's a functional bug.

Feel free to open a PR with a fix.
Not right now, sorry.

@bergmeister
Copy link
Collaborator

@LaurentDardenne After carefully going through all links, I agree now with you that the rule should warn irrespective if [CmdletBinding()] is used or not. Some of the existing tests also need enhancement. And yes, you are right, the documentation should be updated as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants