Skip to content

fix: renderpath for type-extensions - #1499

Open
m-hau wants to merge 2 commits into
mvantellingen:mainfrom
m-hau:fix/renderpath-typeextension
Open

fix: renderpath for type-extensions#1499
m-hau wants to merge 2 commits into
mvantellingen:mainfrom
m-hau:fix/renderpath-typeextension

Conversation

@m-hau

@m-hau m-hau commented Jul 23, 2026

Copy link
Copy Markdown

The child_path variable can still be set from the attribute loop above, but then it has the wrong value. Without any attributes it is unset and causes an UnboundLocalError. Set it always to a proper value.

Fixes: 0c8a3fc ("Add support to pass base-types to type extensions")

The `child_path` variable can still be set from the attribute loop
above, but then it has the wrong value. Without any attributes it is
unset and causes an UnboundLocalError. Set it always to a proper value.

Fixes: 0c8a3fc ("Add support to pass base-types to type extensions")

@victorwon2001 victorwon2001 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Confirmed the fix against 1b7072c: rendering a raw string through an attribute-free simpleContent extension raises UnboundLocalError, while 907f6c8 renders it correctly. The full suite passes locally (475 passed, 18 skipped), and Ruff is clean.

Could you add a regression test for that attribute-free path? A minimal schema with <xsd:simpleContent><xsd:extension base="xsd:string"/></xsd:simpleContent> and element.render(..., "hello") reproduces the failure on main. That would keep this one-line fix covered.

The side-effect free function `render_node(…)` is called twice, remove
the second call.

Fixes: d88d427 ("When Nil is set on an element which is built …")
@m-hau
m-hau force-pushed the fix/renderpath-typeextension branch from 907f6c8 to 71761c8 Compare August 31, 2026 11:24
@m-hau

m-hau commented Aug 31, 2026

Copy link
Copy Markdown
Author

I added a test like requested, making sure it fails without the fix and passes with the fix.

While looking at the other tests to get an idea how to add my own, I noticed one of the tests in that file called render_node(…) twice without any apparent necessity or effect. It is probably a simple copy&paste error, since some tests call that function before the expected constant and others right before the assert_nodes_equal. I took the liberty of adding a second commit removing the second redundant call.

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