The previous implementation intercepted `n / 0` and `n % 0` and returned
`.null`, on the rationale that `null` is friendlier for rule authors and
matches the engine's "missing value" convention. That's the last
remaining intentional deviation from `json-logic-js` in this file —
remove it for full spec parity.
`json-logic-js` delegates `/` and `%` to native JS arithmetic, which
follows IEEE 754: `n / 0` is `±Infinity` (sign matches the dividend),
`0 / 0` is `NaN`, and any `n % 0` is `NaN`. Swift's `Double` operators
and `truncatingRemainder(dividingBy:)` already produce these IEEE
values, so the fix is just dropping the divisor==0 short-circuit and
wrapping the result in `.float`.
Tests:
- Replace `testDivByZeroReturnsNull` with `testDivByZeroFollowsIeee754`
pinning all three cases (`+Infinity`, `-Infinity`, `NaN`).
- Replace `testModByZeroReturnsNull` with `testModByZeroIsNan` covering
`7 % 0` and `0 % 0`.
- Update the `1 / null` / `1 % null` cases inside
`testSubDivAndModUseToNumberPerSpec` (divisor coerces to 0 →
`+Infinity` / `NaN` instead of `.null`).
Co-authored-by: Cursor <cursoragent@cursor.com>
`json-logic-js` is asymmetric about which JS coercion arithmetic uses:
`+` and `*` go through `parseFloat(value)` (stringify, parse longest
numeric prefix); `-`, `/`, `%` go through native arithmetic which calls
`Number(value)` (a.k.a. `ToNumber`). The prior implementation routed
every operand through `Value.asNumber` (a partial `ToNumber`), which
diverged from the spec in two directions:
- For `+` / `*`: `null`, bools, the empty string, and `[1]` produced
numbers instead of `NaN`.
- For `-` / `/` / `%`: arrays produced `NaN` instead of being coerced
via `ToPrimitive("number")` → `toString` → recurse, so `[] - 1` and
`[1] - 1` were `NaN` instead of `-1` and `0`.
Add `jsString(value)` (top-level JS `String()`) and `jsParseFloat(value)`
to `Value.swift`, extend `Value.asNumber` to coerce arrays/objects via
`jsString`, and route `+` / `*` through `jsParseFloat` while leaving
`-` / `/` / `%` on `asNumber ?? .nan`. The existing
divide-/modulo-by-zero short-circuit (`.null` instead of `±Infinity` /
`NaN`) is the only remaining intentional deviation in this file and is
documented as such.
Tests:
- Replace `testNullOperandIsTreatedAsZero` with two coercion-path tests
that pin both branches against the spec, including the array cases
(`[1] + 1 = 2`, `[1,2] + 0 = 1`, `[] - 1 = -1`, `[1] - 1 = 0`,
`[1,2] - 0 = NaN`).
- Update `testAddOneArgActsAsNumericCast` and split
`testAddCoercesStringsAndBools` into `testAddCoercesNumericStrings`
to reflect that bools no longer bridge through `+` / `*`
(`parseFloat("true") === NaN`) but trailing junk in numeric strings
now parses (`parseFloat("3.14abc") === 3.14`).
Co-authored-by: Cursor <cursoragent@cursor.com>
`opMissing` only flagged keys whose path failed to resolve. The
json-logic-js reference routes through `var` and treats any key whose
lookup resolves to `null` (key absent OR leaf is `null`) or to the
empty string as missing — so `{"missing": ["country"]}` against
`{"country": null}` is supposed to return `["country"]`, but our impl
returned `[]`. Backend payloads regularly come down with explicitly
cleared fields as `null`, so the divergence was reachable from real
predicates.
`opMissing` now uses the same lookup `var` does (without the
missing-variable warning, since `missing` is a check, not a read) and
matches against the spec's `value === null || value === ""` test.
Falsy non-empty values (`0`, `false`, `[]`) are NOT missing — pinned
by a regression test so a future truthiness-based simplification can't
silently flip them.
Reported by Cursor Bugbot:
#6789 (comment)
Co-authored-by: Cursor <cursoragent@cursor.com>
Brings in:
- The `RulesEngine` → `RulesEngineInternal` module rename from #6787.
- The `no_leaking_rules_engine_internal_import` SwiftLint rule update from #6788.
- Plus the `main`-merge cascade carried by #6788.
Conflicts resolved:
- `RulesEngineInternal/RulesEngineInternal.swift`: kept this branch's
`Rules.logger` / `LoggerStorage` storage, dropped the
`@_spi(Internal)` marker on `public enum Rules` (since #6787 dropped
those markers; the new SwiftLint rule replaces that gate).
- `RevenueCat.xcodeproj/project.pbxproj`: kept this branch's full file
list (Evaluator, Logger, Value, RuleError, Operators/*, plus the new
test files and helpers), then applied the producer-side rename to
every `RulesEngine\b` / `RulesEngineTests\b` token (target name,
group name, path, productName, bundle IDs `com.revenuecat.RulesEngineInternal[Tests]`,
framework / xctest output names, and the `RulesEngine.swift` → `RulesEngineInternal.swift`
file ref).
Other rename follow-ups in this branch's added files:
- `Tests/RulesEngineInternalTests/*.swift` (8 files) — every
`[@_spi(Internal) ]@testable import RulesEngine` updated to
`@testable import RulesEngineInternal`. The `@_spi(Internal)`
attribute is no longer needed because the producer no longer uses
SPI markers.
- `Projects/RulesEngineInternal/`, `RulesEngineInternal/`, and
`Tests/RulesEngineInternalTests/` directories were moved
automatically by git's directory-rename detection (15 file renames).
Left intentionally unchanged:
- `RulesEngineInternal/Logger.swift` — the development-only
`PrintLogger` keeps its `[RulesEngine]` log prefix because that
string identifies the subsystem (the rules engine) for log readers,
not the producer-side module name. The injected production logger
replaces this struct entirely once the engine is wired into the SDK,
so the prefix is throwaway.
- `RulesEngineLogger` protocol name — internal type name within the
module, not part of the import-site signal that motivated the
module rename.
- The `Rules` namespace name — separate identifier from the module
name, kept stable so call sites still write `Rules.something`.
Verified locally:
- `swift build --target RulesEngineInternal` ✔
- `swift test --filter RulesEngineInternalTests` ✔ — 75 tests passed
- `xcodebuild -scheme RulesEngineInternal -destination 'generic/platform=iOS' build` ✔
- `swiftlint --no-cache` ✔ — 0 violations across 1332 files
- `git ls-files | xargs perl -ne 'if (/\bRulesEngine\b/ && !/RulesEngineInternal/) { ... }'` returns only the `[RulesEngine]` log prefix mentioned above.
Co-authored-by: Cursor <cursoragent@cursor.com>
