Fix JSONValue’s interpretation of 0 and 1
#194
Conversation
It was incorrectly interpreting them as booleans. Mistake in 80e8585.
WalkthroughThis pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🔇 Additional comments (5)Sources/AblyChat/JSONValue.swift (1)
The implementation correctly uses identity comparison with CoreFoundation boolean constants to distinguish between boolean and numeric NSNumber values. This is the recommended approach as discussed in the Swift forums. Let's verify that this fix addresses all cases where NSNumber values might be misinterpreted: ✅ Verification successfulThe boolean/number distinction fix is correctly implemented and well-tested The verification shows that:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Search for any other potential NSNumber conversions in the codebase that might need similar fixes
rg -g '*.swift' 'NSNumber.*as.*Bool|Bool.*as.*NSNumber'
Length of output: 57 Script: #!/bin/bash
# Let's search for NSNumber usage patterns more broadly
rg -g '*.swift' 'NSNumber' -A 2 -B 2
Length of output: 3895 Script: #!/bin/bash
# Let's also check for any other JSONValue initialization from NSNumber
ast-grep --pattern 'init($$$) {
$$$
if $number === kCFBooleanTrue {
$$$
}
$$$
}'
Length of output: 94 Script: #!/bin/bash
# Search for any other boolean conversion patterns
rg -g '*.swift' 'kCFBoolean(True|False)'
Length of output: 202 Tests/AblyChatTests/JSONValueTests.swift (4)
The added test cases properly verify that NSNumber values 0 and 1 are correctly interpreted as numbers rather than booleans.
The end-to-end test properly includes the edge cases of 0 and 1 in the JSON structure, ensuring they're correctly deserialized. Also applies to: 60-61
Test cases verify that numeric 0 and 1 values are correctly serialized to NSNumber instances rather than being converted to booleans.
The end-to-end serialization test properly includes the edge cases of 0 and 1, ensuring they're correctly handled throughout the entire serialization process. Also applies to: 133-134 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
lawrence-forooghian
deleted the
fix-JSONValue-interpretation-of-0-and-1
branch
It was incorrectly interpreting them as booleans. Mistake in 80e8585.
Summary by CodeRabbit
New Features
Bug Fixes
Tests