Skip to content

fix(ifex_to_json_schema): use None sentinels for mutable default args - #169

Open
SoundMatt wants to merge 1 commit into
COVESA:masterfrom
SoundMatt:fix/ifex-json-schema-mutable-defaults
Open

fix(ifex_to_json_schema): use None sentinels for mutable default args#169
SoundMatt wants to merge 1 commit into
COVESA:masterfrom
SoundMatt:fix/ifex-json-schema-mutable-defaults

Conversation

@SoundMatt

Copy link
Copy Markdown
Contributor

collect_type_info(t, collection={}, seen={}) uses mutable dicts as default arguments. Python creates these once at function-definition time and reuses the same objects on every call that omits those arguments.

After the first call:

  • seen retains all visited type names, so any subsequent call skips every node and collects nothing.
  • collection accumulates entries from previous calls, polluting results.

Fix with the standard None-sentinel pattern.

`collect_type_info(t, collection={}, seen={})` creates both dicts once
at function-definition time and reuses them across all calls.  After
the first run, `seen` already contains all visited types so subsequent
calls find no new types, and `collection` accumulates data across
unrelated invocations.

Replace with `None` sentinels initialised inside the function body.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
@gunnar-mb

Copy link
Copy Markdown
Collaborator

Same as in other places but the described problem is never triggered because the function (the whole module) is only run once. Maybe if the function was shown to be internal to the module, this would be evident to Claude.

Anyhow, I guess the pattern is considered bad practice, so it should be avoided.

@gunnar-mb

Copy link
Copy Markdown
Collaborator

It looks good anyhow - will either merge it, or combine it with the same style of change in other modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants