fix(iohandler): don't drop empty list/dict/tuple args to keep.* functions - #6729
Open
Prabal864 wants to merge 1 commit into
Open
fix(iohandler): don't drop empty list/dict/tuple args to keep.* functions#6729Prabal864 wants to merge 1 commit into
Prabal864 wants to merge 1 commit into
Conversation
…ions
The positional-argument guard in IOHandler's keep.* template-function
parser was patched to special-case "" and 0/0.0 (per its own comment)
but never handled empty containers: a literal [] or {} argument to a
keep.* function (e.g. keep.join([], "-")) parses to a real, falsy-but-
legitimate empty list/dict via ast.literal_eval, fails every branch of
the old truthy-based guard, and gets silently dropped from the call -
shifting every subsequent positional argument one slot to the left.
The same guard also drops a real (non-string) False value, since
False == 0 is True in Python but the guard's `and _arg is not False`
clause then makes the whole condition false.
_arg only ever stays at its None sentinel default when none of the
parsing branches produced a value (e.g. a nested keep.* call that
itself returned nothing), so replacing the guard with a plain
`if _arg is not None:` is a strict improvement: it keeps everything
the old guard kept ("", 0, 0.0) plus [], {}, (), and False, and only
drops the genuine no-value case.
Closes keephq#6728
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 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.
What
Fixes #6728.
IOHandler'skeep.*template-function-call parser built its positional-argument list with:The comment above it says this exists so
""and0/0.0aren't dropped as "empty" — but it only special-cases those two:[]/{}/()argument (parsed a few lines above viaast.literal_evalinto a real, empty container) is falsy, not== "", and not== 0/0.0, so it fails every branch and gets silently dropped.False— which can come back from a nestedkeep.*call — also gets dropped:False == 0isTruein Python, so(_arg == 0 or _arg == 0.0)isTrue, but the trailingand _arg is not Falsethen makes the whole conditionFalse.Dropping an argument shifts every subsequent positional argument one slot left, either raising a confusing
TypeErrordeep insidekeep.functions, or silently binding the wrong value to the wrong parameter.Fix
_argis initialized toNoneand every parsing branch always reassigns it to a real value — it only staysNonewhen nothing was parsed (e.g. a nestedkeep.*call that returned nothing). So the guard can simply beif _arg is not None:, which keeps everything the old guard kept ("",0,0.0) plus[],{},(), andFalse, and only drops the genuine no-value case.Testing
Added 3 cases to
tests/test_iohandler.py::test_with_function_empty_container_argument, exercised through the realIOHandler.render():keep.join([], '-')→""(previously the[]was dropped, leavingjoin('-'), which treats'-'as theiterablestring and fails to parse it as JSON)keep.join({}, '-')→""keep.len([])→0Same environment limitation as my last two PRs here (#6720, #6726):
poetry installfails on this platform becauseuvloopdoesn't support Windows, so I couldn't run the full test suite. What I did verify:python -m py_compilepasses on both changed files.black --checkandisort --checkreport no new issues introduced by this diff (I confirmed the two pre-existing formatting complaints elsewhere iniohandler.pyandtest_iohandler.pyare unrelated to and untouched by this change).ast-parsed[],{},(), and non-empty-list arguments — confirmed the empty containers vanish under the old guard and are preserved under the new one, with no change to the already-correct cases.A maintainer running this on Linux/macOS should see
tests/test_iohandler.py::test_with_function_empty_container_argumentpass outright.Checklist