Do not expose Xerces Types in API

[Treehopper]

I am currently implementing an IXMLExtension to enhance the editing of Liquibase XML files.

I am using JPMS on my artifact with the following module-info.java

import org.eclipse.lsp4xml.services.extensions.IXMLExtension;
import org.foobar.LiquibasePlugin;

module liquibase.lsp {
    exports org.foobar;

//  requires transitive org.eclipse.lemminx;
    requires transitive org.eclipse.lsp4xml;
    requires transitive org.eclipse.lsp4j;
    requires transitive org.eclipse.lsp4j.jsonrpc;

    requires transitive xercesImpl;

    provides IXMLExtension with LiquibasePlugin;
}

The only reason for me to require Xerces is that I need it to implement for example URIResolverExtension without running into compilation errors. When I do require it, I get this however: java.lang.module.ResolutionException: Modules jdk.xml.dom and xercesImpl export package org.w3c.dom.html to module org.eclipse.lsp4j

Maybe this instance of the problem is more a Xerces than a lemminx issue, but I would additionally argue that binding the API to a any specific library or framework, will make it impossible to evolve that API long-term. Also, I am doubtful that Xerces would fix this issue on their end anytime soon. I guess wrapping those Xerces types would be an opportunity to fix both of those issues.

I am currently using lsp4xml to be compatible with the latest WWD release, though I have seen that lemminx has the same issue.

[angelozerr]

I agree with you, and the whole API in LemMinx should not be linked to Xerces and it should be the case (except for URIResolverExtension or perhaps some other API ?)

Even if LemMinx is working with extensions (XSD, DTD support, etc), we have not a lot of external extension (today it exists just the https://github.com/eclipse/lemminx-maven and an extension from Liferay, so the API is not frozen yet) (it's one reason why we are not in 1.0.0 version)

If you want to remove Xerces dependencies from URIResolverExtension, any contribution are welcome!

[Treehopper]

Probably this isn't a small effort. Today I looked into implementing an IDiagnosticsParticipant. Following the implementation of the XSDDiagnosticsParticipant, I'm also stumbling over Xerces implementation classes, such as the XMLErrorReporter, when trying to implement the AbstractLSPErrorReporter. I guess I could start creating a PR, where I wrap and delegate all Xerces types I find, but I'm not really sure where to begin or end. Would it make sense to start with creating an "api" package to define the scope?

[angelozerr]

Probably this isn't a small effort.

Indeed I fear it's a very long and hard issue -(

Let's go step by step. I suggest you that you remove Xerces dependencies from the pom.xml to see compilation problem:

• errors coming from extensions package must be ignored because it's not API, just implementation:

• error coming from services packages must be fixed because it's the API:

I suggest you that you move the 2 classes linked to Xerces to the org.eclipse.lemminx.extensions.xerces package

• uriresolver package must be fixed by using wrapper class (I fear that it's an hard issue)

• utils has error with XMLPositionUtility. I suggest you that you move XMLPositionUtility#toLSPPosition as private static method in AbstractLSPErrorReporter

Good luck!

[Treehopper]

This first PR implements your suggestions for moving the code around. I suppose we can merge it independently. Afterwards, I'd have a look at hiding the Xerces-dependencies.

[Treehopper]

I've written some wrapper code as part of liquibase-lsp here: https://github.com/Treehopper/liquibase-lsp/tree/develop/org.eclipse.lemminx.api/src/main/java

It certainly only covers a small part of the API, and it is not a well thought out implementation, but it might be a start. I suppose I will eventually get to it, but since what I have now works for me, it is not among my top priorities.

[Treehopper]

I had an idea for approaching this in a way that allows solving the problem iteratively, measurably and at the same time, safe-guard against re-introducing new Xerces dependencies. Given that ArchUnit tests are fairly readable, the PR might speak for itself, but I guess this might be a useful reference: https://www.archunit.org/userguide/html/000_Index.html#_freezing_arch_rules