3

I'm building a custom module in Powershell to factorize some code.

In the functions in the module, I use variables. However, if the caller use the same variable names, it can interfer with my module.

For example, here a small module (MyModule.psm1) :

function Get-Foo{
    param(
        [int]$x,
        [int]$y
    )

    try{
        $result = $x/$y

    } catch{
        Write-Warning "Something get wrong"
    }
    if($result -ne 0){
        Write-Host "x/y = $result"
    }
}

Export-ModuleMember -Function "Get-Foo"

And a sample script that use the module:

Import-Module "$PSScriptRoot\MyModule\MyModule.psm1" -Force

$result = 3 # some other computation

Get-Foo -x 42 -Y 0

The output is :

x/y = 3

As you can see, the caller declared a variable name that conflicts with the one in my module.

What is the best practice to avoid this behavior ?

As a requirement, I have to assume that the module's developer won't be the main script developer. Thus, the internal on the module is not supposed to be known (kinda black box)

Steve B
  • 36,818
  • 21
  • 101
  • 174
  • 1
    Even though scoping does play a role here, one problem is that a terminating error was generated. As a result, `$result` is never set as intended. Your `if` statement in its current position never considers this possibility. If you move your `if` statement inside of your `try {}` block, your main symptom does not occur. – AdminOfThings Oct 10 '19 at 11:35
  • @AdminOfThings, I was assuming that if `$result` isn't set, it will contains the default value (0 for an integer, or null for a more real scenario) – Steve B Oct 10 '19 at 12:01
  • @SteveB, referencing a non-existent variable in PowerShell by default evaluates to `$null` (if you have `Set-StrictMode -Version 1` or higher in effect, a statement-terminating error is triggered). Since `$null -ne 0` is `$true`, your `if` statement's block is entered. – mklement0 Mar 11 '20 at 09:23
  • I appreciate it, Steve. – mklement0 Mar 13 '20 at 10:38

4 Answers4

2

I'm not sure if this is actually best practice, but my way to avoid this is to always declare the variables I use (unless I specifically need to use variable from parent scope, which sometimes happen). That way you make sure you never reach the value from parent scope in your module:

# Declare
$result = $null
# Do something
$result = $x/$y

Of course in your example if seems like overkill, but in real life might be reasonable.

Another way I can think of is to change the scope.

$result => $private:result

Or to $script:result like Mike suggested.

Robert Dyjas
  • 4,979
  • 3
  • 19
  • 34
  • The "declaration" (initialization and thereby implicit creation) is a sensible approach. Instead of `$private:`, I assume you meant `$local:` to refer to a (potentially non-existent and therefore `$null`) local variable. Referring to the `$script:` scope technically works, but is conceptually confusing. – mklement0 Mar 11 '20 at 09:17
2

Ivan Mirchev's helpful answer, robdy's helpful answer and AdminOfThings's comments on the question provide the crucial pointers; let me summarize and complement them:

  • Inside your function, a local $result variable is never created if parameter variable $y contains 0, because the division by zero causes a statement-terminating error (which triggers the catch block).

  • In the absence of a local $result variable, one defined in an ancestral scope may be visible (parent scope, grandparent scope, ...), thanks to PowerShell's dynamic scoping.

    • In your case, $result was defined in the global scope, which modules see as well, so your module function saw that value.

      • However, note that assigning to $result implicitly creates a local variable by that name rather than modifying the ancestral one. Once created locally, the variable shadows the ancestral one by the same name; that is, it hides it, unless you explicitly reference it in the scope in which it was defined.
    • Also note that modules by design do not see variables from a module-external caller other than the global scope, such as if your module is called from a script.

    • See this answer for more information about scopes in PowerShell.


Solutions:

  • Initialize local variable $result at the start of your function to guarantee its existence - see below.

  • Alternatively, refer to a local variable explicitly - I mention these options primarily for the sake of completeness and to illustrate fundamental concepts, I don't think they're practical:

    • You can use scope specifier $local:, which in the case of $y being 0 will cause $local:result to refer to a non-existent variable (unless you've initialized it before the failing division), which PowerShell defaults to $null:

      • if ($null -ne $local:result) { Write-Host "x/y = $result" }

      • Caveat: If Set-StrictMode -Version 1 or higher is in effect, a reference to a non-existent variable causes a statement-terminating error (which means that the function / script as a whole will by default continue to execute at the next statement).

    • A strict-mode-independent, but verbose and slower alternative is to use the Get-Variable cmdlet to explicitly test for the existence of your local variable:

      • if (Get-Variable -ErrorAction Ignore -Scope Local result) { Write-Host "x/y = $result" }

Solution with initialization == creation of the local variable up front:

function Get-Foo{
    param(
        [int]$x,
        [int]$y
    )

    # Initialize and thereby implicitly create
    # $result as a local variable.
    $result = 0

    try{
        $result = $x/$y
    } catch{
        Write-Warning "Something get wrong"
    }
    # If the division failed, $result still has its initial value, 0
    if($result -ne 0){
        Write-Host "x/y = $result"
    }
}
mklement0
  • 382,024
  • 64
  • 607
  • 775
1

The reason for this is that you are not assigning a value to the variable result, when dividing by zero, as it generates an error. You have a variable $result in the global scope (PowerShell console), which is being inherited into the function scope (Child scope and inheritance is parent to child, not vice versa)! If you have a value, that is being assigned to the variable $result in the chatch block, for example, it could solve the issue. Something like:

function Get-Foo{
    param(
        [int]$x,
        [int]$y
    )

    try{
        $result = $x/$y

    } catch{
        Write-Warning "Something get wrong"
        $result = $_.exception.message 
    }
    if($result -ne 0){
        Write-Host "x/y = $result"
    }
}

Note: $_.exception.message = $error.Exception.message in this case

Another way would be to use the scope modifier for the variable result at the begining of the function: $global:result = $null. This way you will null the global variable (or provide other value), but then the result would be:

WARNING: Something get wrong
x/y =

Which is not really meaningful.

more details: get-help about_Scopes -ShowWindow or: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_scopes?view=powershell-6

If you have more questions, I would be happy to address.

Ivan Mirchev
  • 829
  • 5
  • 8
  • Good pointers, but I suggest not even mentioning the modification of the global variable (except to recommend against it). – mklement0 Mar 11 '20 at 09:15
1

For this you need to have a good understanding of Powershell Scope Types. There are four different types of scopes: Global Scope, Script Scope, Private Scope, Local Scope

I think you need to make use of the script scope, because these scopes are created when you run/execute a PS1 script/module. This means that you have to define the variable like:

$script:x
$script:y
Mike
  • 131
  • 1
  • 7
  • using `$script:` prefix works perfectly. However, that make the module code a bit less readable and a bit more complex to write. – Steve B Oct 10 '19 at 09:27
  • 1
    I agree. I don't like it either but we have to work with scopes in this situations. Maybe someone has a good alternative to avoid using a scopetype before every variable? Great i could help you with this :-) – Mike Oct 10 '19 at 09:32
  • Bringing the _script_ scope into the mix is unnecessary and conceptually confusing (aside from the fact that the problem relates to `$result`, not `$x` and `$y`); you can use `$local:` to refer to a local variable explicitly (`$local:result`) which if non-existent by default evaluates to `$null`) or, better yet, explicitly and initialize and thereby create the local variable of interest at the start of your function: `$result = 0`. – mklement0 Mar 11 '20 at 09:20