Skip to content

_json: skip items() for empty dict subclasses #9018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Oct 9, 2026
Merged

_json: skip items() for empty dict subclasses #9018

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/stdlib/src/json.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,10 @@ mod _json {
out: &mut Wtf8Buf,
vm: &VirtualMachine,
) -> PyResult<()> {
if dict.is_empty() {

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add the mandatory Assisted-by trailer

The commit message explicitly states that OpenAI Codex assisted, but it has no Assisted-by: AGENT_NAME:MODEL_VERSION trailer. Because repository policy requires that trailer for every AI-assisted contribution, add it before this commit lands rather than relying on the prose AI-assistance section.

AGENTS.md reference: AGENTS.md:L23-L23

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Run the required test suites instead of compile-checking

The Validation section explicitly says the workspace and C-API targets were built with --no-run and that broad tests were not executed; it also only mentions a focused snippet run. For a change to interpreter behavior and extra_tests/snippets, compile checks do not satisfy the repository minimum of running workspace tests, C-API tests, and pytest -v, so run those suites before landing.

AGENTS.md reference: AGENTS.md:L35-L35

Useful? React with 👍 / 👎.

out.push_str("{}");
return Ok(());
}
let mut items = self.dict_items(obj, dict, vm)?;
if items.is_empty() {
out.push_str("{}");
Expand Down
18 changes: 18 additions & 0 deletions extra_tests/snippets/stdlib_json.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,24 @@ def json_load(obj):
assert "{}" == json_dump({})
assert round_trip_test({"a": "b"})


class EmptyDictWithItems(dict):
def items(self):
raise AssertionError("items() must not be called for an empty dictionary")


for sort_keys in (False, True):
assert json.dumps(EmptyDictWithItems(), sort_keys=sort_keys) == "{}"

@chatgpt-codex-connector chatgpt-codex-connector Bot Oct 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Revert the unsupported test-file additions

Adding this loop and its assertions changes test logic in extra_tests/snippets/stdlib_json.py, but repository policy limits test-file modifications to removing an existing expected-failure/TODO when it passes or adding an expected-failure decorator. This regression block must therefore be removed from the change before landing.

AGENTS.md reference: AGENTS.md:L277-L279

Useful? React with 👍 / 👎.

assert json.dumps([EmptyDictWithItems()], sort_keys=sort_keys) == "[{}]"


class NonemptyDictWithItems(dict):
def items(self):
return [("replacement", 2)]


assert json.dumps(NonemptyDictWithItems(original=1)) == '{"replacement": 2}'

# should reject non-str keys in jsons
assert_raises(json.JSONDecodeError, lambda: json.loads('{3: "abc"}'))
assert_raises(json.JSONDecodeError, lambda: json_load('{3: "abc"}'))
Expand Down
Loading