Psscriptanalyzer: $null should be on left side of equality comparison

[PrzemyslawKlys]

Before submitting a bug report:

• Make sure you are able to repro it on the latest released version
• Perform a quick search for existing issues to check if this bug has already been reported

Steps to reproduce

function Test {
param (
[string[]] $ListData = $null
)
if ($ListData -eq $null) {
}
}

Expected behavior

No warnings

Actual behavior

$null should be on left side of equality comparison

If an unexpected error was thrown then please report the full error details using e.g. $error[0] | Select-Object *

Environment data

> $PSVersionTable


Name                           Value
----                           -----
PSVersion                      5.1.17672.1000
PSEdition                      Desktop
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
BuildVersion                   10.0.17672.1000
CLRVersion                     4.0.30319.42000
WSManStackVersion              3.0
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1

> (Get-Module -ListAvailable PSScriptAnalyzer).Version | ForEach-Object { $_.ToString() }

1.17.1

[bergmeister]

@PrzemyslawKlys Thanks for taking the time to submit the issue with details, I am always up for discussing things but my thinking is that this behaviour is ok and by design, especially since the variable on the left hand side can not be $null when a parameter value is supplied. The rule complains when $null is not on the left hand side, therefore I do not understand why your expected behaviour would be not have any warnings? Maybe you can elaborate on that?
I can understand where you are coming from if there was a situation where one knew that the variable on the LHS was always $null but PSSA does not do analysis of the value of the variable itself in general. Even if it did that (which would either difficult to implement or minimalistic), then it would not improve the rule.
The point of the rule is that when comparing something against $null then $null should be on the left hand side to avoid unexpected behaviour. Therefore it does not matter what the value of the other variable actually is (and this could change in different execution contexts or scopes anyway), therefore the rule only looks at $null, which is a constant, to make sure it is always on the left hand side for comparison.
Does that make sense?

[PrzemyslawKlys]

What I do with the functions is that i assign $null to the param by default so that if no parameters are provided I will terminate function by using return. The message I got, gets me confused as it's not describing it the way you do now. While in this case I could probably skip it as it wouldn't go thru foreach but for standard string I need to check what it holds to not provide that $empty string into other function which will for example write to registry. Unless there is 'better way' to do that then I am up for game..

[bergmeister]

@PrzemyslawKlys
When you run PSScriptAnalyzer, it tells you the line it complains about, it is not complaining about the parameter being $null but the way how you compare the variable against $null. I am not sure if I understand your problem. Let me therefore show how I would fix it:

Invoke-ScriptAnalyzer $reproFile

RuleName                            Severity     ScriptName Line  Message
--------                            --------     ---------- ----  -------
PSPossibleIncorrectComparisonWithNu Warning      a.ps1      5     $null should be on the left side of equality
ll                                                                comparisons.

The correct way would therefore be to fix the warning by having $null on the left hand side:

function Test {
param (
[string[]] $ListData = $null
)
if ($null -eq $ListData) {

}
}

Does this make sense or did I miss something?

[PrzemyslawKlys]

It makes sense but it means I've been testing variables wrong my whole life... I always thought variable name goes on the left, and value goes on the right. In my case I assumed $null is my value.

[bergmeister]

@PrzemyslawKlys That is the point of this rule, see rule documentation here. I always like to give the following example why $null should not be on the right hand side in PowerShell:

if (@() -eq $null) { 'true' }else { 'false' }
if (@() -ne $null) { 'true' }else { 'false' }

There are multiple reasons for using $null on the LHS but most boil down to how the comparison operator was designed for, which is not very intuitive and in many cases not the desired outcome.

[PrzemyslawKlys]

No problem. Thank you for explanation. You learn something new every day. I guess there's a lot of scripts I've used it wrong ;-)

[bergmeister]

Well, it does not always mean that you were using it wrong, in the sense that it always resulted in wrong results but having $null on the left hand side brings you in a position where you are safe from nasty special cases.

[avelis26]

And Microsoft continues to lose intelligence ... shm

I know TECHNICALLY these are the same to a computer:
If ($var -eq $null) {function}
If ($null -eq $var) {function}

But HUMANS tend to think more like this:
If the wall is wider than 50 inches then ...
Whereas THIS is just plain retarded:
If 50 inches is less than the width of the wall then ...

Every time VS Code updates, I find another issue that is "by design" according to you idiots. Pro tip... if you have people reporting it as a BUG... then it your design sucks!

[SteveL-MSFT]

@avelis26 did you read the documentation linked above explaining the reasoning? The rule is about a specific use case.

I appreciate you providing your opinion, however, I would also appreciate it if you could have worded it a bit more constructive and not so negative. Keep in mind that we are also humans and many contributors here are not even Microsoft employees.

[DillonFlohr]

@avelis26 I agree with you that it is strange that Powershell handles null comparison like this. This is not a problem that PSScriptAnalyzer can fix though. It is just providing the best suggestion based on how Powershell works. Perhaps there is a way we can bring up this oddity with the Powershell developers or it could possibly be being discussed already.

[ducttapecoder-vt]

This is an interesting special case but the examples on https://github.com/PowerShell/PSScriptAnalyzer/blob/development/RuleDocumentation/PossibleIncorrectComparisonWithNull.md seem to be written poorly: swapping the -eq and -ne while _also_ swapping the left/right seems to be a bad comparison.

In case anyone is still confused, the evaluations

if (@() -eq $null) { 'true' } else { 'false' }  # Returns false
if (@() -ne $null) { 'true' } else { 'false' }  # Returns false

both return false. If you think about it, how could it be both equal to $null and _not_ equal to $null?
Meanwhile

if ($null -eq @()) { 'true' } else { 'false' }  # Returns false
if ($null -ne @()) { 'true' } else { 'false' }  # Returns true

[bergmeister]

Hmm, @ducttapecoder-vt I agree that your first example (which I actually gave a few comments earlier here) would be better suited for the documentation example, feel free to open a PR

[BennyClauss]

Sorry, but from the view of an tester. There are implicit and explecit requirements. I think that the meaning of equality (implicit req.) has to be like in the language, in math and in logical expressions. The equality has to be like: a=b <=> b=a. If i where the tester on the PowerShell development project, i would say the test failed, because the operator (techn.) is not äquivalent to the reality (logical).

[galvanopus]

I always like to give the following example why $null should not be on the left hand side in PowerShell:

if (@() -eq $null) { 'true' }else { 'false' }
if (@() -ne $null) { 'true' }else { 'false' }

@bergmeister Could you confirm this example shows whether $null should be or should not be on the LHS?

[bergmeister]

@galvanopus Yes

[galvanopus]

@bergmeister Sorry, could you clarify whether $null should be or should not be on the LHS? You told it should not be previously but I think there you wanted to say the opposite one.

[bergmeister]

The point of the rule is that $null should always be on the left hand side to avoid unexpected results, the example that I gave was to show what could happen if it is not on the left hand side.

[galvanopus]

Thanks. Would you like to edit https://github.com/PowerShell/PSScriptAnalyzer/issues/1021#issuecomment-396404723 from

I always like to give the following example why $null should not be on the left hand side in PowerShell

to

I always like to give the following example why $null should be on the left hand side in PowerShell

to decrease future misunderstandings?

[bergmeister]

Ok, thanks, I edited the typo