Simplify string normalization - don't convert numeric strings to numbers #46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a simpler fix for the string comparison issues. Instead of the complex `extract_string_compared_variables` mechanism that tried to selectively skip normalization, we now simply don't convert numeric strings to numbers at all. Changes: - Modified `normalize_variables` to only convert "true"/"false" strings to booleans, leaving all other strings as strings - Modified `normalize_ast_variables` similarly for AST atoms - Removed `extract_string_compared_variables` and related functions - Removed `is_number` and `is_atom_number` functions - Removed `skip_normalization` parameter threading through the code This fixes: - `device.appBuildString == "5690201"` now works correctly - `device.appVersionPadded > "007.003.001"` still works for version comparisons - Version strings like "009.000" stay as strings for lexicographic comparison The fix is simpler and more predictable: quoted strings stay as strings. If users want a number, they should write it without quotes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
yusuftor
force-pushed
the
feature/fix-string-to-int
branch
from
9675c37 to
106601f
Compare
yusuftor
changed the title
When comparing a value to a numeric string (e.g., "1" or "1.23"), the comparison now matches both the string and numeric representations. For example: - `x == "1"` becomes `(x == "1") || (x == 1)` - `x != "1"` becomes `(x != "1") && (x != 1)` This allows: - `1 == "1"` → true (int matches numeric string) - `1.23 == "1.23"` → true (float matches numeric string) - `"5690201" == "5690201"` → true (string matches string) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
Author
Updated: Type coercion for numeric string comparisonsThis commit adds type coercion for equality comparisons with numeric strings. What changed?When comparing with a numeric string (like Why?Two scenarios now work correctly:
Test results
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
extract_string_compared_variablesmechanism in favor of this simpler approachWhat was the issue?
device.appBuildString == "5690201"was returningfalseeven whendevice.appBuildStringwas the string"5690201". The previous fix tried to selectively skip normalization usingextract_string_compared_variables, but this was complex and had edge cases.The fix
Instead of selective normalization, we now simply don't convert numeric strings to numbers. Quoted strings stay as strings - if users want a number, they write it without quotes.
Test plan
cargo test --lib- all 107 tests passdevice.appBuildString == "5690201"returnstruewhen variable is string"5690201"device.appVersionPadded > "007.003.001")🤖 Generated with Claude Code