Skip to content

Fixes #5480: clear when re-inverting an 'in' Op#5501

Open
rdeforest wants to merge 2 commits into
jashkenas:mainfrom
rdeforest:bug-5480
Open

Fixes #5480: clear when re-inverting an 'in' Op#5501
rdeforest wants to merge 2 commits into
jashkenas:mainfrom
rdeforest:bug-5480

Conversation

@rdeforest

@rdeforest rdeforest commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

fixes #5480

Please note that tests other than the one I added are failing and were already failing. I will address those in a future PR if nobody else gets to it first.

In Op::invert in nodes.coffee, there was no reverse mechanism, so op.invert().invert() !== op. I added a check for already being inverted and removed the inversion flag. It feels like a bad code smell to use a string as a boolean, but I'm not heart to change the code style. I made the simplest, most obvious edit I could. Code style is probably a v3 thing, if such an effort were ever to come about.

This fix seems to only be relevant to switch case expressions, so I used that in the test I added. The test checks all eight permutations of subject included, value is actually in the list and expression is inverted. The bug was not triggered when the switch expression included a subject because the case expressions don't get inverted then. I didn't quite understand the logic behind inverting the case expressions, but since that part was working I didn't touch it. I separated each case into its own switch expression so that the fallthrough on match wouldn't hide any permutations.

The only changed files are nodes.coffee for the four-line fix, nodes.js after the rebuild and test/operations.coffee to add the test for this bug.

@rdeforest

rdeforest commented Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

The build error is "Error: Unable to find Node version '12.x' for platform darwin and architecture arm64." I'm guessing the rest were canceled because the first failed. I know we officially support Node back to v6, but the NodeJS project doesn't support anything earliert than v22. I know earlier releases are still available because NVM was able to find them when I tried to do my own testing.

Do I need to dig into how to obtain v12 for MacOS in the GitHub actions side of things? I'm pretty sure I don't have the access needed to do anything about fixing them, so I'd just be researching to help out.

Comment thread test/operators.coffee Outdated
Comment on lines +469 to +476
switch "with subject"
when needle in hayStack then ok false
switch "with subject"
when needle not in hayStack then ok true
switch "with subject"
when needle in needleStack then ok true
switch "with subject"
when needle not in needleStack then ok false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these switch constructs always hit the default branch because the string is never a boolean

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I updated the tests by adding "and true" after all the strings.

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.

Bug: using "not in" operator in switch works as "in" instead

2 participants