fix: getValue/inDayTimeRange/toNum — 3 glm-hunt bugs (#90 #91 #92) - #93
Merged
Conversation
…nsistent, toNum() cast through numberType Fix three glm-hunt bugs in CalculationContext / P4TypedAstEvaluator: #90: AbstractCalculationContext.getValue() did an unchecked (Float) cast on valueByName, so set(String, Number) with Double/Integer/BigInteger etc. caused ClassCastException on read. Convert via Number.floatValue() when the stored value is not already a Float; Float values are preserved as-is. #91: P4TypedAstEvaluator.evalToNumExpr() returned a raw Double and cast the default value via (Number), causing ClassCastException for non-Number defaults (e.g. toNum('abc','xyz')) and ignoring the configured numberType. Route both the parsed value and the default through castToNumberType so the result honors numberType, and fall back to 0.0 when the default is not a Number instead of throwing. #92: AbstractCalculationContext.inDayTimeRange() returned false for a same-day range with fromHour > toHour (a midnight-spanning range), inconsistent with EmbeddedFunction.inTimeRange(). When fromHour > toHour on the same day, evaluate as a midnight span (nowHour >= fromHour || nowHour < toHour), matching inTimeRange(). Each bug is covered by a dedicated regression test (Issue90/91/92*Test). Full test suite passes (703 tests, 0 failures).
Uh oh!
There was an error while loading. Please reload this page.
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
Sign up for freeto 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
Fixes three
[glm-hunt]issues reported in the previous stage. Each is a small, independent, reversible bug fix; bundled into one PR because the touch points do not overlap and splitting would only add CI/merge overhead.Closes#90
Closes#91
Closes#92
Why one PR
The three bugs live in two files (
AbstractCalculationContext.java,P4TypedAstEvaluator.java) and do not interact. Each fix is minimal and locally reversible (revert the relevant hunk only). Bundling avoids three CI runs and three merge commits for what is effectively a single bug-hunt pass.Changes
#90 —
getValue()ClassCastException on non-Float NumberAbstractCalculationContext.getValue(String)did(Float) valueByName.get(name), butset(String, Number)stores arbitraryNumbersubtypes (Double/Integer/BigInteger...). Reading such a value viagetValuethrewClassCastException, affecting e.g.inTimeRange/getValue("nowHour")after aset(String, Number)declaration.Fix: read as
Number; if not alreadyFloat, convert vianumber.floatValue().Floatvalues are preserved exactly (no extra boxing). Empty map →Optional.empty()as before.#91 —
toNum()ClassCastException + missingcastToNumberTypeP4TypedAstEvaluator.evalToNumExprreturned a rawDouble(ignoring the configurednumberType) and cast the default value via(Number) eval(...), so a non-Number default (toNum('abc','xyz'),toNum('abc', true)) threwClassCastException. Other number functions (sin/sqrt/abs...) already route throughcastToNumberType; onlytoNumwas inconsistent. The JavaCode backend rejects non-Number defaults at parse time, so this also improves cross-backend parity (AST no longer crashes at eval time).Fix: route both the parsed value and the default through
castToNumberType(double); when the default is not aNumber, fall back to0.0instead of throwing. This is the minimal safe option; tightening the grammar to reject non-Number defaults at parse time is a larger change left for a follow-up if desired.#92 —
inDayTimeRangealways false for same-day midnight spaninDayTimeRange(MONDAY, 22, MONDAY, 6)(same day,fromHour > toHour) always returnedfalse, whileEmbeddedFunction.inTimeRange(22, 6)treatsfromHour > toHouras a midnight span and returnstruefor23/3. User intent ("Monday 22:00 → Tuesday 06:00" expressed intra-day) was silently dropped.Fix: when the from/to days are equal and
fromHour > toHour, evaluate as a midnight span (nowHour >= fromHour || nowHour < toHour), mirroringinTimeRange. Normal same-day ranges are unchanged.Tests
Issue90GetValueCastTest—set(String, Number)withDouble/Integer/BigInteger;getValuereturns thefloatValue();Floatpath exact; absent → empty.Issue91ToNumTest— non-Number defaults no longer throw;toNum('3.14', 0)returnsFloatfornumberType=_floatandDoublefornumberType=_double; default value is cast throughnumberType.Issue92InDayTimeRangeTest— same-day midnight span includes23/3/22, excludes6(boundary) and12(gap); consistency withinTimeRange; normal same-day range still works.Full suite: 703 tests, 0 failures (9 pre-existing skips).
Risk / revert
All changes are additive/safe-conversion paths; no public API signature changes. Revert is
git revert <commit>(single commit) or per-hunk.