Prevent from NPE when there's a validation error

Fixes eclipse-lemminx#927 Signed-off-by: azerr <azerr@redhat.com>
angelozerr · Nov 10, 2020 · c1a2ea3 · c1a2ea3
1 parent 28685d8
commit c1a2ea3
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 21 deletions.
diff --git a/.../src/main/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogURIResolverExtension.java b/.../src/main/java/org/eclipse/lemminx/extensions/catalog/XMLCatalogURIResolverExtension.java
@@ -33,7 +33,7 @@ public class XMLCatalogURIResolverExtension implements URIResolverExtension {
 	 */
 	private static final String CATALOG_NAMESPACE_URI = "urn:oasis:names:tc:entity:xmlns:xml:catalog"; //$NON-NLS-1$
 
-	private static final String CATALOG_SYSTEM = "http://www.oasis-open.org/committees/entity/release/1.1/catalog.xsd";
+	private static final String CATALOG_SYSTEM = "https://www.oasis-open.org/committees/entity/release/1.1/catalog.xsd";
 
 	private final XMLExtensionsRegistry extensionsRegistry;
 

diff --git a/...ipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java b/...ipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPErrorReporterForXML.java
@@ -49,15 +49,18 @@ public class LSPErrorReporterForXML extends AbstractLSPErrorReporter {
 
 	private Set<ReferencedGrammarInfo> referencedGrammars;
 
-	private Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache;
+	private final Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache;
 
 	private final boolean hasRelatedInformation;
 
 	public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnostics,
-			ContentModelManager contentModelManager, boolean hasRelatedInformation) {
+			ContentModelManager contentModelManager, boolean hasRelatedInformation,
+			Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache) {
 		super(XML_DIAGNOSTIC_SOURCE, xmlDocument, diagnostics);
 		this.contentModelManager = contentModelManager;
 		this.hasRelatedInformation = hasRelatedInformation;
+		this.referencedGrammarDiagnosticsInfoCache = referencedGrammarDiagnosticsInfoCache == null ? new HashMap<>()
+				: referencedGrammarDiagnosticsInfoCache;
 	}
 
 	/**
@@ -72,8 +75,8 @@ public LSPErrorReporterForXML(DOMDocument xmlDocument, List<Diagnostic> diagnost
 	@Override
 	protected Range toLSPRange(XMLLocator location, String key, Object[] arguments, String message,
 			DiagnosticSeverity diagnosticSeverity, boolean fatalError, DOMDocument document) {
-		String documentURI = location.getExpandedSystemId();
-		boolean errorForDocument = documentURI.endsWith(document.getDocumentURI());
+		String documentOrGrammarURI = location.getExpandedSystemId();
+		boolean errorForDocument = documentOrGrammarURI != null ? documentOrGrammarURI.endsWith(document.getDocumentURI()) : true;
 		// try adjust positions for XML syntax error
 		XMLSyntaxErrorCode syntaxCode = XMLSyntaxErrorCode.get(key);
 		if (syntaxCode != null) {
@@ -84,7 +87,7 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments,
 				}
 			} else {
 				fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity, fatalError,
-						document.getResolverExtensionManager(), syntaxCode, null, null, documentURI);
+						document.getResolverExtensionManager(), syntaxCode, null, null, documentOrGrammarURI);
 				return NO_RANGE;
 			}
 		} else {
@@ -106,20 +109,19 @@ protected Range toLSPRange(XMLLocator location, String key, Object[] arguments,
 						}
 					} else {
 						fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity,
-								fatalError, document.getResolverExtensionManager(), null, dtdCode, null, documentURI);
+								fatalError, document.getResolverExtensionManager(), null, dtdCode, null, documentOrGrammarURI);
 						return NO_RANGE;
 					}
 				} else {
 					XSDErrorCode xsdCode = XSDErrorCode.get(key);
-					if (xsdCode != null) {
+					if (xsdCode != null && !errorForDocument) {
 						// 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();
 						fillReferencedGrammarDiagnostic(location, key, arguments, message, diagnosticSeverity,
-								fatalError, document.getResolverExtensionManager(), null, null, xsdCode, grammarURI);
+								fatalError, document.getResolverExtensionManager(), null, null, xsdCode, documentOrGrammarURI);
 						return NO_RANGE;
 					}
 				}
@@ -197,9 +199,6 @@ private void fillReferencedGrammarDiagnostic(XMLLocator location, String key, Ob
 	 */
 	private ReferencedGrammarDiagnosticsInfo getReferencedGrammarDiagnosticsInfo(String grammarURI,
 			URIResolverExtensionManager resolverExtensionManager) {
-		if (referencedGrammarDiagnosticsInfoCache == null) {
-			referencedGrammarDiagnosticsInfoCache = new HashMap<>();
-		}
 		ReferencedGrammarDiagnosticsInfo info = referencedGrammarDiagnosticsInfoCache.get(grammarURI);
 		if (info == null) {
 			// Create diagnostic where DDT, XSD which have errors is declared
@@ -244,7 +243,7 @@ protected boolean isIgnoreFatalError(String key) {
 	}
 
 	public void endReport() {
-		if (referencedGrammarDiagnosticsInfoCache == null) {
+		if (referencedGrammarDiagnosticsInfoCache.isEmpty()) {
 			return;
 		}
 		// When a XML is validated by a DTD or XSD which have syntax error, the XSD, DTD

diff --git a/...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/XMLValidator.java b/...va/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/XMLValidator.java
@@ -14,6 +14,7 @@
 
 import java.io.IOException;
 import java.io.StringReader;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CancellationException;
@@ -57,14 +58,17 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
 			List<Diagnostic> diagnostics, XMLValidationSettings validationSettings,
 			ContentModelManager contentModelManager, CancelChecker monitor) {
 		XMLGrammarPool grammarPool = contentModelManager.getGrammarPool();
+		Map<String, ReferencedGrammarDiagnosticsInfo> referencedGrammarDiagnosticsInfoCache = new HashMap<>();
 		final LSPErrorReporterForXML reporterForXML = new LSPErrorReporterForXML(document, diagnostics,
-				contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false);
+				contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false,
+				referencedGrammarDiagnosticsInfoCache);
 		// When referenced grammar (XSD, DTD) have an error (ex : syntax error), the
 		// error must be reported.
 		// We create a reporter for grammar since Xerces reporter stores the XMLLocator
 		// for XML and Grammar.
 		final LSPErrorReporterForXML reporterForGrammar = new LSPErrorReporterForXML(document, diagnostics,
-				contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false);
+				contentModelManager, validationSettings != null ? validationSettings.isRelatedInformation() : false,
+				referencedGrammarDiagnosticsInfoCache);
 		try {
 			LSPXMLParserConfiguration configuration = new LSPXMLParserConfiguration(grammarPool,
 					isDisableOnlyDTDValidation(document), reporterForXML, reporterForGrammar, validationSettings);

diff --git a/...c/main/java/org/eclipse/lemminx/extensions/dtd/participants/diagnostics/DTDValidator.java b/...c/main/java/org/eclipse/lemminx/extensions/dtd/participants/diagnostics/DTDValidator.java
@@ -15,6 +15,7 @@
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.util.HashMap;
 import java.util.List;
 
 import org.apache.xerces.impl.dtd.XMLDTDLoader;
@@ -37,7 +38,7 @@ public static void doDiagnostics(DOMDocument document, XMLEntityResolver entityR
 		try {
 			XMLDTDLoader loader = new XMLDTDLoader();
 			loader.setProperty("http://apache.org/xml/properties/internal/error-reporter",
-					new LSPErrorReporterForXML(document, diagnostics, contentModelManager, false));
+					new LSPErrorReporterForXML(document, diagnostics, contentModelManager, false, new HashMap<>()));
 
 			if (entityResolver != null) {
 				loader.setEntityResolver(entityResolver);

diff --git a/...x/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaDiagnosticsTest.java b/...x/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSchemaDiagnosticsTest.java
@@ -962,16 +962,20 @@ public void diagnosticRelatedInformationWithNoNamespaceSchemaLocationSyntaxProbl
 		String xsdFileURI = getGrammarFileURI("foo-invalid-syntax.xsd");
 		diagnostic.getRelatedInformation().add(new DiagnosticRelatedInformation(l(xsdFileURI, r(1, 1, 1, 54)), ""));
 
+		Diagnostic diagnosticBasedOnXSD = new Diagnostic(r(0, 1, 0, 4),
+				"cvc-elt.1.a: Cannot find the declaration of element 'foo'.", DiagnosticSeverity.Error, "xml",
+				XMLSchemaErrorCode.cvc_elt_1_a.getCode());
+
 		XMLLanguageService xmlLanguageService = new XMLLanguageService();
 		// First validation
 		XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
 				settings, //
-				diagnostic);
+				diagnostic, diagnosticBasedOnXSD);
 		// Restart the validation to check the validation is working since Xerces cache
 		// the invalid XSD grammar
 		XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
 				settings, //
-				diagnostic);
+				diagnostic, diagnosticBasedOnXSD);
 	}
 
 	@Test
@@ -994,16 +998,24 @@ public void diagnosticRelatedInformationWithSchemaLocationSyntaxProblem() throws
 		String xsdFileURI = getGrammarFileURI("foo-ns-invalid-syntax.xsd");
 		diagnostic.getRelatedInformation().add(new DiagnosticRelatedInformation(l(xsdFileURI, r(1, 1, 4, 29)), ""));
 
+		Diagnostic diagnosticBasedOnXSD1 = new Diagnostic(r(1, 8, 1, 20),
+				"TargetNamespace.1: Expecting namespace 'http://foo', but the target namespace of the schema document is 'xs:element name=\"foo\">'.",
+				DiagnosticSeverity.Error, "xml", XMLSchemaErrorCode.TargetNamespace_1.getCode());
+
+		Diagnostic diagnosticBasedOnXSD2 = new Diagnostic(r(0, 1, 0, 4),
+				"cvc-elt.1.a: Cannot find the declaration of element 'foo'.", DiagnosticSeverity.Error, "xml",
+				XMLSchemaErrorCode.cvc_elt_1_a.getCode());
+
 		XMLLanguageService xmlLanguageService = new XMLLanguageService();
 		// First validation
 		XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
 				settings, //
-				diagnostic);
+				diagnostic, diagnosticBasedOnXSD1, diagnosticBasedOnXSD2);
 		// Restart the validation to check the validation is working since Xerces cache
 		// the invalid XSD grammar
 		XMLAssert.testDiagnosticsFor(xmlLanguageService, xml, null, null, "src/test/resources/test.xml", false,
 				settings, //
-				diagnostic);
+				diagnostic, diagnosticBasedOnXSD1, diagnosticBasedOnXSD2);
 	}
 
 	@Test