Skip to content

fix: handle non-string dict keys holding lists in keylists/keypaths#583

Merged
fabiocaccamo merged 1 commit into
fabiocaccamo:mainfrom
Sanjays2402:fix/keylists-non-string-keys-with-lists
Jul 7, 2026
Merged

fix: handle non-string dict keys holding lists in keylists/keypaths#583
fabiocaccamo merged 1 commit into
fabiocaccamo:mainfrom
Sanjays2402:fix/keylists-non-string-keys-with-lists

Conversation

@Sanjays2402

Copy link
Copy Markdown
Contributor

Summary

keylists(d, indexes=True) raises TypeError whenever a non-string dict key (int / None / bool / tuple) holds a list value. Because keypaths, match, and the public benedict.keypaths() / benedict.keylists() all build on keylists, the crash also surfaces through the public API.

Reproduce

from benedict import benedict

benedict({0: [1, 2]}).keypaths(indexes=True)
# TypeError: unsupported operand type(s) for +=: 'int' and 'str'

from benedict.core.keylists import keylists
keylists({0: [1, 2]}, indexes=True)      # TypeError
keylists({None: [1, 2]}, indexes=True)   # TypeError

Root cause

In benedict/core/keylists.py, _get_keylist_for_list appended the list index to the parent key with:

keys[-1] += f"[{key}]"

This assumes the last parent key is a string. A string key "a" becomes "a[0]", but an int key 0 evaluates 0 += "[0]" and crashes.

The behaviour was inconsistent: the same non-string key holding a dict worked fine, and non-string keys are explicitly supported elsewhere in the module (see the existing test_keylists_with_non_string_keys). Only the list branch assumed stringness.

input keylists(..., indexes=True) before after
{"a": [1, 2]} [['a'], ['a[0]'], ['a[1]']] [['a'], ['a[0]'], ['a[1]']] (unchanged)
{0: {"x": 1}} [[0], [0, 'x']] [[0], [0, 'x']] (unchanged)
{0: [1, 2]} TypeError [[0], ['0[0]'], ['0[1]']]
{None: [1, 2]} TypeError [[None], ['None[0]'], ['None[1]']]

Fix

Stringify the parent key before appending the index:

keys[-1] = f"{keys[-1]}[{key}]"

String keys are unaffected (f"{'a'}[0]" == "a[0]"); non-string keys now produce a consistent stringified keypath (0"0[0]", None"None[0]"), matching how keypaths already stringifies every key via f"{key}".

Tests

Added test_keylists_with_non_string_keys_and_lists_and_indexes_included, which follows the existing test_keylists_with_non_string_keys membership style (mixed int/str/None keylists can't be sorted for comparison).

Proven to guard the bug — stash the source fix and the new test fails with the original TypeError at keylists.py:26; restore the fix and it passes:

# without fix
FAILED tests/core/test_keylists.py::keylists_test_case::test_keylists_with_non_string_keys_and_lists_and_indexes_included
  E   TypeError: unsupported operand type(s) for +=: 'int' and 'str'
  benedict/core/keylists.py:26: TypeError

# with fix
tests/core/test_keylists.py ... 1 passed

Full tests/core + tests/utils suite: 123 passed (was 122). ruff check, ruff format --check, black --check and mypy are all clean on the changed files (the 3 pre-existing mypy errors in serializers/* and parse_util.py are unrelated optional-dependency stub issues, present with or without this change).

Diff: +27 / −1 across benedict/core/keylists.py and tests/core/test_keylists.py.

`keylists(d, indexes=True)` raised `TypeError: unsupported operand
type(s) for +=: 'int' and 'str'` whenever a non-string dict key
(int / None / bool / tuple) held a list value.

Root cause: in `_get_keylist_for_list` the index was appended to the
parent key with `keys[-1] += f"[{key}]"`, which assumes the last parent
key is a string. A string key `"a"` became `"a[0]"`, but an int key `0`
hit `0 += "[0]"` and crashed.

This was inconsistent: the same non-string key holding a *dict* worked
fine (`keylists({0: {"x": 1}})` -> `[[0], [0, "x"]]`), and non-string
keys are explicitly supported elsewhere (see
`test_keylists_with_non_string_keys`). Only the list branch assumed
stringness.

Because `keypaths`, `match` and `benedict.keypaths()/keylists()` all
build on `keylists`, the crash surfaced through the public API too, e.g.
`benedict({0: [1, 2]}).keypaths(indexes=True)`.

Fix: stringify the parent key before appending the index
(`keys[-1] = f"{keys[-1]}[{key}]"`). String keys are unaffected
(`f"{'a'}[0]" == "a[0]"`); non-string keys now produce a consistent
stringified keypath (`0` -> `"0[0]"`, `None` -> `"None[0]"`), matching
how `keypaths` already stringifies every key via `f"{key}"`.

Added `test_keylists_with_non_string_keys_and_lists_and_indexes_included`
which fails without the fix (TypeError) and passes with it.
@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.32%. Comparing base (068813d) to head (3425c1f).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #583   +/-   ##
=======================================
  Coverage   98.32%   98.32%           
=======================================
  Files          64       64           
  Lines        2392     2392           
=======================================
  Hits         2352     2352           
  Misses         40       40           
Flag Coverage Δ
unittests 98.32% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a TypeError in keylists(..., indexes=True) when traversing list values under non-string dict keys, which also affected the public benedict.keypaths() / benedict.keylists() APIs that build on keylists.

Changes:

  • Update list-index key composition to stringify the parent key before appending "[i]", avoiding int += str (and similar) failures.
  • Add a regression test covering non-string keys (e.g., 0, None) that hold list values when indexes=True.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
benedict/core/keylists.py Prevents TypeError by building indexed list keys via an f-string assignment instead of += on potentially non-string keys.
tests/core/test_keylists.py Adds regression coverage for non-string dict keys that hold lists when indexes=True.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@fabiocaccamo
fabiocaccamo merged commit a87b689 into fabiocaccamo:main Jul 7, 2026
18 checks passed
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.

3 participants