Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenQuoteExpected error for ATTLIST breaks DTD validation #1599

Merged
merged 1 commit into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand Down Expand Up @@ -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())));

}
Expand Down