Skip to content

Fix $range and $random ignoring zero-valued optional arguments#118

Merged
zeroSteiner merged 1 commit into
zeroSteiner:masterfrom
gaoflow:fix/builtin-zero-arg-check
Jun 25, 2026
Merged

Fix $range and $random ignoring zero-valued optional arguments#118
zeroSteiner merged 1 commit into
zeroSteiner:masterfrom
gaoflow:fix/builtin-zero-arg-check

Conversation

@gaoflow

@gaoflow gaoflow commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

What

_builtin_range and _builtin_random use truthiness checks (if stop:,
if step:, if boundary:) to detect whether optional numeric arguments
were supplied. This is wrong when the argument value is 0, which is
falsy but a valid integer.

$range(start, stop=0)

$range(5, 0)   # returned [0, 1, 2, 3, 4] — should be []

stop=0 evaluated as falsy, so the function ignored it and called
range(int(start)) = range(5) instead of range(5, 0) = [].

$range(start, stop, step=0)

step=0 evaluated as falsy, so the zero-step check was skipped.
Python's range() then raised a bare ValueError rather than a
library FunctionCallError.

$random(boundary=0)

$random(0)   # returned a random float in [0, 1) — should always be 0

boundary=0 evaluated as falsy, so the function ignored it and returned
random.random() instead of random.randint(0, 0) = 0.

Fix

Replace if stop: / if step: / if boundary: with is not None
comparisons in lib/rule_engine/builtins.py. Add regression tests for
$range(5, 0) and $random(0).


This pull request was prepared with the assistance of AI, under my direction and review.

$range(start, stop=0) silently fell through to range(start) because the
'if stop:' guard treated 0 as falsy, producing [0..start-1] instead of
an empty sequence.  Similarly, $range(start, stop, step=0) skipped the
step validation, letting Python's range() receive a zero step without a
meaningful error message.

$random(boundary=0) ignored the argument and returned a float in [0, 1)
instead of the expected integer 0, again because 'if boundary:' treated 0
as falsy.

Fix both by using 'is not None' comparisons, which match only the absence
of an argument rather than its falsy value.  Add regression tests for
$range(5, 0) and $random(0).
@zeroSteiner

Copy link
Copy Markdown
Owner

Yup, that looks legit. Thanks for reporting and submitting a fix for this.

@zeroSteiner

Copy link
Copy Markdown
Owner

LGTM

(rule-engine)   : rule-engine: 13:07:05 madcat rule-engine PYTHONPATH=$(pwd)/lib python -m rule_engine.debug_repl
rule > $range(0)
result: 
()
rule > $range(5, 0)
result: 
(Decimal('0'), Decimal('1'), Decimal('2'), Decimal('3'), Decimal('4'))
rule > $random(0)
result: 
Decimal('0.2432948731525687')
rule >                                                                                                                                                                                                             
(rule-engine)   : rule-engine: 13:07:57 madcat rule-engine git checkout-pr 118
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Total 7 (delta 5), reused 7 (delta 5), pack-reused 0 (from 0)
Unpacking objects: 100% (7/7), 1.30 KiB | 1.30 MiB/s, done.
From github:zeroSteiner/rule-engine
 * [new ref]         refs/pull/118/head -> pr/118
Switched to branch 'pr/118'
(rule-engine)   : rule-engine: 11813:08:01 madcat rule-engine PYTHONPATH=$(pwd)/lib python -m rule_engine.debug_repl
rule > $range(0)
result: 
()
rule > $range(5, 0)
result: 
()
rule > $random(0)
result: 
Decimal('0')
rule >

@zeroSteiner zeroSteiner merged commit 3ec40d8 into zeroSteiner:master Jun 25, 2026
8 checks passed
@zeroSteiner

Copy link
Copy Markdown
Owner

Patch shipped in 5.0.1. Thanks again.

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.

2 participants