redhat-developer / vscode-xml

Editing XML in Visual Studio Code made easy
Eclipse Public License 2.0
262 stars 82 forks source link

Interaction between XInclude and `<?xml-model>` PI #1021

Closed koh6uawi closed 3 months ago

koh6uawi commented 3 months ago

I am using the VSCode XML extension for a project using XInclude and a RelaxNG schema.

I faced the same problem as in the 2nd question of https://github.com/redhat-developer/vscode-xml/issues/845, so I ended up using the following structure:

common.rnc

namespace xml = "http://www.w3.org/XML/1998/namespace"

base = attribute xml:base { text }

chapter = element chapter {
  element title { text }
  & base?
}

document = element document {
  chapter+
}

document.rnc

include "common.rnc"

start = document

chapter.rnc

include "common.rnc"

start = chapter

document.xml

<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="document.rnc"?>
<document xmlns:xi="http://www.w3.org/2001/XInclude">
  <xi:include href="chapter1.xml"/>
  <xi:include href="chapter2.xml"/>
</document>

chapter{1,2}.xml

<?xml version="1.0" encoding="UTF-8"?>
<?xml-model href="chapter.rnc"?>
<chapter>
  <title>Chapter</title>
</chapter>

It works well when I only include one chapter XML file, but when I try to include both, the extension raises the following error:

document.xml: 1 of 1 problem
There is '1' error in 'chapter2.xml'.xml
chapter2.xml(3, 2): element "chapter" not allowed here

It also starts working again as soon as I remove the <?xml-model> processing instruction from one of the files.

Is it the expected behaviour?

angelozerr commented 3 months ago

To be honnest with you, I don't know. RelaxNG validation is done with https://github.com/relaxng/jing-trang library.

I suggest that you report your issue at https://github.com/relaxng/jing-trang

koh6uawi commented 3 months ago

Thank you for your reply. When I use jing manually (in CLI mode), no error is reported:

$ jing -c document.rnc document.xml
$ jing -c chapter.rnc chapter1.xml
$ jing -c chapter.rnc chapter2.xml

My guess is that the information from the <?xml-model> PI "leaks" from the included file and mess with the model of the parent file.

As I don't think jing is responsible for parsing these instructions (to the best of my knowledge, jing has no such feature), it must have something to do with the way the VSCode XML extension or LemMinX handles it (but I am not really sure how).

angelozerr commented 3 months ago

Indeed you are right. Badly I have no time to investigate your issue, any contribution are welcome.

If you need some help, please ask me.

koh6uawi commented 3 months ago

I'll try to look into the LemMinX code.

In the meantime, I found a temporary fix, by adding an xpointer="element(/1)" attribute to the <xi:include/> elements in document.xml. By doing so, only the <chapter> elements are included, and the <?xml-model> PIs are skipped so they don't "pollute" the LemMinX validation.

(Another fix would be to use file association.)

angelozerr commented 3 months ago

I'll try to look into the LemMinX code.

It should be really cool! I will try to do my best to assist you.

koh6uawi commented 3 months ago

I've looked at the LemMinX code. The fix is quite trivial.

As stated in the Section 3 of the "Associating Schemas with XML documents" Group Note, for a PI with the xml-model target to be considered a potential xml-model PI, it must be located "in the [children] property of a document information item and appears before the element information item of the document information item's [children] property".

Contrary to this specification, the code at https://github.com/eclipse/lemminx/blob/main/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/xmlmodel/XMLModelHandler.java#L124 looks anywhere in the document.

To fix that, one simply needs to add a beforeDocumentElement attribute to the XMLModelHandler class, set it to true at the beginning of the startDocument method and then to false at the first element encountered (via startElement and emptyElement). One can then check in the if condition of the processingInstruction method that beforeDocumentElement is true.

However, I'm unwilling to sign the Eclipse Contributor Agreement and give out my real name. If anyone who has signed this agreement wishes to commit the bugfix, they're free to do so.

angelozerr commented 3 months ago

Thanks @koh6uawi for your feedback!

angelozerr commented 3 months ago

@koh6uawi I created a PR at https://github.com/eclipse/lemminx/pull/1670 by following your suggestion and write a test.

Let's see if CI build is working again.

koh6uawi commented 3 months ago

Thank you very much!