Update PossibleIncorrectComparisonWithNull documentation with better example

[bergmeister]

PR Summary

Fixes comments received in issue #1227 after it was closed

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Change is not breaking
• Make sure all .cs, .ps1 and .psm1 files have the correct copyright header
• NA Make sure you've added a new test if existing tests do not effectively test the code changed and/or updated documentation
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: to the beginning of the title and remove the prefix when the PR is ready.

[rjmholt]

Is this just a repetition of the line above? I think the existing examples make sense and are illustrative of the difference between a naive comparison with $null and the suggested fix.

It might possibly be worth a more complicated example, but the purpose of the documentation here really isn't to teach people PowerShell concepts.

[bergmeister]

The point of changing the example was that the existing example was too confusing for people because it changed 2 things at once, the example in this PR changes only 1 thing (eq to ne) to demonstrate the behaviour that might be unexpected for some people (yes, I know this is how the comparison operator works but most people do not use it with this intention and rather just want a null checks)

[rjmholt]

Two points there:

1. Unrelated to the example complexity discussion: it looks like this change just uses the same example twice. Is that intended? It seems unhelpful (and arguably confusing) to have the same example twice. Maybe I've missed something.

2. I don't think taking an example away prevents confusion. I think adding another simpler example as a stepping stone will be much more helpful. In the original issue this PR addresses, I don't think the issue was "the current examples are confusing" so much as "the current examples don't look like what I have"; @PrzemyslawKlys understood the behaviour of the -eq comparison, but not why he was being warned. So I think the answer is more examples and some discussion about the effects, and what to do if you really do intend null comparison like this.

[bergmeister]

At first, yes it looks like it is the same example twice (if one already understands the root cause) but the point of the example is to also show on a high level (if the LHS and RHS were a complex, unknown expression) that changing -eq to -ne does not return in inverting the result, which is basic assumption and thereby shows in what kind of tricky situations one can land

[rjmholt]

Oh I honestly missed the operator change. Without trying to blame my own failing on an extrinsic factor, I suspect others might also miss the important difference.

Maybe it's worth breaking up the comment so there's one for each example, like:

# In PowerShell, `$array -operator $value` works the same as `$array | Where-Object -operator $value` because of pipeline enumeration
# For example:
@(1, 2, 3, 4) -ge 3             # Emits `3, 4`
'a','b','c','ab' -like 'a*'     # Emits 'a','ab'
'hi',$null,'bye' -ne $null      # Emits 'hi','bye'
@() -ne $null`                  # Emits nothing (an empty array)

# Equality comparison doesn't work as often intended, because `@() -ne $null` results in a falsey empty value
if (@() -ne $null) { 'true' } else { 'false' }     # Emits 'false'

# Comparison misleadingly appears to work as intended with -eq, but again because a falsey empty value is emitted (not because the `@()` value is non-null)
if (@() -eq $null) { 'true' } else { 'false' }     # Emits 'false'

# Because of this caveat, PSScriptAnalyzer warns about comparisons with $null by default
# If you want to use a filter behaviour, it's recommended to explicitly filter instead:

# Don't use
if ($array -ne $null) { ... }

# DO use
if ($array | Where-Object -ne $null) { ... }

[bergmeister]

Yee, I agree, maybe it is best to have an additional section with more examples so people can go deep and look at different scenarios and root causes, please open a PR for it since this PR got already merged. It makes me wonder if PowerShell would benefit e.g. from a == operator that would be more intuitive than the -eq operator..

[JamesWTruher]

the text is fine; the missing space should get fixed.

[bergmeister]

I applied Rob's suggestion about the whitespace and added Jim's example as well

[SuperflyJon]

What seems odd is that swapping the $null in the example, i.e.:
$null -eq @() # returns $false
$null -ne @() # returns $true
doesn't make much sense either?

More importantly if a user wanted to check for $null in an array then you have to write this with the $null on the right of the equals test.

I'm not convinced this rule has any value - maybe I'm missing something?