From d1b91208616c2cff204091f3d41eba7718153d49 Mon Sep 17 00:00:00 2001 From: azerr Date: Fri, 5 Jun 2020 16:38:14 +0200 Subject: [PATCH] NPE with TypeDefinition Fixes #629 Signed-off-by: azerr --- .../DTDDefinitionParticipant.java | 4 +- .../CMXSDContentModelProvider.java | 19 +-- .../xsd/contentmodel/CMXSDDocument.java | 154 ++++++++++++++++-- .../XSDDocumentLinkParticipant.java | 2 +- .../extensions/xsd/utils/XSDUtils.java | 23 ++- .../lemminx/utils/XMLPositionUtility.java | 21 +++ ...XMLSchemaTypeDefinitionExtensionsTest.java | 30 ++++ .../src/test/resources/xml/Format.xml | 31 ++++ 8 files changed, 238 insertions(+), 46 deletions(-) create mode 100644 org.eclipse.lemminx/src/test/resources/xml/Format.xml diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/participants/DTDDefinitionParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/participants/DTDDefinitionParticipant.java index e299cb5471..3d299241f6 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/participants/DTDDefinitionParticipant.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/participants/DTDDefinitionParticipant.java @@ -15,6 +15,7 @@ import org.eclipse.lemminx.dom.DOMDocument; import org.eclipse.lemminx.dom.DOMNode; +import org.eclipse.lemminx.dom.DOMRange; import org.eclipse.lemminx.dom.DTDDeclNode; import org.eclipse.lemminx.dom.DTDDeclParameter; import org.eclipse.lemminx.extensions.dtd.utils.DTDUtils; @@ -54,7 +55,8 @@ protected void doFindDefinition(IDefinitionRequest request, List l DTDDeclParameter originName = ((DTDDeclNode) node).getReferencedElementNameAt(offset); if (originName != null) { DTDUtils.searchDTDTargetElementDecl(originName, true, targetElementName -> { - LocationLink location = XMLPositionUtility.createLocationLink(originName, targetElementName); + LocationLink location = XMLPositionUtility.createLocationLink((DOMRange) originName, + (DOMRange) targetElementName); locations.add(location); }); } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDContentModelProvider.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDContentModelProvider.java index bbf55fff9d..2cc67927bf 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDContentModelProvider.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDContentModelProvider.java @@ -37,8 +37,6 @@ public class CMXSDContentModelProvider implements ContentModelProvider { private final URIResolverExtensionManager resolverExtensionManager; - private XSLoaderImpl loader; - public CMXSDContentModelProvider(URIResolverExtensionManager resolverExtensionManager) { this.resolverExtensionManager = resolverExtensionManager; } @@ -82,10 +80,11 @@ public Collection getSystemIds(DOMDocument xmlDocument, String namespace @Override public CMDocument createCMDocument(String key) { - XSModel model = getLoader().loadURI(key); + XSLoaderImpl loader = getLoader(); + XSModel model = loader.loadURI(key); if (model != null) { // XML Schema can be loaded - return new CMXSDDocument(model); + return new CMXSDDocument(model, loader); } return null; } @@ -95,17 +94,7 @@ public CMDocument createInternalCMDocument(DOMDocument xmlDocument) { return null; } - public XSLoaderImpl getLoader() { - if (loader == null) { - loader = getSynchLoader(); - } - return loader; - } - - private synchronized XSLoaderImpl getSynchLoader() { - if (loader != null) { - return loader; - } + public XSLoaderImpl getLoader() { XSLoaderImpl loader = new XSLoaderImpl(); loader.setParameter("http://apache.org/xml/properties/internal/entity-resolver", resolverExtensionManager); loader.setParameter(Constants.DOM_ERROR_HANDLER, new DOMErrorHandler() { diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDDocument.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDDocument.java index bd0fd09dce..eca7786e65 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDDocument.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDDocument.java @@ -25,9 +25,14 @@ import org.apache.xerces.impl.dv.XSSimpleType; import org.apache.xerces.impl.xs.SchemaGrammar; +import org.apache.xerces.impl.xs.XMLSchemaLoader; import org.apache.xerces.impl.xs.XSComplexTypeDecl; import org.apache.xerces.impl.xs.XSElementDecl; import org.apache.xerces.impl.xs.XSElementDeclHelper; +import org.apache.xerces.impl.xs.XSLoaderImpl; +import org.apache.xerces.impl.xs.XSParticleDecl; +import org.apache.xerces.impl.xs.opti.ElementImpl; +import org.apache.xerces.impl.xs.traversers.XSDHandler; import org.apache.xerces.impl.xs.util.SimpleLocator; import org.apache.xerces.xni.QName; import org.apache.xerces.xni.XMLLocator; @@ -77,8 +82,11 @@ public class CMXSDDocument implements CMDocument, XSElementDeclHelper { private final FilesChangedTracker tracker; - public CMXSDDocument(XSModel model) { + private final XSLoaderImpl xsLoader; + + public CMXSDDocument(XSModel model, XSLoaderImpl xsLoaderImpl) { this.model = model; + this.xsLoader = xsLoaderImpl; this.elementMappings = new HashMap<>(); this.tracker = createFilesChangedTracker(model); } @@ -235,7 +243,7 @@ public LocationLink findTypeLocation(DOMNode originNode) { originAttribute = (DOMAttr) originNode; originElement = originAttribute.getOwnerElement(); } - if (originElement == null) { + if (originElement == null || originElement.getLocalName() == null) { return null; } // Try to retrieve XSD element declaration from the given element. @@ -244,12 +252,19 @@ public LocationLink findTypeLocation(DOMNode originNode) { if (elementDeclaration == null) { return null; } + + // Try to find the Xerces xs:element (which stores the offset) bound with the + // XSElementDeclaration + // case when xs:element is declared inside xs:choice, xs:all, xs:sequence, etc + ElementImpl xercesElement = findLocalMappedXercesElement(elementDeclaration.getElementDeclaration(), xsLoader); + // case when xs:element is declared as global or inside xs:complexType SchemaGrammar schemaGrammar = getOwnerSchemaGrammar(elementDeclaration.getElementDeclaration()); - if (schemaGrammar == null) { + if (schemaGrammar == null && xercesElement == null) { return null; } - String documentURI = getSchemaURI(schemaGrammar); + String documentURI = xercesElement != null ? xercesElement.getOwnerDocument().getDocumentURI() + : getSchemaURI(schemaGrammar); if (URIUtils.isFileResource(documentURI)) { // Only XML Schema file is supported. In the case of XML file is bound with an // HTTP url and cache is enable, documentURI is a file uri from the cache @@ -274,7 +289,6 @@ public LocationLink findTypeLocation(DOMNode originNode) { if (attributeDecl.getScope() == XSConstants.SCOPE_LOCAL) { return findLocalXSAttribute(originAttribute, targetSchema, attributeDecl.getEnclosingCTDefinition(), schemaGrammar); - } } } else { @@ -282,10 +296,20 @@ public LocationLink findTypeLocation(DOMNode originNode) { boolean globalElement = elementDeclaration.getElementDeclaration() .getScope() == XSElementDecl.SCOPE_GLOBAL; if (globalElement) { + // global xs:element return findGlobalXSElement(originElement, targetSchema); } else { - return findLocalXSElement(originElement, targetSchema, - elementDeclaration.getElementDeclaration().getEnclosingCTDefinition(), schemaGrammar); + // local xs:element + // 1) use the Xerces xs:element strategy + if (xercesElement != null) { + return findLocalXSElement(originElement, targetSchema, xercesElement.getCharacterOffset()); + } + // 2) use the Xerces xs:complexType strategy + XSComplexTypeDefinition complexTypeDefinition = elementDeclaration.getElementDeclaration() + .getEnclosingCTDefinition(); + if (complexTypeDefinition != null) { + return findLocalXSElement(originElement, targetSchema, complexTypeDefinition, schemaGrammar); + } } } } @@ -392,6 +416,94 @@ private static LocationLink findGlobalXSElement(DOMElement originElement, DOMDoc return findXSElement(originElement, children, false); } + /** + * Returns the Xerces DOM xs:element which have created the given instance + * elementDeclaration and null otherwise. + * + * @param elementDeclaration + * @param xsLoader + * @return the Xerces DOM xs:element which have created the given instance + * elementDeclaration and null otherwise + */ + private static ElementImpl findLocalMappedXercesElement(XSElementDeclaration elementDeclaration, + XSLoaderImpl xsLoader) { + try { + // When XML Schema is loaded by XSLoaderImpl, it uses XMLSchemaLoader which uses + // XSDHandler. + // // Xerces stores in XSDHandler instance, the location in 2 arrays: + // - fParticle array of XSParticleDecl where fValue is an instance of + // XSElementDeclaration + // - fLocalElementDecl array of Xerces Element which stores the element offset. + + // Get the XMLSchemaLoader instance from the XSLoader instance + Field f = XSLoaderImpl.class.getDeclaredField("fSchemaLoader"); + f.setAccessible(true); + XMLSchemaLoader schemaLoader = (XMLSchemaLoader) f.get(xsLoader); + + // Get the XSDHandler instance from the XMLSchemaLoader instance + Field f2 = XMLSchemaLoader.class.getDeclaredField("fSchemaHandler"); + f2.setAccessible(true); + XSDHandler handler = (XSDHandler) f2.get(schemaLoader); + + // Get the XSParticleDecl array from the XSDHandler instance + Field f3 = XSDHandler.class.getDeclaredField("fParticle"); + f3.setAccessible(true); + XSParticleDecl[] fParticle = (XSParticleDecl[]) f3.get(handler); + + // Get the index where elementDeclaration is associated with a XSParticleDecl + int i = getXSElementDeclIndex(elementDeclaration, fParticle); + if (i >= 0) { + // Get the Xerces Element array from the XSDHandler instance + Field f4 = XSDHandler.class.getDeclaredField("fLocalElementDecl"); + f4.setAccessible(true); + Element[] fLocalElementDecl = (Element[]) f4.get(handler); + return (ElementImpl) fLocalElementDecl[i]; + } + } catch (Exception e) { + LOGGER.log(Level.SEVERE, + "Error while retrieving mapped Xerces xs:element of '" + elementDeclaration.getName() + "'.", e); + } + return null; + + } + + /** + * Returns the location link for the given origin element and target element at + * the given offset. + * + * @param originElement the origin element + * @param targetSchema the target DOM document + * @param offset the offset of the element range to return. + * @return the location link for the given origin element and target element at + * the given offset. + */ + private static LocationLink findLocalXSElement(DOMElement originElement, DOMDocument targetSchema, int offset) { + DOMNode node = targetSchema.findNodeAt(offset); + if (node != null && node.isElement()) { + return findXSElement(originElement, (Element) node); + } + return null; + } + + /** + * Returns the index where the given element declaration is store in the + * particles array and null otherwise. + * + * @param elementDeclaration the XS element declaration. + * @param particles the XS particles declaration. + * @return the index where the given element declaration is store in the + * particles array and null otherwise. + */ + private static int getXSElementDeclIndex(XSElementDeclaration elementDeclaration, XSParticleDecl[] particles) { + for (int i = 0; i < particles.length; i++) { + XSParticleDecl particle = particles[i]; + if (particle != null && elementDeclaration.equals(particle.fValue)) { + return i; + } + } + return -1; + } + /** * Returns the location of the local xs:element declared in the given XML Schema * targetSchema which matches the given XML element @@ -409,7 +521,7 @@ private static LocationLink findGlobalXSElement(DOMElement originElement, DOMDoc private static LocationLink findLocalXSElement(DOMElement originElement, DOMDocument targetSchema, XSComplexTypeDefinition enclosingType, SchemaGrammar schemaGrammar) { // In local xs:element case, xs:element is declared inside a complex type - // (enclosing tye). + // (enclosing type). // Xerces stores in the SchemaGrammar the locator (offset) for each complex type // (XSComplexTypeDecl) // Here we get the offset of the local enclosing complex type xs:complexType. @@ -442,7 +554,7 @@ private static LocationLink findLocalXSElement(DOMElement originElement, DOMDocu */ private static int getComplexTypeOffset(XSComplexTypeDefinition complexType, SchemaGrammar grammar) { try { - // Xerces stores location in 2 arrays: + // Xerces stores in SchemaGrammar instance, the location in 2 arrays: // - fCTLocators array of locator // - fComplexTypeDecls array of XSComplexTypeDecl @@ -501,16 +613,12 @@ private static LocationLink findXSElement(DOMElement originElement, NodeList chi Node n = children.item(i); if (n.getNodeType() == Node.ELEMENT_NODE) { Element elt = (Element) n; - if (XSDUtils.isXSElement(elt)) { - if (originElement.getLocalName().equals(elt.getAttribute("name"))) { - DOMAttr targetAttr = (DOMAttr) elt.getAttributeNode("name"); - LocationLink location = XMLPositionUtility.createLocationLink(originElement, - targetAttr.getNodeAttrValue()); - return location; - } + LocationLink location = findXSElement(originElement, elt); + if (location != null) { + return location; } if (inAnyLevel && elt.hasChildNodes()) { - LocationLink location = findXSElement(originElement, elt.getChildNodes(), inAnyLevel); + location = findXSElement(originElement, elt.getChildNodes(), inAnyLevel); if (location != null) { return location; } @@ -520,6 +628,18 @@ private static LocationLink findXSElement(DOMElement originElement, NodeList chi return null; } + private static LocationLink findXSElement(DOMElement originElement, Element elt) { + if (XSDUtils.isXSElement(elt)) { + if (originElement.getLocalName().equals(elt.getAttribute("name"))) { + DOMAttr targetAttr = (DOMAttr) elt.getAttributeNode("name"); + LocationLink location = XMLPositionUtility.createLocationLink(originElement, + targetAttr.getNodeAttrValue()); + return location; + } + } + return null; + } + private static LocationLink findLocalXSAttribute(DOMAttr originAttribute, DOMDocument targetSchema, XSComplexTypeDefinition enclosingType, SchemaGrammar schemaGrammar) { int complexTypeOffset = getComplexTypeOffset(enclosingType, schemaGrammar); diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/XSDDocumentLinkParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/XSDDocumentLinkParticipant.java index 984d9e1aa1..fcdb11855b 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/XSDDocumentLinkParticipant.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/XSDDocumentLinkParticipant.java @@ -45,7 +45,7 @@ public class XSDDocumentLinkParticipant implements IDocumentLinkParticipant { @Override public void findDocumentLinks(DOMDocument document, List links) { DOMElement root = document.getDocumentElement(); - if (root == null || !XSDUtils.isXSSchema(root)) { + if (!XSDUtils.isXSSchema(root)) { return; } String xmlSchemaPrefix = root.getPrefix(); diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/utils/XSDUtils.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/utils/XSDUtils.java index 6b4171c065..911bc5b279 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/utils/XSDUtils.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/utils/XSDUtils.java @@ -19,8 +19,6 @@ import java.util.Vector; import java.util.function.BiConsumer; -import com.google.common.base.Objects; - import org.apache.xerces.impl.xs.SchemaGrammar; import org.apache.xerces.xs.StringList; import org.eclipse.lemminx.dom.DOMAttr; @@ -39,6 +37,8 @@ import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import com.google.common.base.Objects; + /** * XSD utilities. * @@ -384,28 +384,27 @@ private static void searchXSOriginAttributes(NodeList nodes, List targe } public static boolean isXSComplexType(Element element) { - return "complexType".equals(element.getLocalName()); + return element != null && "complexType".equals(element.getLocalName()); } public static boolean isXSSimpleType(Element element) { - return "simpleType".equals(element.getLocalName()); + return element != null && "simpleType".equals(element.getLocalName()); } public static boolean isXSElement(Element element) { - return "element".equals(element.getLocalName()); + return element != null && "element".equals(element.getLocalName()); } public static boolean isXSGroup(Element element) { - return "group".equals(element.getLocalName()); + return element != null && "group".equals(element.getLocalName()); } public static boolean isXSInclude(Element element) { - return "include".equals(element.getLocalName()); + return element != null && "include".equals(element.getLocalName()); } - - public static boolean isXSImport(Element child) { - return "import".equals(child.getLocalName()); + public static boolean isXSImport(Element element) { + return element != null && "import".equals(element.getLocalName()); } public static boolean isXSTargetElement(Element element) { @@ -413,11 +412,11 @@ public static boolean isXSTargetElement(Element element) { } public static boolean isXSAttribute(DOMElement element) { - return "attribute".equals(element.getLocalName()); + return element != null && "attribute".equals(element.getLocalName()); } public static boolean isXSSchema(Element element) { - return "schema".equals(element.getLocalName()); + return element != null && "schema".equals(element.getLocalName()); } public static FilesChangedTracker createFilesChangedTracker(SchemaGrammar grammar) { diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java index 5f1749faf7..4245c51111 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java @@ -943,6 +943,27 @@ public static LocationLink createLocationLink(Range origin, TargetRange target) return new LocationLink(target.getTargetURI(), targetRange, targetSelectionRange, origin); } + /** + * Returns the location link for the given origin and + * target nodes. + * + * @param origin the origin node. + * @param target the target node. + * @return the location link for the given origin and + * target nodes. + */ + public static LocationLink createLocationLink(DOMRange origin, TargetRange target) { + Range originSelectionRange = null; + if (origin instanceof DOMElement) { + originSelectionRange = selectStartTagName((DOMElement) origin); + } else { + originSelectionRange = XMLPositionUtility.createRange(origin); + } + Range targetRange = target.getTargetRange(); + Range targetSelectionRange = targetRange; + return new LocationLink(target.getTargetURI(), targetRange, targetSelectionRange, originSelectionRange); + } + /** * Returns the location for the given target node. * diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaTypeDefinitionExtensionsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaTypeDefinitionExtensionsTest.java index 73c49c2a6b..6f2b1f9f41 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaTypeDefinitionExtensionsTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaTypeDefinitionExtensionsTest.java @@ -145,4 +145,34 @@ public void localXSAttributeWithCatalog() throws BadLocationException, Malformed } + @Test + public void localXSElementOutsideXSComplexType() throws BadLocationException { + String xmlFile = "src/test/resources/Format.xml"; + String xsdFile = "xsd/Format.xsd"; + + String xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " false\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " "; + XMLLanguageService xmlLanguageService = new XMLLanguageService(); + String targetSchemaURI = xmlLanguageService.getResolverExtensionManager().resolve(xmlFile, null, xsdFile); + testTypeDefinitionFor(xmlLanguageService, xml, xmlFile, + ll(targetSchemaURI, r(18, 13, 18, 25), r(268, 23, 268, 37))); + } + } diff --git a/org.eclipse.lemminx/src/test/resources/xml/Format.xml b/org.eclipse.lemminx/src/test/resources/xml/Format.xml new file mode 100644 index 0000000000..86641038e3 --- /dev/null +++ b/org.eclipse.lemminx/src/test/resources/xml/Format.xml @@ -0,0 +1,31 @@ + + + + + false + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file