Skip to content

Conversation

actuallymentor
Copy link

@actuallymentor actuallymentor commented Jun 25, 2025

This might be what was causing null values still causing matches.

Description

Boolean logic issue where || null is not parsed as "is not undefined or null" but "is not undefined" as || null always evaluates to "or is false" which is always false.

Related issue

Potentially fixes issues like #1406.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my code.
  • I have linted and formatted my code.
  • My changes generate no new warnings.
  • New and existing unit tests pass locally with my changes.

Additional context

Thank you very much for spending time on this excellent piece of software. You've saved me so much time 🙏

@actuallymentor
Copy link
Author

Hey @jorenn92! Please let me know if you need any more info to merge this PR. Happy to amend if needed.

@benscobie
Copy link
Collaborator

This is indeed a bug but 'fixing' it would require a major version bump due to the logic change and I'm not sure we want to do that right now.

I removed the null condition completely in #1705 - with the intention to provide proper NULL handling in the future.

@actuallymentor
Copy link
Author

All good. Can I help with the null handling? I was running into issues with it which caused me to find this

@benscobie
Copy link
Collaborator

@actuallymentor Let's see how well Copilot does with this: #1894

Copilot AI added a commit that referenced this pull request Jul 24, 2025
- Fixed boolean logic bug from PR #1857 (line 201-203)
- Added IS_NULL and IS_NOT_NULL operators to RulePossibility enum
- Created NULL rule type for null-specific operations
- Updated rule comparison logic to handle null values appropriately
- Added comprehensive tests covering all null handling scenarios
- Maintained backwards compatibility with existing rules

Co-authored-by: benscobie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants