Skip to content

Commit

Permalink
XML validation should aggregate XSD errors where is referenced
Browse files Browse the repository at this point in the history 
Fixes eclipse-lemminx#768

Signed-off-by: azerr <azerr@redhat.com>
  • Loading branch information
@angelozerr
angelozerr committed Oct 15, 2020
1 parent af09659 commit 5bdab81
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 28 deletions.
3 changes: 1 addition & 2 deletions .../extensions/contentmodel/participants/diagnostics/ContentModelDiagnosticsParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ public void doDiagnostics(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
XMLEntityResolver entityResolver = xmlDocument.getResolverExtensionManager();
// Process validation
XMLValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics,
contentModelPlugin.getContentModelSettings(),
contentModelPlugin.getContentModelManager().getGrammarPool(), monitor);
contentModelPlugin.getContentModelSettings(), contentModelPlugin.getContentModelManager(), monitor);
}

}
132 changes: 127 additions & 5 deletions ...ipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,29 @@
*/
package org.eclipse.lemminx.extensions.contentmodel.participants.diagnostics;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;

import org.apache.xerces.xni.XMLLocator;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMRange;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.extensions.contentmodel.model.ReferencedGrammarInfo;
import org.eclipse.lemminx.extensions.contentmodel.participants.DTDErrorCode;
import org.eclipse.lemminx.extensions.contentmodel.participants.XMLSchemaErrorCode;
import org.eclipse.lemminx.extensions.contentmodel.participants.XMLSyntaxErrorCode;
import org.eclipse.lemminx.extensions.xerces.AbstractLSPErrorReporter;
import org.eclipse.lemminx.extensions.xsd.participants.XSDErrorCode;
import org.eclipse.lemminx.uriresolver.URIResolverExtensionManager;
import org.eclipse.lemminx.utils.DOMUtils;
import org.eclipse.lemminx.utils.XMLPositionUtility;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticRelatedInformation;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.eclipse.lsp4j.Location;
import org.eclipse.lsp4j.Range;

/**
Expand All @@ -31,8 +45,39 @@ public class LSPErrorReporterForXML extends AbstractLSPErrorReporter {

private static final String XML_DIAGNOSTIC_SOURCE = "xml";

public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnostics) {
private static class GrammarInfo {
private final DOMDocument document;

private final Diagnostic diagnostic;

public GrammarInfo(DOMDocument document, Diagnostic diagnostic) {
super();
this.document = document;
this.diagnostic = diagnostic;
}

public DOMDocument getDocument() {
return document;
}

public void addDiagnosticRelatedInformation(DiagnosticRelatedInformation relatedInformation) {
if (diagnostic.getRelatedInformation() == null) {
diagnostic.setRelatedInformation(new ArrayList<>());
}
diagnostic.getRelatedInformation().add(relatedInformation);
}
}

private final ContentModelManager contentModelManager;

private Set<ReferencedGrammarInfo> referencedGrammars;

private Map<String, GrammarInfo> domReferencedGrammars;

public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
ContentModelManager contentModelManager) {
super(XML_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics);
this.contentModelManager = contentModelManager;
}

/**
Expand All @@ -45,7 +90,8 @@ public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnost
* @return the LSP range from the SAX error.
*/
@Override
protected Range toLSPRange(XMLLocator location, String key, Object[] arguments, DOMDocument document) {
protected Range toLSPRange(XMLLocator location, String key, Object[] arguments, String message,
DiagnosticSeverity diagnosticSeverity, DOMDocument document) {
// try adjust positions for XML syntax error
XMLSyntaxErrorCode syntaxCode = XMLSyntaxErrorCode.get(key);
if (syntaxCode != null) {
Expand All @@ -65,16 +111,92 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments,
// try adjust positions for DTD error
DTDErrorCode dtdCode = DTDErrorCode.get(key);
if (dtdCode != null) {
Range range = DTDErrorCode.toLSPRange(location, dtdCode, arguments, document);
if (range != null) {
return range;
String loc = location.getExpandedSystemId();
if (document.getDocumentURI().equals(loc)) {
Range range = DTDErrorCode.toLSPRange(location, dtdCode, arguments, document);
if (range != null) {
return range;
}
} else {
String grammarURI = location.getExpandedSystemId();
GrammarInfo info = getGrammarInfo(grammarURI, document.getResolverExtensionManager());
if (info.getDocument() != null) {
Range dtdRange = DTDErrorCode.toLSPRange(location, dtdCode, arguments, info.getDocument());
Location dtdLocation = null;
if (dtdRange != null) {
dtdLocation = new Location(grammarURI, dtdRange);
}
DiagnosticRelatedInformation r = new DiagnosticRelatedInformation(dtdLocation, message);
info.addDiagnosticRelatedInformation(r);
}
return NO_RANGE;
}
} else {
XSDErrorCode xsdCode = XSDErrorCode.get(key);
if (xsdCode != null) {
// The error comes from the referenced XSD (with xsi:schemaLocation, xml-model,
// etc)

// Try to get the declared xsi:schemaLocation, xsi:noNamespaceLocation range
// which declares the XSD.
String grammarURI = location.getExpandedSystemId();
GrammarInfo info = getGrammarInfo(grammarURI, document.getResolverExtensionManager());
if (info.getDocument() != null) {
Range xsdRange = XSDErrorCode.toLSPRange(location, xsdCode, arguments, info.getDocument());
Location xsdLocation = null;
if (xsdRange != null) {
xsdLocation = new Location(grammarURI, xsdRange);
}
DiagnosticRelatedInformation r = new DiagnosticRelatedInformation(xsdLocation, message);
info.addDiagnosticRelatedInformation(r);
}
return NO_RANGE;
}
}
}
}
return null;
}

private GrammarInfo getGrammarInfo(String grammarURI, URIResolverExtensionManager resolverExtensionManager) {
if (domReferencedGrammars == null) {
domReferencedGrammars = new HashMap<>();
}
GrammarInfo info = domReferencedGrammars.get(grammarURI);
if (info == null) {
DOMDocument document = DOMUtils.loadDocument(grammarURI, resolverExtensionManager);
Range range = getReferencedGrammarRange(document, grammarURI);
String message = "Error from grammar '" + grammarURI + ".";
Diagnostic diagnostic = super.addDiagnostic(range, message, DiagnosticSeverity.Error, "");
info = new GrammarInfo(document, diagnostic);
domReferencedGrammars.put(grammarURI, info);
}
return info;
}

private Range getReferencedGrammarRange(DOMDocument document, String grammarURI) {
Set<ReferencedGrammarInfo> referencedGrammars = getReferencedGrammars();
for (ReferencedGrammarInfo referencedGrammarInfo : referencedGrammars) {
if (grammarURI.equals(referencedGrammarInfo.getResolvedURIInfo().getResolvedURI())) {
DOMRange range = referencedGrammarInfo.getIdentifier() != null
? referencedGrammarInfo.getIdentifier().getRange()
: null;
if (range != null) {
return XMLPositionUtility.createRange(range);
}
}
}
// Set the error range in the root start tag
return XMLPositionUtility.selectRootStartTag(document);
}

private Set<ReferencedGrammarInfo> getReferencedGrammars() {
if (referencedGrammars != null) {
return referencedGrammars;
}
return referencedGrammars = contentModelManager.getReferencedGrammarInfos(super.getDOMDocument());
}

@Override
protected boolean isIgnoreFatalError(String key) {
// Don't stop the validation when there are
Expand Down
12 changes: 8 additions & 4 deletions ...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/XMLValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMDocumentType;
import org.eclipse.lemminx.dom.DOMElement;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.extensions.contentmodel.participants.XMLSyntaxErrorCode;
import org.eclipse.lemminx.extensions.contentmodel.settings.ContentModelSettings;
import org.eclipse.lemminx.extensions.contentmodel.settings.XMLValidationSettings;
Expand All @@ -52,9 +53,10 @@ public class XMLValidator {
private static final Logger LOGGER = Logger.getLogger(XMLValidator.class.getName());

public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityResolver,
List<Diagnostic> diagnostics, ContentModelSettings contentModelSettings, XMLGrammarPool grammarPool,
CancelChecker monitor) {
List<Diagnostic> diagnostics, ContentModelSettings contentModelSettings,
ContentModelManager contentModelManager, CancelChecker monitor) {
try {
XMLGrammarPool grammarPool = contentModelManager.getGrammarPool();
XMLValidationSettings validationSettings = contentModelSettings != null
? contentModelSettings.getValidation()
: null;
Expand All @@ -65,7 +67,8 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
configuration.setProperty("http://apache.org/xml/properties/internal/entity-resolver", entityResolver); //$NON-NLS-1$
}

final LSPErrorReporterForXML reporter = new LSPErrorReporterForXML(document, diagnostics);
final LSPErrorReporterForXML reporter = new LSPErrorReporterForXML(document, diagnostics,
contentModelManager);

SAXParser parser = new LSPSAXParser(document, reporter, configuration, grammarPool);

Expand Down Expand Up @@ -188,7 +191,8 @@ private static void warnNoGrammar(DOMDocument document, List<Diagnostic> diagnos
private static void checkExternalSchema(Map<String, String> result, SAXParser reader)
throws SAXNotRecognizedException, SAXNotSupportedException {
if (result != null) {
String noNamespaceSchemaLocation = result.get(IExternalGrammarLocationProvider.NO_NAMESPACE_SCHEMA_LOCATION);
String noNamespaceSchemaLocation = result
.get(IExternalGrammarLocationProvider.NO_NAMESPACE_SCHEMA_LOCATION);
if (noNamespaceSchemaLocation != null) {
reader.setProperty(IExternalGrammarLocationProvider.NO_NAMESPACE_SCHEMA_LOCATION,
noNamespaceSchemaLocation);
Expand Down
4 changes: 2 additions & 2 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/DTDPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,13 @@
*/
public class DTDPlugin implements IXMLExtension {

private final IDiagnosticsParticipant diagnosticsParticipant;
private IDiagnosticsParticipant diagnosticsParticipant;
private final IDefinitionParticipant definitionParticipant;
private final IHighlightingParticipant highlightingParticipant;
private final IReferenceParticipant referenceParticipant;
private final ICodeLensParticipant codeLensParticipant;

public DTDPlugin() {
diagnosticsParticipant = new DTDDiagnosticsParticipant();
definitionParticipant = new DTDDefinitionParticipant();
highlightingParticipant = new DTDHighlightingParticipant();
referenceParticipant = new DTDReferenceParticipant();
Expand All @@ -55,6 +54,7 @@ public void start(InitializeParams params, XMLExtensionsRegistry registry) {
ContentModelManager modelManager = registry.getComponent(ContentModelManager.class);
modelManager.registerModelProvider(modelProvider);
// register diagnostic participant
diagnosticsParticipant = new DTDDiagnosticsParticipant(modelManager);
registry.registerDiagnosticsParticipant(diagnosticsParticipant);
// register definition participant
registry.registerDefinitionParticipant(definitionParticipant);
Expand Down
9 changes: 8 additions & 1 deletion ...rg/eclipse/lemminx/extensions/dtd/participants/diagnostics/DTDDiagnosticsParticipant.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import org.apache.xerces.xni.parser.XMLEntityResolver;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.services.extensions.diagnostics.IDiagnosticsParticipant;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.jsonrpc.CancelChecker;
Expand All @@ -26,6 +27,12 @@
*/
public class DTDDiagnosticsParticipant implements IDiagnosticsParticipant {

private final ContentModelManager modelManager;

public DTDDiagnosticsParticipant(ContentModelManager modelManager) {
this.modelManager = modelManager;
}

@Override
public void doDiagnostics(DOMDocument xmlDocument, List<Diagnostic> diagnostics, CancelChecker monitor) {
if (!xmlDocument.isDTD()) {
Expand All @@ -36,7 +43,7 @@ public void doDiagnostics(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
// associations settings., ...)
XMLEntityResolver entityResolver = xmlDocument.getResolverExtensionManager();
// Process validation
DTDValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics, monitor);
DTDValidator.doDiagnostics(xmlDocument, entityResolver, diagnostics, modelManager, monitor);
}

}
5 changes: 3 additions & 2 deletions ...c/main/java/org/eclipse/lemminx/extensions/dtd/participants/diagnostics/DTDValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.apache.xerces.xni.parser.XMLEntityResolver;
import org.apache.xerces.xni.parser.XMLInputSource;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
import org.eclipse.lemminx.extensions.contentmodel.participants.diagnostics.LSPErrorReporterForXML;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.jsonrpc.CancelChecker;
Expand All @@ -32,11 +33,11 @@
public class DTDValidator {

public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityResolver,
List<Diagnostic> diagnostics, CancelChecker monitor) {
List<Diagnostic> diagnostics, ContentModelManager contentModelManager, CancelChecker monitor) {
try {
XMLDTDLoader loader = new XMLDTDLoader();
loader.setProperty("http://apache.org/xml/properties/internal/error-reporter",
new LSPErrorReporterForXML(document, diagnostics));
new LSPErrorReporterForXML(document, diagnostics, contentModelManager));

if (entityResolver != null) {
loader.setEntityResolver(entityResolver);
Expand Down
33 changes: 22 additions & 11 deletions ...lemminx/src/main/java/org/eclipse/lemminx/extensions/xerces/AbstractLSPErrorReporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
*/
public abstract class AbstractLSPErrorReporter extends XMLErrorReporter {

protected final static Range NO_RANGE = new Range();
private final DOMDocument xmlDocument;
private final List<Diagnostic> diagnostics;

Expand Down Expand Up @@ -81,17 +82,20 @@ public String reportError(XMLLocator location, String domain, String key, Object
message = str.toString();
}

Range adjustedRange = internalToLSPRange(location, key, arguments, xmlDocument);

DiagnosticSeverity diagnosticSeverity = toLSPSeverity(severity);
Range adjustedRange = internalToLSPRange(location, key, arguments, message, diagnosticSeverity, xmlDocument);
if (adjustedRange == null) {
return null;
}
if (adjustedRange.equals(NO_RANGE)) {
return message;
}

if (!addDiagnostic(adjustedRange, message, toLSPSeverity(severity), key)) {
if (addDiagnostic(adjustedRange, message, diagnosticSeverity, key) == null) {
return null;
}

if (severity == SEVERITY_FATAL_ERROR && !fContinueAfterFatalError && !isIgnoreFatalError(key)) {
if (severity == SEVERITY_FATAL_ERROR && !fContinueAfterFatalError && key.equals("MSG_CLOSE_PAREN_REQUIRED_IN_CHILDREN")) {
XMLParseException parseException = (exception != null) ? new XMLParseException(location, message, exception)
: new XMLParseException(location, message);
throw parseException;
Expand All @@ -103,14 +107,14 @@ protected boolean isIgnoreFatalError(String key) {
return false;
}

public boolean addDiagnostic(Range adjustedRange, String message, DiagnosticSeverity severity, String key) {
public Diagnostic addDiagnostic(Range adjustedRange, String message, DiagnosticSeverity severity, String key) {
Diagnostic d = new Diagnostic(adjustedRange, message, severity, source, key);
if (diagnostics.contains(d)) {
return false;
return null;
}
// Fill diagnostic
diagnostics.add(d);
return true;
return d;
}

/**
Expand All @@ -134,17 +138,20 @@ private static DiagnosticSeverity toLSPSeverity(int severity) {
* @param location
* @param key
* @param arguments
* @param diagnosticSeverity
* @param message
* @param document
* @return the LSP range from the SAX error.
*/
private Range internalToLSPRange(XMLLocator location, String key, Object[] arguments, DOMDocument document) {
private Range internalToLSPRange(XMLLocator location, String key, Object[] arguments, String message,
DiagnosticSeverity diagnosticSeverity, DOMDocument document) {
if (location == null) {
Position start = toLSPPosition(0, location, document.getTextDocument());
Position end = toLSPPosition(0, location, document.getTextDocument());
return new Range(start, end);
}

Range range = toLSPRange(location, key, arguments, document);
Range range = toLSPRange(location, key, arguments, message, diagnosticSeverity, document);
if (range != null) {
return range;
}
Expand All @@ -161,8 +168,8 @@ private Range internalToLSPRange(XMLLocator location, String key, Object[] argum
return new Range(start, end);
}

protected abstract Range toLSPRange(XMLLocator location, String key, Object[] arguments, DOMDocument document);

protected abstract Range toLSPRange(XMLLocator location, String key, Object[] arguments, String message,
DiagnosticSeverity diagnosticSeverity, DOMDocument document);

/**
* Returns the LSP position from the SAX location.
Expand All @@ -182,4 +189,8 @@ private static Position toLSPPosition(int offset, XMLLocator location, TextDocum
return location != null ? new Position(location.getLineNumber() - 1, location.getColumnNumber() - 1) : null;
}
}

protected DOMDocument getDOMDocument() {
return xmlDocument;
}
}
Loading

0 comments on commit 5bdab81

Please sign in to comment.