C#: Bugfix for nullguards for complex patterns.

[aschackmull]

Patterns like x is (string or null) and x is not string were being identified as null guards in the wrong branch.

[Copilot]

Pull Request Overview

This PR fixes a bug in the C# code analysis library where pattern matching expressions were incorrectly identifying null guards in the wrong branches. The fix addresses complex patterns like x is (string or null) and x is not string that were being analyzed incorrectly.

Key changes:

• Introduces a new helper function patternContainsNull to properly evaluate whether a pattern matches null values
• Replaces simple null literal checks with comprehensive pattern analysis that handles complex nested patterns
• Fixes the logic for determining which branch represents a null guard condition

[michaelnebel]

Thank you for doing this!

• Added some questions/comments.
• It looks like some tests are failing.
• We should probably run DCA.

[michaelnebel]

The name might be a bit misleading at the pattern can have "mentions" (and thus containing) null without matching null. Maybe rename to matchesNull.

[aschackmull]

Yeah, I was thinking "contains" in the sense that the pattern represents a set of values. But yes, I'll change it, patternMatchesNull is better.

[michaelnebel]

Maybe consider elaborating the comment above.

Do we need the dual

// E.g. `x is null` or `x is null or ...`
branch = patternContainsNull(pm.getPattern()) and
isNull = true

[aschackmull]

No. Just because a pattern will match null doesn't mean that it's the only value that it'll match, so we can't conclude that the value is null based on a match or no-match branch edge (unless the pattern is literally null). I guess we could add another case for x is not null, which does imply x == null in its false branch, but I think I'll postpone that particular tweak to my work on instantiating Guards, because then we can handle those cases in slightly more generality.

[michaelnebel]

Ah yes, that is of course correct.

There is some overlap in this disjunct with

// E.g. `x is null`
pm.getPattern() instanceof NullLiteral and
isNull = branch

should the above be changed to only handle isNull = branch = true as a special case?

[aschackmull]

True, but probably doesn't matter. Regardless, this is just a stepping stone - I'm working on a shared Guards instantiation in a separate branch where this will be tweaked anyway.

[michaelnebel]

Excellent! Once again, thank you!

[aschackmull]

• We should probably run DCA.

Already done, we get some nice FP reduction. I'll add a change note describing that.

[michaelnebel]

LGTM!