Skip to content

Resolve circulair includes#190

Open
friedelschoen wants to merge 5 commits into
GoComply:masterfrom
friedelschoen:fix/circulair_include
Open

Resolve circulair includes#190
friedelschoen wants to merge 5 commits into
GoComply:masterfrom
friedelschoen:fix/circulair_include

Conversation

@friedelschoen

@friedelschoen friedelschoen commented May 24, 2026

Copy link
Copy Markdown
Contributor

By definition XSD does allow circulair includes, but trying to do so results in a "hot-potato" loop. This fix holds a loaded schema in memory which won't be re-resolved over and over.

Example of such case: https://github.com/BISONNL/NeTEx-NL

netex-nl-enum.xsd <=> netex-nl-basic.xsd

@friedelschoen friedelschoen changed the title Resolve circulair includes [WIP] Resolve circulair includes May 25, 2026
@friedelschoen friedelschoen force-pushed the fix/circulair_include branch from c763ad6 to a842c24 Compare May 25, 2026 10:22
@friedelschoen

Copy link
Copy Markdown
Contributor Author

When merging circulair dependencies, types will be re-added to the schema, this fix merged and deduplicate things.

@friedelschoen friedelschoen changed the title [WIP] Resolve circulair includes Resolve circulair includes May 25, 2026
@friedelschoen

Copy link
Copy Markdown
Contributor Author

P.S. NeTEx also has a name-clash coming from case-sensitivity:

<xsd:element name="FlexibleServiceProperties" type="flexibleServiceProperties" maxOccurs="unbounded"/>

GoName() turns both names to FlexibleServiceProperties, my quick-fix is to append 'Element' in Element.GoName(). This is a specific problem to NeTEx, is a pull-request desirable?

@isimluk

isimluk commented May 27, 2026

Copy link
Copy Markdown
Member

Thank You for your contribution, @friedelschoen . Don't worry about the lint.

Let me find some time later to review this in depth.

In the mean time, would you perhaps like to add a minimalist test for this?

@isimluk

isimluk commented May 27, 2026

Copy link
Copy Markdown
Member

GoName() turns both names to FlexibleServiceProperties, my quick-fix is to append 'Element' in Element.GoName(). This is a specific problem to NeTEx, is a pull-request desirable?

There is an open semi-abandoned pr #45, to rework name clash resolver. You can take a look to see whether you like that path.

@friedelschoen

Copy link
Copy Markdown
Contributor Author

I'll add some tests next week 😄

I want to look into #45 as NeTEx has many such shenanigans but cannot promise anything.

@friedelschoen friedelschoen force-pushed the fix/circulair_include branch from 6bb3490 to f8d4aab Compare June 2, 2026 18:54
@friedelschoen

Copy link
Copy Markdown
Contributor Author

Hey! I've added some short unit tests for circular includes and imports, is that alright?

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