From 0344c8fc196811854b94ed4419bea790ceb55fb5 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Fri, 19 Mar 2021 14:13:03 -0400 Subject: [PATCH] Use related information for unclosed elements Revert MarkupEntityMismatch and ETagRequired error ranges so they only cover the start tag element name. Keep existing CodeAction behaviour intact. If the client supports DiagnosticRelatedInformation, add the expected location of the close tag as related info to MarkupEntityMismatch and ETagRequired errors. Closes #963 --- .../AggregateRelatedInfoFinder.java | 54 ++++++ .../participants/IRelatedInfoFinder.java | 35 ++++ .../participants/SyntaxErrorUtils.java | 41 +++++ .../participants/XMLSyntaxErrorCode.java | 28 ++-- .../XMLSyntaxRelatedInfoFinder.java | 67 ++++++++ .../codeactions/CloseTagCodeAction.java | 45 ++--- .../diagnostics/LSPErrorReporterForXML.java | 10 +- .../diagnostics/LSPSAXParser.java | 12 +- .../xerces/AbstractLSPErrorReporter.java | 31 +++- .../diagnostics/LSPErrorReporterForXSD.java | 6 +- .../XSDDiagnosticsParticipant.java | 2 +- .../diagnostics/XSDValidator.java | 6 +- .../contentmodel/DTDDiagnosticsTest.java | 4 +- .../XMLRelatedInfoFinderTest.java | 156 ++++++++++++++++++ .../XMLSyntaxDiagnosticsTest.java | 66 ++++---- 15 files changed, 465 insertions(+), 98 deletions(-) create mode 100644 org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/AggregateRelatedInfoFinder.java create mode 100644 org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/IRelatedInfoFinder.java create mode 100644 org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/SyntaxErrorUtils.java create mode 100644 org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxRelatedInfoFinder.java create mode 100644 org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLRelatedInfoFinderTest.java diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/AggregateRelatedInfoFinder.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/AggregateRelatedInfoFinder.java new file mode 100644 index 0000000000..c44e05a302 --- /dev/null +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/AggregateRelatedInfoFinder.java @@ -0,0 +1,54 @@ +/******************************************************************************* +* Copyright (c) 2021 Red Hat Inc. and others. +* All rights reserved. This program and the accompanying materials +* which accompanies this distribution, and is available at +* http://www.eclipse.org/legal/epl-v20.html +* +* Contributors: +* Red Hat Inc. - initial API and implementation +*******************************************************************************/ +package org.eclipse.lemminx.extensions.contentmodel.participants; + +import java.util.ArrayList; +import java.util.List; + +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lsp4j.DiagnosticRelatedInformation; + +/** + * Finds related info for any error code + */ +public class AggregateRelatedInfoFinder implements IRelatedInfoFinder { + + private static IRelatedInfoFinder[] RELATED_INFO_FINDERS = { + new XMLSyntaxRelatedInfoFinder() + }; + + private static AggregateRelatedInfoFinder INSTANCE = null; + + private AggregateRelatedInfoFinder() {} + + public static AggregateRelatedInfoFinder getInstance() { + if (INSTANCE == null) { + INSTANCE = new AggregateRelatedInfoFinder(); + } + return INSTANCE; + } + + @Override + public List findRelatedInformation( + int offset, + String errorKey, + DOMDocument document) { + List relatedInfo = new ArrayList<>(); + for (IRelatedInfoFinder relatedInfoFinder : RELATED_INFO_FINDERS) { + relatedInfo.addAll(relatedInfoFinder.findRelatedInformation( + offset, + errorKey, + document + )); + } + return relatedInfo; + } + +} diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/IRelatedInfoFinder.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/IRelatedInfoFinder.java new file mode 100644 index 0000000000..43a53bf700 --- /dev/null +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/IRelatedInfoFinder.java @@ -0,0 +1,35 @@ +/******************************************************************************* +* Copyright (c) 2021 Red Hat Inc. and others. +* All rights reserved. This program and the accompanying materials +* which accompanies this distribution, and is available at +* http://www.eclipse.org/legal/epl-v20.html +* +* Contributors: +* Red Hat Inc. - initial API and implementation +*******************************************************************************/ +package org.eclipse.lemminx.extensions.contentmodel.participants; + +import java.util.List; + +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lsp4j.DiagnosticRelatedInformation; + +/** + * Provdies an interface to find related info for a given error + */ +public interface IRelatedInfoFinder { + + /** + * Returns a list of related information + * + * @param offset The LemMinX reported error offset + * @param errorKey The error key + * @param document The document + * @return a list of related information + */ + List findRelatedInformation( + int offset, + String errorKey, + DOMDocument document); + +} diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/SyntaxErrorUtils.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/SyntaxErrorUtils.java new file mode 100644 index 0000000000..c0519f1b1e --- /dev/null +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/SyntaxErrorUtils.java @@ -0,0 +1,41 @@ +/******************************************************************************* +* Copyright (c) 2021 Red Hat Inc. and others. +* All rights reserved. This program and the accompanying materials +* which accompanies this distribution, and is available at +* http://www.eclipse.org/legal/epl-v20.html +* +* Contributors: +* Red Hat Inc. - initial API and implementation +*******************************************************************************/ +package org.eclipse.lemminx.extensions.contentmodel.participants; + +import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.dom.DOMElement; + +/** + * Reusable utility functions for resolving syntax errors + */ +public class SyntaxErrorUtils { + + /** + * Returns the offset at which the given unclosed start tag should be closed + * + * @param document The xml document with the unclosed start tag + * @param element The element that has an unclosed start tag + * @return the offset at which the given unclosed start tag should be closed + */ + public static int getUnclosedStartTagCloseLocation(DOMDocument document, DOMElement element) { + String documentText = document.getText(); + int i = element.getStart() + 1; + for (; i < documentText.length() && documentText.charAt(i) != '/' && documentText.charAt(i) != '<'; i++) { + } + if (i < documentText.length() && documentText.charAt(i) == '/') { + return i + 1; + } + i--; + for (; i > 0 && Character.isWhitespace(documentText.charAt(i)); i--) { + } + return i + 1; + } + +} diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java index a47fecd138..f94d4d8607 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java @@ -43,7 +43,7 @@ /** * XML error code. - * + * * @see https://wiki.xmldation.com/Support/Validator * */ @@ -120,7 +120,7 @@ public static XMLSyntaxErrorCode get(String name) { /** * Create the LSP range from the SAX error. - * + * * @param location * @param key * @param arguments @@ -181,11 +181,11 @@ public static Range toLSPRange(XMLLocator location, XMLSyntaxErrorCode code, Obj case ETagUnterminated: { /** * Cases: - * + * * - * + * * - * + * * findRelatedInformation(int offset, String errorKey, + DOMDocument document) { + + XMLSyntaxErrorCode syntaxCode = XMLSyntaxErrorCode.get(errorKey); + + if (syntaxCode == null) { + return Collections.emptyList(); + } + + switch (syntaxCode) { + case ETagRequired: + case MarkupEntityMismatch: { + DOMNode node = document.findNodeAt(offset); + while (node != null && !node.isElement()) { + node = node.getParentNode(); + } + if (node == null) { + return Collections.emptyList(); + } + + int closeTagOffset; + int numChildren = node.getChildren().size(); + if (numChildren == 0) { + closeTagOffset = node.getEnd(); + } else { + closeTagOffset = node.getChildren().get(numChildren - 1).getEnd(); + } + + Range range = XMLPositionUtility.createRange(closeTagOffset, closeTagOffset, document); + return Collections.singletonList(new DiagnosticRelatedInformation( + new Location(document.getDocumentURI(), range), CLOSING_TAG_EXPECTED_HERE)); + } + default: + } + return Collections.emptyList(); + } + +} diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/codeactions/CloseTagCodeAction.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/codeactions/CloseTagCodeAction.java index 38f48ebe38..e78a5c57a6 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/codeactions/CloseTagCodeAction.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/codeactions/CloseTagCodeAction.java @@ -19,6 +19,7 @@ import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMNode; import org.eclipse.lemminx.dom.LineIndentInfo; +import org.eclipse.lemminx.extensions.contentmodel.participants.SyntaxErrorUtils; import org.eclipse.lemminx.services.extensions.ICodeActionParticipant; import org.eclipse.lemminx.services.extensions.IComponentProvider; import org.eclipse.lemminx.settings.SharedSettings; @@ -34,6 +35,7 @@ */ public class CloseTagCodeAction implements ICodeActionParticipant { + @Override public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument document, List codeActions, SharedSettings sharedSettings, IComponentProvider componentProvider) { Range diagnosticRange = diagnostic.getRange(); @@ -63,12 +65,12 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen /** * Add code actions to fix unclosed end-tag. - * + * * @param element the end tag element to fix. * @param document the owner DOM document. * @param diagnostic the diagnostic. * @param codeActions the code actions list to fill. - * + * * @throws BadLocationException */ private void doCodeActionsForEndTagUnclosed(DOMElement element, DOMDocument document, Diagnostic diagnostic, @@ -84,30 +86,31 @@ private void doCodeActionsForEndTagUnclosed(DOMElement element, DOMDocument docu /** * Add code actions to fix unclosed start-tag. - * + * * @param element the start tag element to fix. * @param document the owner DOM document. * @param diagnosticRange the diagnostic range. * @param diagnostic the diagnostic. * @param codeActions the code actions list to fill. - * + * * @throws BadLocationException */ private void doCodeActionsForStartTagUnclosed(DOMElement element, DOMDocument document, Range diagnosticRange, Diagnostic diagnostic, List codeActions) throws BadLocationException { // Here start tag element is not closed with '>'. String text = document.getText(); + int closeAngleBracketOffset = SyntaxErrorUtils.getUnclosedStartTagCloseLocation(document, element); + final Position closeAngleBracketPosition = document.positionAt(closeAngleBracketOffset); if (!element.hasEndTag()) { // The element has no an end tag // ex : '", diagnosticRange.getEnd(), + CodeAction autoCloseAction = CodeActionFactory.insert("Close with '/>'", closeAngleBracketPosition, "/>", document.getTextDocument(), diagnostic); codeActions.add(autoCloseAction); // Close with '>' String insertText = ">"; CodeAction closeEndTagAction = CodeActionFactory.insert("Close with '" + insertText + "'", - diagnosticRange.getEnd(), insertText, document.getTextDocument(), diagnostic); + closeAngleBracketPosition, insertText, document.getTextDocument(), diagnostic); codeActions.add(closeEndTagAction); } } } else { // ex : - CodeAction autoCloseAction = insertGreaterThanCharacterCodeAction(document, diagnostic, diagnosticRange); + CodeAction autoCloseAction = insertGreaterThanCharacterCodeAction(document, diagnostic, closeAngleBracketPosition); codeActions.add(autoCloseAction); } } /** * Add code actions to fix closed start-tag. - * + * * @param element the start tag element to fix. * @param document the owner DOM document. * @param diagnostic the diagnostic. * @param codeActions the code actions list to fill. - * + * * @throws BadLocationException */ private void doCodeActionsForStartTagClosed(DOMElement element, DOMDocument document, Range diagnosticRange, @@ -233,22 +236,22 @@ private void doCodeActionsForStartTagClosed(DOMElement element, DOMDocument docu /** * Create a code action which insert '>' at the end of the diagnostic error. - * + * * @param diagnostic the diagnostic. - * @param document the DOM docment. - * @param diagnosticRange the diagnostic range + * @param document the DOM document. + * @param position the position where the '>' should be inserted * @return a code action which insert '>' at the end of the diagnostic error. */ private static CodeAction insertGreaterThanCharacterCodeAction(DOMDocument document, Diagnostic diagnostic, - Range diagnosticRange) { - CodeAction autoCloseAction = CodeActionFactory.insert("Close with '>'", diagnosticRange.getEnd(), ">", + Position position) { + CodeAction autoCloseAction = CodeActionFactory.insert("Close with '>'", position, ">", document.getTextDocument(), diagnostic); return autoCloseAction; } /** * Create a code action which remove the content of the given DOM element. - * + * * @param element the DOM element to remove. * @param document the DOM document. * @param diagnostic the diagnostic. @@ -269,7 +272,7 @@ private static CodeAction removeTagCodeAction(DOMElement element, DOMDocument do /** * Create a code action which replaces the end tag name of the given element * with the given replace tag name. - * + * * @param element the DOM element to replace * @param replaceTagName the replace tag name * @param diagnostic the diagnostic. @@ -297,9 +300,9 @@ private static CodeAction replaceEndTagNameCodeAction(DOMElement element, String /** * Returns true if the given element has elements as children and false * otherwise. - * + * * @param element the DOM element. - * + * * @return true if the given element has elements as children and false * otherwise. */ diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java index ec06d9956b..18b7067ad1 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java @@ -56,7 +56,7 @@ public class LSPErrorReporterForXML extends AbstractLSPErrorReporter { public LSPErrorReporterForXML(DOMDocument xmlDocument, List diagnostics, ContentModelManager contentModelManager, boolean hasRelatedInformation, Map referencedGrammarDiagnosticsInfoCache) { - super(XML_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics); + super(XML_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics, hasRelatedInformation); this.contentModelManager = contentModelManager; this.hasRelatedInformation = hasRelatedInformation; this.referencedGrammarDiagnosticsInfoCache = referencedGrammarDiagnosticsInfoCache == null ? new HashMap<>() @@ -65,7 +65,7 @@ public LSPErrorReporterForXML(DOMDocument xmlDocument, List diagnost /** * Create the LSP range from the SAX error. - * + * * @param location * @param key * @param arguments @@ -137,7 +137,7 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments, /** * Create a diagnostic root where XSD, DTD which have the error if needed and * attach the error as related information if LSP client support it. - * + * * @param location the Xerces location. * @param key the Xerces error key * @param arguments the Xerces error arguments @@ -191,7 +191,7 @@ private void fillReferencedGrammarDiagnostic(XMLLocator location, String key, Ob /** * Returns the referenced grammar diagnostics info from the given grammar URI. - * + * * @param grammarURI the referenced grammar URI. * @param resolverExtensionManager the resolver used to load the DOM document of * the referenced grammar. @@ -204,7 +204,7 @@ private ReferencedGrammarDiagnosticsInfo getReferencedGrammarDiagnosticsInfo(Str // Create diagnostic where DDT, XSD which have errors is declared Range range = getReferencedGrammarRange(grammarURI); String message = ""; - Diagnostic diagnostic = super.addDiagnostic(range, message, DiagnosticSeverity.Error, null); + Diagnostic diagnostic = super.addDiagnostic(range, message, DiagnosticSeverity.Error, null, null); // Register the diagnostic as root diagnostic for the XSD, DTD grammar uri info = new ReferencedGrammarDiagnosticsInfo(grammarURI, resolverExtensionManager, diagnostic); referencedGrammarDiagnosticsInfoCache.put(grammarURI, info); diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java index e67b77de1c..5bc5a63793 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPSAXParser.java @@ -43,7 +43,7 @@ /** * Extension of Xerces SAX Parser to fix some Xerces bugs: - * + * *
    *
  • [BUG 1]: when the DTD file path is wrong on DOCTYPE, Xerces breaks all * validation like syntax validation
    • @@ -51,7 +51,7 @@ * ignore the existing of entities. See * https://github.com/redhat-developer/vscode-xml/issues/234 *
- * + * * @author Angelo ZERR * */ @@ -122,7 +122,7 @@ public void doctypeDecl(String rootElement, String publicId, String systemId, Au Range range = new Range(document.positionAt(docType.getSystemIdNode().getStart()), document.positionAt(docType.getSystemIdNode().getEnd())); reporter.addDiagnostic(range, MessageFormat.format(DTD_NOT_FOUND, expandedSystemId), - DiagnosticSeverity.Error, DTDErrorCode.dtd_not_found.getCode()); + DiagnosticSeverity.Error, DTDErrorCode.dtd_not_found.getCode(), null); } catch (BadLocationException e) { // Do nothing } @@ -158,7 +158,7 @@ public void doctypeDecl(String rootElement, String publicId, String systemId, Au /** * Create DTD grammar description by expanding the system id. - * + * * @param rootElement the root element * @param publicId the public ID. * @param systemId the system ID. @@ -178,7 +178,7 @@ private XMLDTDDescription createGrammarDescription(String rootElement, String pu * Resolve the expanded system ID by resolving the system ID of the given * grammar description with uri resolver (XML catalog, cache, etc with * {@link URIResolverExtensionManager}). - * + * * @param grammarDesc the DTD grammar description. * @param entityManager the entity manager. * @return the expanded system ID by resolving the system ID of the given @@ -213,7 +213,7 @@ private static boolean isDTDExists(String expandedSystemId) { /** * Fill entities from the given DTD grammar to the given entity manager. - * + * * @param grammar the DTD grammar * @param entityManager the entitymanager to update with entities of the DTD * grammar. diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/AbstractLSPErrorReporter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/AbstractLSPErrorReporter.java index 71b2357218..908274cbc6 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/AbstractLSPErrorReporter.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/AbstractLSPErrorReporter.java @@ -30,8 +30,10 @@ import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.commons.TextDocument; import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.extensions.contentmodel.participants.AggregateRelatedInfoFinder; import org.eclipse.lemminx.extensions.xerces.xmlmodel.msg.XMLModelMessageFormatter; import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.DiagnosticRelatedInformation; import org.eclipse.lsp4j.DiagnosticSeverity; import org.eclipse.lsp4j.Position; import org.eclipse.lsp4j.Range; @@ -53,11 +55,13 @@ public abstract class AbstractLSPErrorReporter extends XMLErrorReporter { private final List diagnostics; private final String source; + private boolean hasRelatedInfo; - public AbstractLSPErrorReporter(String source, DOMDocument xmlDocument, List diagnostics) { + public AbstractLSPErrorReporter(String source, DOMDocument xmlDocument, List diagnostics, boolean hasRelatedInfo) { this.source = source; this.xmlDocument = xmlDocument; this.diagnostics = diagnostics; + this.hasRelatedInfo = hasRelatedInfo; XMLMessageFormatter xmft = new XMLMessageFormatter(); super.putMessageFormatter(XMLMessageFormatter.XML_DOMAIN, xmft); super.putMessageFormatter(XMLMessageFormatter.XMLNS_DOMAIN, xmft); @@ -94,10 +98,18 @@ public String reportError(XMLLocator location, String domain, String key, Object DiagnosticSeverity diagnosticSeverity = toLSPSeverity(severity); Range adjustedRange = internalToLSPRange(location, key, arguments, message, diagnosticSeverity, fatalError, xmlDocument); + List relatedInformations = null; if (adjustedRange == null || NO_RANGE.equals(adjustedRange)) { return null; } - if (addDiagnostic(adjustedRange, message, diagnosticSeverity, key) == null) { + if (hasRelatedInfo) { + try { + relatedInformations = AggregateRelatedInfoFinder.getInstance().findRelatedInformation(xmlDocument.offsetAt(adjustedRange.getStart()), key, xmlDocument); + } catch (BadLocationException e) { + LOGGER.severe("Passed bad Range: " + e); + } + } + if (addDiagnostic(adjustedRange, message, diagnosticSeverity, key, relatedInformations) == null) { return null; } if (fatalError && !fContinueAfterFatalError) { @@ -112,8 +124,11 @@ protected boolean isIgnoreFatalError(String key) { return false; } - public Diagnostic addDiagnostic(Range adjustedRange, String message, DiagnosticSeverity severity, String key) { + public Diagnostic addDiagnostic(Range adjustedRange, String message, DiagnosticSeverity severity, String key, List relatedInformation) { Diagnostic d = new Diagnostic(adjustedRange, message, severity, source, key); + if (hasRelatedInfo && relatedInformation != null && relatedInformation.size() > 0){ + d.setRelatedInformation(relatedInformation); + } if (diagnostics.contains(d)) { return null; } @@ -124,7 +139,7 @@ public Diagnostic addDiagnostic(Range adjustedRange, String message, DiagnosticS /** * Returns the LSP diagnostic severity according the SAX severity. - * + * * @param severity the SAX severity * @return the LSP diagnostic severity according the SAX severity. */ @@ -139,7 +154,7 @@ private static DiagnosticSeverity toLSPSeverity(int severity) { /** * Create the LSP range from the SAX error. - * + * * @param location the Xerces location. * @param key the Xerces error key. * @param arguments the Xerces error arguments. @@ -182,7 +197,7 @@ protected Range createDefaultRange(XMLLocator location, DOMDocument document) { /** * Returns the range of the given error information, or {{@link #NO_RANGE} if * diagnostic must not be created and null otherwise. - * + * * @param location the Xerces location. * @param key the Xerces error key. * @param arguments the Xerces error arguments. @@ -198,7 +213,7 @@ protected abstract Range toLSPRange(XMLLocator location, String key, Object[] ar /** * Returns the LSP position from the SAX location. - * + * * @param offset the adjusted offset. * @param location the original SAX location. * @param document the text document. @@ -217,7 +232,7 @@ private static Position toLSPPosition(int offset, XMLLocator location, TextDocum /** * Returns the DOM document which is validating. - * + * * @return the DOM document which is validating. */ protected DOMDocument getDOMDocument() { diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/LSPErrorReporterForXSD.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/LSPErrorReporterForXSD.java index 2f674f0af7..e9c86f9205 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/LSPErrorReporterForXSD.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/LSPErrorReporterForXSD.java @@ -35,13 +35,13 @@ public class LSPErrorReporterForXSD extends AbstractLSPErrorReporter { private static final String XSD_DIAGNOSTIC_SOURCE = "xsd"; - public LSPErrorReporterForXSD(DOMDocument xmlDocument, List diagnostics) { - super(XSD_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics); + public LSPErrorReporterForXSD(DOMDocument xmlDocument, List diagnostics, boolean hasRelatedInfo) { + super(XSD_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics, hasRelatedInfo); } /** * Create the LSP range from the SAX error. - * + * * @param location * @param key * @param arguments diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDDiagnosticsParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDDiagnosticsParticipant.java index f678efd3fd..6df5871d77 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDDiagnosticsParticipant.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDDiagnosticsParticipant.java @@ -39,7 +39,7 @@ public void doDiagnostics(DOMDocument xmlDocument, List diagnostics, // associations settings., ...) XMLEntityResolver entityResolver = xmlDocument.getResolverExtensionManager(); // Process validation - XSDValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics, cancelChecker); + XSDValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics, validationSettings.isRelatedInformation(), cancelChecker); } } diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDValidator.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDValidator.java index b91943e09f..013a820297 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDValidator.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsd/participants/diagnostics/XSDValidator.java @@ -46,10 +46,10 @@ public class XSDValidator { private static boolean canCustomizeReporter = true; public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityResolver, - List diagnostics, CancelChecker monitor) { + List diagnostics, boolean isRelatedInformation, CancelChecker monitor) { try { - XMLErrorReporter reporter = new LSPErrorReporterForXSD(document, diagnostics); + XMLErrorReporter reporter = new LSPErrorReporterForXSD(document, diagnostics, isRelatedInformation); XMLGrammarPreparser grammarPreparser = new LSPXMLGrammarPreparser(); XMLSchemaLoader schemaLoader = createSchemaLoader(reporter); @@ -97,7 +97,7 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR /** * Create the XML Schema loader to use to validate the XML Schema. - * + * * @param reporter the lsp reporter. * @return the XML Schema loader to use to validate the XML Schema. * @throws NoSuchFieldException diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java index bb9ed09770..dfd234ac8f 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java @@ -634,7 +634,7 @@ public void testDTDNotFoundWithSYSTEM() throws Exception { XMLAssert.testDiagnosticsFor(xml, d(1, 29, 1, 46, DTDErrorCode.dtd_not_found), // [1] d(2, 1, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [2] d(5, 4, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [3] - d(5, 4, 5, 21, XMLSyntaxErrorCode.ETagRequired)); // [4] + d(5, 4, 12, XMLSyntaxErrorCode.ETagRequired)); // [4] } @Test @@ -651,7 +651,7 @@ public void testDTDNotFoundWithPUBLIC() throws Exception { XMLAssert.testDiagnosticsFor(xml, d(1, 33, 1, 50, DTDErrorCode.dtd_not_found), // [1] d(2, 1, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [2] d(5, 4, 12, DTDErrorCode.MSG_ELEMENT_NOT_DECLARED), // [3] - d(5, 4, 5, 21, XMLSyntaxErrorCode.ETagRequired)); // [4] + d(5, 4, 12, XMLSyntaxErrorCode.ETagRequired)); // [4] } @Test diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLRelatedInfoFinderTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLRelatedInfoFinderTest.java new file mode 100644 index 0000000000..632d932c2b --- /dev/null +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLRelatedInfoFinderTest.java @@ -0,0 +1,156 @@ +/******************************************************************************* +* Copyright (c) 2021 Red Hat Inc. and others. +* All rights reserved. This program and the accompanying materials +* which accompanies this distribution, and is available at +* http://www.eclipse.org/legal/epl-v20.html +* +* Contributors: +* Red Hat Inc. - initial API and implementation +*******************************************************************************/ +package org.eclipse.lemminx.extensions.contentmodel; + +import static org.eclipse.lemminx.XMLAssert.d; +import static org.eclipse.lemminx.XMLAssert.l; +import static org.eclipse.lemminx.XMLAssert.r; + +import java.util.Arrays; + +import org.eclipse.lemminx.XMLAssert; +import org.eclipse.lemminx.extensions.contentmodel.participants.XMLSyntaxErrorCode; +import org.eclipse.lemminx.extensions.contentmodel.settings.ContentModelSettings; +import org.eclipse.lemminx.extensions.contentmodel.settings.XMLValidationSettings; +import org.eclipse.lemminx.services.XMLLanguageService; +import org.eclipse.lsp4j.Diagnostic; +import org.eclipse.lsp4j.DiagnosticRelatedInformation; +import org.eclipse.lsp4j.DiagnosticSeverity; +import org.eclipse.lsp4j.PublishDiagnosticsCapabilities; +import org.junit.jupiter.api.Test; + +public class XMLRelatedInfoFinderTest { + + final String XML_FILE_PATH = "src/test/resources/test.xml"; + + @Test + public void eTagRequiredRelatedInfo() { + String xml = // + "\n" + // + " \n" + // + ""; + + Diagnostic diagnostic = d(1, 3, 1, 7, + XMLSyntaxErrorCode.ETagRequired, + "The element type \"boot\" must be terminated by the matching end-tag \"\"."); + diagnostic.setRelatedInformation(Arrays.asList( + new DiagnosticRelatedInformation(l(XML_FILE_PATH, r(2, 0, 2, 0)), "") + )); + diagnostic.setSeverity(DiagnosticSeverity.Error); + diagnostic.setSource("xml"); + + assertDiagnosticsWithRelatedInfo(xml, diagnostic); + } + + @Test + public void eTagRequiredIncompleteClosing1RelatedInfo() { + String xml = // + "\n" + // + " "; + + Diagnostic diagnostic = d(1, 3, 1, 7, + XMLSyntaxErrorCode.ETagRequired, + "The element type \"boot\" must be terminated by the matching end-tag \"\"."); + diagnostic.setRelatedInformation(Arrays.asList( + new DiagnosticRelatedInformation(l(XML_FILE_PATH, r(1, 10, 1, 10)), "") + )); + diagnostic.setSeverity(DiagnosticSeverity.Error); + diagnostic.setSource("xml"); + + assertDiagnosticsWithRelatedInfo(xml, diagnostic); + } + + @Test + public void eTagRequiredIncompleteClosing2RelatedInfo() { + String xml = // + "\n" + // + " "; + + Diagnostic diagnostic = d(1, 3, 1, 7, + XMLSyntaxErrorCode.ETagRequired, + "The element type \"boot\" must be terminated by the matching end-tag \"\"."); + diagnostic.setRelatedInformation(Arrays.asList( + new DiagnosticRelatedInformation(l(XML_FILE_PATH, r(2, 0, 2, 0)), "") + )); + diagnostic.setSeverity(DiagnosticSeverity.Error); + diagnostic.setSource("xml"); + + assertDiagnosticsWithRelatedInfo(xml, diagnostic); + } + + @Test + public void markupEntityMismatchRelatedInfoWithNestedElement() { + String xml = // + "\n" + // + " "; + + Diagnostic diagnostic = d(0, 1, 0, 5, + XMLSyntaxErrorCode.MarkupEntityMismatch, + "XML document structures must start and end within the same entity."); + diagnostic.setRelatedInformation(Arrays.asList( + new DiagnosticRelatedInformation(l(XML_FILE_PATH, r(1, 8, 1, 8)), "") + )); + diagnostic.setSeverity(DiagnosticSeverity.Error); + diagnostic.setSource("xml"); + + assertDiagnosticsWithRelatedInfo(xml, diagnostic); + } + + @Test + public void markupEntityMismatchRelatedInfoWithAttributes() { + String xml = // + "\n" + // + " \n" + // + "Name\r\n" + // " \r\n" + // " "; - Diagnostic d = d(1, 5, 2, 2, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 5, 1, 7, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(1, 12, 1, 12, ""))); } @@ -285,7 +285,7 @@ public void testETagRequired2() throws Exception { String xml = "\r\n" + // " Nm>Name\r\n" + // " "; - testDiagnosticsFor(xml, d(0, 1, 1, 11, XMLSyntaxErrorCode.ETagRequired)); + testDiagnosticsFor(xml, d(0, 1, 0, 10, XMLSyntaxErrorCode.ETagRequired)); } @Test @@ -295,7 +295,7 @@ public void testETagRequired3() throws Exception { " \r\n" + // " \r\n" + // ""; - Diagnostic d = d(3, 5, 4, 0, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(3, 5, 3, 7, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(3, 8, 3, 8, ""))); } @@ -305,7 +305,7 @@ public void testETagRequiredWithReplace() throws Exception { String xml = "\r\n" + // " \r\n" + // " "; - Diagnostic d = d(1, 2, 2, 2, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 2, 1, 3, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, // ca(d, te(2, 4, 2, 5, "b")), ca(d, te(2, 6, 2, 6, "\r\n "))); @@ -316,7 +316,7 @@ public void testETagRequiredWithText() throws Exception { String xml = "\r\n" + // "def\r\n" + // ""; - Diagnostic d = d(1, 1, 2, 0, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 1, 1, 4, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(1, 8, 1, 8, ""))); } @@ -327,7 +327,7 @@ public void testETagRequiredWithOrpheanEndTag() throws Exception { " \r\n" + // " "; - Diagnostic d = d(1, 2, 2, 2, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 2, 1, 5, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(2, 4, 2, 4, "foo>"))); } @@ -339,7 +339,7 @@ public void testETagRequiredClosedWithOrpheanEndTag() throws Exception { " \r\n" + // ""; - Diagnostic d = d(1, 2, 2, 2, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 2, 1, 5, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(2, 2, 2, 4, ""))); } @@ -351,14 +351,14 @@ public void testETagRequiredClosedWithOrpheanEndTag2() throws Exception { " \r\n" + // " \r\n" + // ""; - Diagnostic d = d(1, 2, 2, 2, XMLSyntaxErrorCode.ETagRequired); + Diagnostic d = d(1, 2, 1, 5, XMLSyntaxErrorCode.ETagRequired); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(2, 2, 2, 8, ""))); } /** * Test ETagUnterminated - * + * * @see https://wiki.xmldation.com/Support/Validator/ETagUnterminated * @throws Exception */ @@ -373,7 +373,7 @@ public void testETagUnterminated() throws Exception { /** * Test ETagUnterminated - * + * * @see https://wiki.xmldation.com/Support/Validator/ETagUnterminated * @throws Exception */ @@ -391,7 +391,7 @@ public void testETagUnterminated2() throws Exception { /** * Test ETagUnterminated - * + * * @see https://wiki.xmldation.com/Support/Validator/ETagUnterminated * @throws Exception */ @@ -405,7 +405,7 @@ public void testETagUnterminated3() throws Exception { /** * Test ETagUnterminated - * + * * @see https://wiki.xmldation.com/Support/Validator/ETagUnterminated * @throws Exception */ @@ -449,7 +449,7 @@ public void testMarkupEntityMismatch() throws Exception { + // "\r\n" + // ""; - Diagnostic d = d(1, 1, 3, 19, XMLSyntaxErrorCode.MarkupEntityMismatch); + Diagnostic d = d(1, 1, 1, 9, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(3, 19, 3, 19, "\r\n"))); } @@ -457,7 +457,7 @@ public void testMarkupEntityMismatch() throws Exception { @Test public void testMarkupEntityMismatch2() throws Exception { String xml = ""; - Diagnostic d = d(0, 1, 0, 5, XMLSyntaxErrorCode.MarkupEntityMismatch); + Diagnostic d = d(0, 1, 0, 4, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(0, 5, 0, 5, ""))); } @@ -465,7 +465,7 @@ public void testMarkupEntityMismatch2() throws Exception { @Test public void testMarkupEntityMismatch3() throws Exception { String xml = "<"; - Diagnostic d = d(0, 0, 0, 1, XMLSyntaxErrorCode.MarkupEntityMismatch); + Diagnostic d = d(0, 1, 0, 1, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d); // no code actions } @@ -473,7 +473,7 @@ public void testMarkupEntityMismatch3() throws Exception { @Test public void testMarkupEntityMismatch4() throws Exception { String xml = "")), // @@ -529,7 +529,7 @@ public void testMarkupEntityMismatchWithAttributesAndSlash() throws Exception { @Test public void testMarkupEntityMismatchWithText() throws Exception { String xml = "def"; - Diagnostic d = d(0, 1, 0, 8, XMLSyntaxErrorCode.MarkupEntityMismatch); + Diagnostic d = d(0, 1, 0, 4, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(0, 8, 0, 8, ""))); } @@ -537,7 +537,7 @@ public void testMarkupEntityMismatchWithText() throws Exception { @Test public void testMarkupEntityMismatchWithTextAndNewLine() throws Exception { String xml = "def\r\n"; - Diagnostic d = d(0, 1, 0, 8, XMLSyntaxErrorCode.MarkupEntityMismatch); + Diagnostic d = d(0, 1, 0, 4, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(0, 8, 0, 8, ""))); } @@ -546,7 +546,7 @@ public void testMarkupEntityMismatchWithTextAndNewLine() throws Exception { public void testMarkupEntityMismatchMultiLine() throws Exception { String xml = ""))); @@ -765,12 +765,12 @@ public void closeTag() throws Exception { ca(d, te(0, 2, 0, 2, ">"))); xml = ""; - d = d(0, 1, 0, 3, XMLSyntaxErrorCode.MarkupEntityMismatch); + d = d(0, 1, 0, 2, XMLSyntaxErrorCode.MarkupEntityMismatch); testDiagnosticsFor(xml, d); testCodeActionsFor(xml, d, ca(d, te(0, 3, 0, 3, ""))); xml = ""))); @@ -780,7 +780,7 @@ public void closeTag() throws Exception { testCodeActionsFor(xml, d, ca(d, te(0, 4, 0, 4, ">"))); xml = ""))); }