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

I can use $null value with the 'DependsOn' parameter, but not an array that contains $null value : #191

Closed
LaurentDardenne opened this issue Jun 18, 2019 · 3 comments · Fixed by #205
Labels
bug Something isn't working
Milestone

Comments

@LaurentDardenne
Copy link

Use case: I declare a dependency using a variable that contains $null :

$Path='C:\temp'
$File='Test.Rule.ps1'
@'
$name=$null
Rule Main -DependsOn 'Dependency1' {
    Write-warning 'Main'
    $True
} 

Rule Dependency1 -DependsOn $name {
    Write-warning 'Dependency1'
    $true
}
'@ >"$Path\$File"

cd $path
'object'|Invoke-psrule -path "$path\$File"

# AVERTISSEMENT : Dependency1
# AVERTISSEMENT : Main


#    TargetName : 5b863ffbc007f3c294c6739f6359359cfd3db547

# RuleName                            Outcome    Recommendation
# --------                            -------    --------------
# Dependency1                         Pass
# Main                                Pass

No error.
Is it by design that $null can be passed to the 'DependsOn' parameter?

Use case: I declare a dependency using a variable that contains an array of one element that has the value $null:

@'
$name=@($null)
Rule Main -DependsOn 'Dependency1' {
    Write-warning 'main'
    $True
} 

Rule Dependency1 -DependsOn $name {
    Write-warning 'Dependency1'
    $true
}

Rule Dependency2   { 
    Write-warning 'Dependency2'
    $false
}
'@ >"$Path\$File"

'object'|Invoke-psrule -path "$path\$File"
# Exception lors de l'appel de « Build » avec « 0 » argument(s) : « La valeur ne peut pas être null.
# Nom du paramètre : key »
# Au caractère C:\Prive\Modules\psrule\0.7.0\PSRule.psm1:163 : 9
# +         $pipeline = $builder.Build();
# +         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#     + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
#     + FullyQualifiedErrorId : ArgumentNullException

# Impossible d’extraire la variable « $pipeline », car elle n’a pas été définie.
# Au caractère C:\Prive\Modules\psrule\0.7.0\PSRule.psm1:164 : 9
# +         $pipeline.Begin();
# +         ~~~~~~~~~
#     + CategoryInfo          : InvalidOperation : (pipeline:String) [], RuntimeException
#     + FullyQualifiedErrorId : VariableIsUndefined

Invoke-PsRrule triggers an expected exception.
In my opinion, to prohibit the value $null in this case does not facilitate the memorization of the rules of behavior of this parameter.

Use case: I declare a dependency that does not exist.

@'
$name=@($null,'Dependency2')
Rule Main -DependsOn 'Dependency1' {
    Write-warning 'main'
    $True
} 

Rule Dependency1 -DependsOn NotExist {
    Write-warning 'Dependency1'
    $true
}

Rule Dependency2   { 
    Write-warning 'Dependency2'
    $false
}
'@ >"$Path\$File"

'object'|Invoke-psrule -path "$path\$File"
# Exception lors de l'appel de « Build » avec « 0 » argument(s) : « Dependency name does not exist in index »
# Au caractère C:\Prive\Modules\psrule\0.7.0\PSRule.psm1:163 : 9
# +         $pipeline = $builder.Build();
# +         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#     + CategoryInfo          : NotSpecified: (:) [], MethodInvocationException
#     + FullyQualifiedErrorId : Exception

# Impossible d’extraire la variable « $pipeline », car elle n’a pas été définie.
# Au caractère C:\Prive\Modules\psrule\0.7.0\PSRule.psm1:164 : 9
# +         $pipeline.Begin();
# +         ~~~~~~~~~
#     + CategoryInfo          : InvalidOperation : (pipeline:String) [], RuntimeException
#     + FullyQualifiedErrorId : VariableIsUndefined

Here the error message should indicate which rule declares a non-existent dependency and indicate the name of this non-existent dependency.
This would facilitate the rules debug.

@BernieWhite
Copy link
Member

BernieWhite commented Jun 18, 2019

@LaurentDardenne Fair points. For use case 1, what do you expect the behaviour to be?

For use case 2, agree.

  • Improve error messages for missing dependencies.

@BernieWhite BernieWhite added the enhancement New feature or request label Jun 18, 2019
@LaurentDardenne
Copy link
Author

For use case 1, what do you expect the behaviour to be?

To leave as-is is possible but it is necessary to know this behavior.
And in this case, it is up to the user to make sure not to assign $null:

$Name-Get-MyDependency -Context xyz
if ($Null -eq $Name)
{Throw '$Name is $null'}
Rule Dependency1 -DependsOn $Name {
    ...
}

Hard to say, we could also want to cancel a dependency by assigning, by design, $null to $Name.

What I found 'strange' is to accept $null in one case but not in the other.

@BernieWhite
Copy link
Member

BernieWhite commented Jun 19, 2019

@LaurentDardenne Thanks for your feedback.

As you mentioned, it is strange as this behaviour isn't commonly used in PowerShell. While in some cases it is required, in this cases it probably does more harm then good, and it wasn't orignally intended.

I think allowing a $Null DependsOn to be specified confuses the intent and opens the door for a logic error.

If there was a need to make a dependency optional in code, which seems unlikely, splating the DependsOn parameter would still be possible.

@BernieWhite BernieWhite added bug Something isn't working and removed enhancement New feature or request labels Jun 19, 2019
@BernieWhite BernieWhite added this to the v0.7.0 milestone Jun 19, 2019
BernieWhite added a commit that referenced this issue Jun 20, 2019
BernieWhite added a commit that referenced this issue Jun 21, 2019
…lliseconds #191 #192 (#205)

* Fix null DependsOn parameter #191

* Exclude VS test results

* Record rule time in milliseconds

* Add documentation on PSRule features #68

* Update change log #192
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants