From 51aa4bfa37bb8319dd4bbd6ad61b45a33109cdbf Mon Sep 17 00:00:00 2001 From: azerr Date: Thu, 20 Aug 2020 11:07:48 +0200 Subject: [PATCH] DTD hyperlink with XML catalog and PUBLIC declaration doesn't work Fixes #850 Signed-off-by: azerr --- .../contentmodel/ContentModelPlugin.java | 4 +-- .../ContentModelDocumentLinkParticipant.java | 34 +++++++------------ .../java/org/eclipse/lemminx/XMLAssert.java | 10 +++++- .../contentmodel/DTDDocumentLinkTest.java | 19 ++++++++++- 4 files changed, 41 insertions(+), 26 deletions(-) diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java index 991da4459a..62aa095cf3 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/ContentModelPlugin.java @@ -53,7 +53,7 @@ public class ContentModelPlugin implements IXMLExtension { private final ICodeActionParticipant codeActionParticipant; - private final IDocumentLinkParticipant documentLinkParticipant; + private IDocumentLinkParticipant documentLinkParticipant; private final ITypeDefinitionParticipant typeDefinitionParticipant; @@ -66,7 +66,6 @@ public ContentModelPlugin() { hoverParticipant = new ContentModelHoverParticipant(); diagnosticsParticipant = new ContentModelDiagnosticsParticipant(this); codeActionParticipant = new ContentModelCodeActionParticipant(); - documentLinkParticipant = new ContentModelDocumentLinkParticipant(); typeDefinitionParticipant = new ContentModelTypeDefinitionParticipant(); } @@ -144,6 +143,7 @@ public void start(InitializeParams params, XMLExtensionsRegistry registry) { if (params != null) { contentModelManager.setRootURI(params.getRootUri()); } + documentLinkParticipant = new ContentModelDocumentLinkParticipant(resolverManager); registry.registerCompletionParticipant(completionParticipant); registry.registerHoverParticipant(hoverParticipant); registry.registerDiagnosticsParticipant(diagnosticsParticipant); diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java index c28b7bafbf..0ce9f069ce 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/ContentModelDocumentLinkParticipant.java @@ -17,8 +17,6 @@ import java.util.Collection; import java.util.List; -import org.apache.xerces.impl.XMLEntityManager; -import org.apache.xerces.util.URI.MalformedURIException; import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.dom.DOMDocument; import org.eclipse.lemminx.dom.DOMDocumentType; @@ -28,6 +26,7 @@ import org.eclipse.lemminx.dom.SchemaLocationHint; import org.eclipse.lemminx.dom.XMLModel; import org.eclipse.lemminx.services.extensions.IDocumentLinkParticipant; +import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager; import org.eclipse.lsp4j.DocumentLink; /** @@ -44,13 +43,19 @@ */ public class ContentModelDocumentLinkParticipant implements IDocumentLinkParticipant { + private final URIResolverExtensionManager resolverManager; + + public ContentModelDocumentLinkParticipant(URIResolverExtensionManager resolverManager) { + this.resolverManager = resolverManager; + } + @Override public void findDocumentLinks(DOMDocument document, List links) { // Document link for xsi:noNamespaceSchemaLocation NoNamespaceSchemaLocation noNamespaceSchemaLocation = document.getNoNamespaceSchemaLocation(); if (noNamespaceSchemaLocation != null) { try { - String location = getResolvedLocation(document.getDocumentURI(), + String location = resolverManager.resolve(document.getDocumentURI(), null, noNamespaceSchemaLocation.getLocation()); if (location != null) { DOMRange attrValue = noNamespaceSchemaLocation.getAttr().getNodeAttrValue(); @@ -65,7 +70,8 @@ public void findDocumentLinks(DOMDocument document, List links) { // Document link for DTD DOMDocumentType docType = document.getDoctype(); if (docType != null) { - String location = getResolvedLocation(document.getDocumentURI(), docType.getSystemIdWithoutQuotes()); + String location = resolverManager.resolve(document.getDocumentURI(), docType.getPublicIdWithoutQuotes(), + docType.getSystemIdWithoutQuotes()); if (location != null) { try { DOMRange systemIdRange = docType.getSystemIdNode(); @@ -80,7 +86,7 @@ public void findDocumentLinks(DOMDocument document, List links) { // Document link for xml-model/href List xmlModels = document.getXMLModels(); for (XMLModel xmlModel : xmlModels) { - String location = getResolvedLocation(document.getDocumentURI(), xmlModel.getHref()); + String location = resolverManager.resolve(document.getDocumentURI(), null, xmlModel.getHref()); if (location != null) { try { DOMRange hrefRange = xmlModel.getHrefNode(); @@ -99,7 +105,7 @@ public void findDocumentLinks(DOMDocument document, List links) { Collection schemaLocationHints = schemaLocation.getSchemaLocationHints(); String location; for (SchemaLocationHint schemaLocationHint : schemaLocationHints) { - location = getResolvedLocation(document.getDocumentURI(), schemaLocationHint.getHint()); + location = resolverManager.resolve(document.getDocumentURI(), null, schemaLocationHint.getHint()); if (location != null) { links.add(createDocumentLink(schemaLocationHint, location, false)); } @@ -110,20 +116,4 @@ public void findDocumentLinks(DOMDocument document, List links) { } } - /** - * Returns the expanded system location - * - * @return the expanded system location - */ - private static String getResolvedLocation(String documentURI, String location) { - if (location == null) { - return null; - } - try { - return XMLEntityManager.expandSystemId(location, documentURI, false); - } catch (MalformedURIException e) { - return location; - } - } - } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java index 4274b204e2..9d1853626d 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/XMLAssert.java @@ -680,12 +680,20 @@ private static String getHoverLabel(Hover hover) { // ------------------- Links assert public static void testDocumentLinkFor(String xml, String fileURI, DocumentLink... expected) { + testDocumentLinkFor(xml, fileURI, null, expected); + } + + public static void testDocumentLinkFor(String xml, String fileURI, String catalogPath, DocumentLink... expected) { TextDocument document = new TextDocument(xml, fileURI != null ? fileURI : "test.xml"); XMLLanguageService xmlLanguageService = new XMLLanguageService(); ContentModelSettings settings = new ContentModelSettings(); settings.setUseCache(false); + // Configure XML catalog for XML schema + if (catalogPath != null) { + settings.setCatalogs(new String[] { catalogPath }); + } xmlLanguageService.doSave(new SettingsSaveContext(settings)); DOMDocument xmlDocument = DOMParser.getInstance().parse(document, @@ -705,7 +713,7 @@ public static void assertDocumentLinks(List actual, DocumentLink.. assertEquals(expected.length, actual.size()); for (int i = 0; i < expected.length; i++) { assertEquals(expected[i].getRange(), actual.get(i).getRange(), " Range test '" + i + "' link"); - assertEquals(Paths.get(expected[i].getTarget()).toUri().toString(), actual.get(i).getTarget(), + assertEquals(Paths.get(expected[i].getTarget()).toUri().toString().replace("file:///", "file:/"), actual.get(i).getTarget(), " Target test '" + i + "' link"); } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java index facf11a6d2..171d5655d0 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java @@ -45,7 +45,7 @@ public void docTypePUBLIC() throws BadLocationException { XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml", dl(r(1, 38, 1, 62), "src/test/resources/dtd/entities/base.dtd")); } - + @Test public void noLinks() throws BadLocationException { String xml = "\r\n" + // @@ -55,4 +55,21 @@ public void noLinks() throws BadLocationException { ""; XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml"); } + + @Test + public void linkWithCatalogAndPublic() throws Exception { + // This test uses the local DTD with catalog-public.xml by using the PUBLIC ID + // -//Sun Microsystems, Inc.//DTD Web Application 2.3//EN + // + String xml = " \r\n" + // + "\r\n" + // + "\r\n" + // + ""; + XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml", + "src/test/resources/catalogs/catalog-public.xml", + dl(r(3,4,3,12), "src/test/resources/dtd/web-app_2_3.dtd")); + } }