OpenQuoteExpected error for ATTLIST breaks DTD validation

Fixes redhat-developer/vscode-xml#943 Signed-off-by: azerr <[email protected]>
angelozerr · Nov 15, 2023 · de31af8 · de31af8
1 parent d1d67ec
commit de31af8
Show file tree

Hide file tree

Showing 8 changed files with 146 additions and 21 deletions.
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLTextDocumentService.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/XMLTextDocumentService.java
@@ -355,9 +355,11 @@ public CompletableFuture<List<? extends TextEdit>> rangeFormatting(DocumentRange
 	}
 
 	@Override
-	public CompletableFuture<Either3<Range, PrepareRenameResult, PrepareRenameDefaultBehavior>> prepareRename(PrepareRenameParams params) {
+	public CompletableFuture<Either3<Range, PrepareRenameResult, PrepareRenameDefaultBehavior>> prepareRename(
+			PrepareRenameParams params) {
 		return computeDOMAsync(params.getTextDocument(), (xmlDocument, cancelChecker) -> {
-			Either<Range, PrepareRenameResult> either = getXMLLanguageService().prepareRename(xmlDocument, params.getPosition(), cancelChecker);
+			Either<Range, PrepareRenameResult> either = getXMLLanguageService().prepareRename(xmlDocument,
+					params.getPosition(), cancelChecker);
 			if (either != null) {
 				if (either.isLeft()) {
 					return Either3.forFirst((Range) either.get());
@@ -554,11 +556,13 @@ private XMLFormattingOptions getIndentationSettings(@NonNull String uri) {
 
 			newOptions = new XMLFormattingOptions();
 			newOptions.merge(sharedSettings.getFormattingSettings());
-			if (indentationSettings.get(0) != null && (indentationSettings.get(0) instanceof JsonPrimitive)) {
-				newOptions.setInsertSpaces(((JsonPrimitive) indentationSettings.get(0)).getAsBoolean());
-			}
-			if (indentationSettings.get(1) != null && (indentationSettings.get(1) instanceof JsonPrimitive)) {
-				newOptions.setTabSize(((JsonPrimitive) indentationSettings.get(1)).getAsInt());
+			if (!indentationSettings.isEmpty()) {
+				if (indentationSettings.get(0) != null && (indentationSettings.get(0) instanceof JsonPrimitive)) {
+					newOptions.setInsertSpaces(((JsonPrimitive) indentationSettings.get(0)).getAsBoolean());
+				}
+				if (indentationSettings.get(1) != null && (indentationSettings.get(1) instanceof JsonPrimitive)) {
+					newOptions.setTabSize(((JsonPrimitive) indentationSettings.get(1)).getAsInt());
+				}
 			}
 		} catch (Exception e) {
 			LOGGER.log(Level.SEVERE, "Error while processing getting indentation settings for code actions'.", e);

diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMNode.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMNode.java
@@ -289,6 +289,23 @@ public static DOMAttr findAttrAt(DOMNode node, int offset) {
 		return null;
 	}
 
+	public DTDDeclParameter findDTDDeclParameterAt(int offset) {
+		DOMNode node = findNodeAt(offset);
+		return findDTDDeclParameterAt(node, offset);
+	}
+
+	public static DTDDeclParameter findDTDDeclParameterAt(DOMNode node, int offset) {
+		if (node != null && (node.isDTDAttListDecl() || node.isDTDElementDecl() || node.isDTDEntityDecl()
+				|| node.isDTDNotationDecl())) {
+			for (DTDDeclParameter parameter : ((DTDDeclNode) node).getParameters()) {
+				if (isIncluded(parameter, offset)) {
+					return parameter;
+				}
+			}
+		}
+		return null;
+	}
+
 	public static DOMText findTextAt(DOMNode node, int offset) {
 		if (node != null && node.hasChildNodes()) {
 			for (DOMNode child : node.getChildren()) {
@@ -299,7 +316,7 @@ public static DOMText findTextAt(DOMNode node, int offset) {
 		}
 		return null;
 	}
-	
+
 	public static DOMNode findNodeOrAttrAt(DOMDocument document, int offset) {
 		DOMNode node = document.findNodeAt(offset);
 		if (node != null) {

diff --git a/...ain/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java b/...ain/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java
@@ -25,6 +25,8 @@
 import org.eclipse.lemminx.dom.DOMDocumentType;
 import org.eclipse.lemminx.dom.DOMElement;
 import org.eclipse.lemminx.dom.DOMNode;
+import org.eclipse.lemminx.dom.DTDDeclNode;
+import org.eclipse.lemminx.dom.DTDDeclParameter;
 import org.eclipse.lemminx.extensions.contentmodel.participants.codeactions.ETagRequiredCodeAction;
 import org.eclipse.lemminx.extensions.contentmodel.participants.codeactions.ETagUnterminatedCodeAction;
 import org.eclipse.lemminx.extensions.contentmodel.participants.codeactions.ElementUnterminatedCodeAction;
@@ -283,6 +285,13 @@ public static Range toLSPRange(XMLLocator location, XMLSyntaxErrorCode code, Obj
 		case NameRequiredInReference:
 			break;
 		case OpenQuoteExpected: {
+			DOMNode node = document.findNodeAt(offset);
+			if (node instanceof DTDDeclNode) {
+				// ex : <!ATTLIST dadesadministratives idinstitut ID > <-- error on idinstitut which must be quoted.
+				String parameterName = getString(arguments[1] /* idinstitut*/ );
+				return XMLPositionUtility.selectParameterNameFromGivenName(parameterName, (DTDDeclNode) node);
+			}
+			// ex : <foo bar=value > <-- error on value which must be quoted.
 			return XMLPositionUtility.selectAttributeNameAt(offset - 1, document);
 		}
 		case DoctypeNotAllowed:

diff --git a/...lemminx/extensions/contentmodel/participants/codeactions/OpenQuoteExpectedCodeAction.java b/...lemminx/extensions/contentmodel/participants/codeactions/OpenQuoteExpectedCodeAction.java
@@ -18,6 +18,7 @@
 import org.eclipse.lemminx.dom.DOMAttr;
 import org.eclipse.lemminx.dom.DOMDocument;
 import org.eclipse.lemminx.dom.DOMNode;
+import org.eclipse.lemminx.dom.DTDDeclParameter;
 import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant;
 import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest;
 import org.eclipse.lemminx.settings.SharedSettings;
@@ -43,10 +44,26 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
 		} catch (BadLocationException e) {
 			return;
 		}
-		DOMAttr attr = document.findAttrAt(offset);
-		if (attr == null || !attr.isAttribute()) {
-			return;
+		DOMNode attr = document.findAttrAt(offset);
+		if (attr != null && attr.isAttribute()) {
+			// ex : <foo bar=value > <-- error on value which must be quoted.
+			insertQuotationForAttr(attr, document, request, diagnostic, codeActions);
+		} else {
+			DTDDeclParameter parameter = document.findDTDDeclParameterAt(offset);
+			if (parameter != null) {
+				// ex : <!ATTLIST dadesadministratives idinstitut ID > <-- error on idinstitut
+				// which must be quoted.
+
+				// We disable this code action, since it generates DTD code which is not valid
+				// We need to study more usecases of https://www.w3.org/TR/REC-xml/#NT-AttlistDecl
+				// to provide more relevant code actions.
+				// insertQuotationForParameter(parameter, document, request, diagnostic, codeActions);
+			}
 		}
+	}
+
+	private static void insertQuotationForAttr(DOMNode attr, DOMDocument document, ICodeActionRequest request,
+			Diagnostic diagnostic, List<CodeAction> codeActions) {
 		SharedSettings sharedSettings = request.getSharedSettings();
 		String q = sharedSettings.getPreferences().getQuotationAsString();
 		Position codeactionPosition;
@@ -77,4 +94,14 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
 		codeActions.add(removeContentAction);
 	}
 
+	private static void insertQuotationForParameter(DTDDeclParameter parameter, DOMDocument document,
+			ICodeActionRequest request, Diagnostic diagnostic, List<CodeAction> codeActions) {
+		SharedSettings sharedSettings = request.getSharedSettings();
+		String q = sharedSettings.getPreferences().getQuotationAsString();
+		CodeAction insertQuotationsAction = CodeActionFactory.replace("Insert quotations",
+				parameter.getTargetRange(), q + parameter.getParameter() + q,
+				document.getTextDocument(), diagnostic);
+		codeActions.add(insertQuotationsAction);
+	}
+
 }
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
@@ -923,6 +923,16 @@ public static Range getElementDeclMissingContentOrCategory(int offset, DOMDocume
 		}
 		return null;
 	}
+
+	public static Range selectParameterNameFromGivenName(String parameterName, DTDDeclNode declNode) {
+		List<DTDDeclParameter> parameters = declNode.getParameters();
+		for (DTDDeclParameter parameter : parameters) {
+			if (parameterName.equals(parameter.getParameter())) {
+				return createRange(parameter.getStart(), parameter.getEnd(), declNode.getOwnerDocument());
+			}
+		}
+		return null;
+	}
 
 	// ------------ Other selection
 

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
@@ -1065,11 +1065,15 @@ public static void assertCodeActions(List<CodeAction> actual, CodeAction... expe
 	}
 
 	public static CodeAction ca(Diagnostic d, TextEdit... te) {
+		return ca(d, FILE_URI, te);
+	}
+
+	public static CodeAction ca(Diagnostic d, String fileUri, TextEdit... te) {
 		CodeAction codeAction = new CodeAction();
 		codeAction.setTitle("");
 		codeAction.setDiagnostics(Arrays.asList(d));
 
-		TextDocumentEdit textDocumentEdit = tde(FILE_URI, 0, te);
+		TextDocumentEdit textDocumentEdit = tde(fileUri, 0, te);
 		WorkspaceEdit workspaceEdit = new WorkspaceEdit(Collections.singletonList(Either.forLeft(textDocumentEdit)));
 		codeAction.setEdit(workspaceEdit);
 		return codeAction;

diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDiagnosticsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/dtd/DTDDiagnosticsTest.java
@@ -0,0 +1,61 @@
+/*******************************************************************************
+* Copyright (c) 2023 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
+*
+* SPDX-License-Identifier: EPL-2.0
+*
+* Contributors:
+*     Red Hat Inc. - initial API and implementation
+*******************************************************************************/
+package org.eclipse.lemminx.extensions.dtd;
+
+import static org.eclipse.lemminx.XMLAssert.ca;
+import static org.eclipse.lemminx.XMLAssert.d;
+import static org.eclipse.lemminx.XMLAssert.te;
+import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor;
+
+import org.eclipse.lemminx.AbstractCacheBasedTest;
+import org.eclipse.lemminx.XMLAssert;
+import org.eclipse.lemminx.extensions.contentmodel.participants.DTDErrorCode;
+import org.eclipse.lemminx.extensions.contentmodel.settings.ContentModelSettings;
+import org.eclipse.lemminx.services.XMLLanguageService;
+import org.eclipse.lsp4j.Diagnostic;
+import org.junit.jupiter.api.Test;
+
+/**
+ * DTD file diagnostics.
+ *
+ */
+public class DTDDiagnosticsTest extends AbstractCacheBasedTest {
+
+	@Test
+	public void EntityDeclUnterminated() throws Exception {
+		String dtd = "<!ENTITY copyright \"Copyright W3Schools.\"\r\n" + //
+				"<!ELEMENT element-name (#PCDATA)>";
+		testDiagnosticsFor(dtd, "test.dtd", d(0, 41, 42, DTDErrorCode.EntityDeclUnterminated));
+	}
+
+	@Test
+	public void OpenQuoteExpected() throws Exception {
+		String dtd = "<!ATTLIST dadesadministratives idinstitut ID >";
+		Diagnostic d = d(0, 31, 41, DTDErrorCode.OpenQuoteExpected);
+		testDiagnosticsFor(dtd, "test.dtd", d);
+		// testCodeActionsFor(dtd, "test.dtd", d, ca(d, "test.dtd", te(0, 31, 0, 41, "\"idinstitut\"")));
+	}
+
+	public static void testDiagnosticsFor(String xml, String fileURI, Diagnostic... expected) {
+		XMLAssert.testDiagnosticsFor(xml, null, null, fileURI, true, expected);
+	}
+
+	public static void testDiagnosticsFor(String xml, String fileURI, ContentModelSettings settings,
+			Diagnostic... expected) {
+		XMLAssert.testDiagnosticsFor(xml, null, null, fileURI, true, settings, expected);
+	}
+
+	public static void testDiagnosticsFor(XMLLanguageService ls, String xml, String fileURI,
+			ContentModelSettings settings, Diagnostic... expected) {
+		XMLAssert.testDiagnosticsFor(ls, xml, null, null, fileURI, true, settings, expected);
+	}
+}
diff --git a/.../src/test/java/org/eclipse/lemminx/extensions/dtd/DTDValidationExternalResourcesTest.java b/.../src/test/java/org/eclipse/lemminx/extensions/dtd/DTDValidationExternalResourcesTest.java
@@ -13,7 +13,6 @@
 package org.eclipse.lemminx.extensions.dtd;
 
 import static org.eclipse.lemminx.XMLAssert.ca;
-import static org.eclipse.lemminx.XMLAssert.d;
 import static org.eclipse.lemminx.XMLAssert.pd;
 import static org.eclipse.lemminx.XMLAssert.r;
 import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor;
@@ -40,13 +39,6 @@
  */
 public class DTDValidationExternalResourcesTest extends AbstractCacheBasedTest {
 
-	@Test
-	public void EntityDeclUnterminated() throws Exception {
-		String xml = "<!ENTITY copyright \"Copyright W3Schools.\"\r\n" + //
-				"<!ELEMENT element-name (#PCDATA)>";
-		testDiagnosticsFor(xml, "test.dtd", d(0, 41, 42, DTDErrorCode.EntityDeclUnterminated));
-	}
-
 	@Test
 	public void entityRefInvalidUri() throws Exception {
 
@@ -137,7 +129,8 @@ public void entityRefDownloadProblem() throws Exception {
 						new Diagnostic(r(0, 24, 0, 46), "Cannot find DTD 'file:///tmp/secret.txt'.",
 								DiagnosticSeverity.Error, "xml", DTDErrorCode.DTDNotFound.getCode()),
 						new Diagnostic(r(1, 53, 1, 80),
-								"Error while downloading 'http://server:8080/dtd.xml?' to '" + dtdCachePath + "' : '[java.net.UnknownHostException] server'.",
+								"Error while downloading 'http://server:8080/dtd.xml?' to '" + dtdCachePath
+										+ "' : '[java.net.UnknownHostException] server'.",
 								DiagnosticSeverity.Error, "xml", ExternalResourceErrorCode.DownloadProblem.getCode())));
 
 	}