Source Generator uses the name "Content" which conflicts with any element named "Content."

Thanks Egon, I'll review the patch and get it into the CVS soon.

Created an attachment (id=254)
The schema causing the error (stmml.xsd)

Downloaded stmml.xsd from www.xml-cml.org/schema/stmml

Had to make following changes
a) Remove <xsd:minInclusive> and <xsd:MaxInclusive> from "countType" <xsd:simpleType>
b) Had to remove "content" attribute from "metadata" <xsd:element>
c) Had to remove <h2> element from <xsd:enumeration> in "metadataType" <xsd:simpleType>

Than ran Source Generator (without binding file) without issues.

Looking at stack trace could have been issue with binding file being used.

Suspect a) and c) could be bugs in the schema, but will keep call open to address b).

Edward, can you attach whatever you have currently ?

I don't have anything to attach. I only have my investigation for CASTOR-1580. Do you think this is related?

Ignore my previous comment. I was thinking of a different issue and trying to respond too quickly. I hadn't yet done anything on this issue.

Ignore my last comment as well ... with hindsight, I have no idea as to why I asked you .. ?

a) above is the fact that xsd:integer is represented by Castor by an int, not by a BigInteger. aka, CASTOR-1421
b) above is probably a bug in the source generator
c) above must be a bug in the schema.

I'm looking into b).

b) above is a bug because we create a field _content in the method createFieldInfoForContent(XSType, boolean) in MemberFactory.java. This means that any use of an element that will map to the class name Content will clash with this field. A mapping file will fix this, but probably we shouldn't use such an obvious name for our field for internal use.

In addition to my comment here and above, the source generator generates code for me that won't compile because this schema uses names such as Array and Object. (Of course, Andrew said that sourcegen ran without issue. He didn't say that the code it generated would compile.

This brings up the question: What should Castor do when one is not using a binding file and names that overlap names of java.lang.* will be generated? Should we check for conflicts with java.lang.* so we can provide a meaningful complaint?

I will drop the priority of this issue from critical to minor, because at least with our current release, a mapping file will solve all blocking problems encountered other than CASTOR-1421, which is already tracked elsewhere. I don't see the exception above, so I agree with Andrew that it must have been an artifact of the binding file used.

I think that we should actually check for naming collisions with java.lang.*, and at least report an error similar to how naming conflicts are reported already. Given that this will casue uncompilable code any how ( which might not happen with other naming collisions), why not even go a step further and refuse code generation at all ? Yes, the naming of the '_element' is a bit dd, but it conveys a concept that otherwise would be hidden behind a ridiculous variable name.

I have seen a class that checks names for collisions with reserved words of java somewhere. I think we can extend that one to also check for some other reserved names that we need internaly.

And if you only could remember where .. .

It's org.exolab.javasource.JNaming

I'll look into extending JNaming to also look for java.lang.* clashes, and I'll treat those errors as fatal srcgen errors if no mapping is used. I'll also look into a different name for _content that is not obscure or long but is less likely to conflict with user-generated, well, content.

Attaching a patch that resolves the conflict with members of java.lang.*

Thoughts?

Edward, ignoring the patch for a few minutes, I am definitely in favor of fixing naming collisions with 'java.lang.' classes. On the second issue, we have to keep in mind that any changes to the signature of the classes generated should be as *minimal as possible. People - I asume - will have coded against getContent() for months, if not years.

Given patch looks good to me ...

+1

Werner, I hadn't yet explored whether the "content" variable was one with an external interface that could be called. Now I know that it does. That means the most appropriate way to fix that issue is to check for naming collisions and provide a meaningful complaint.

Attaching final patch.

Fix applied. For conflicts with java.lang.*, source generation throws an exception. For conflicts with "Content" source generation prints a warning but does not throw an exception, as the Content field that Castor uses is not always generated.