Skip to content

Commit

Permalink
Improve ETagRequired error range
Browse files Browse the repository at this point in the history 
Fixes eclipse-lemminx#876

Signed-off-by: azerr <azerr@redhat.com>
  • Loading branch information
@angelozerr
angelozerr committed Sep 18, 2020
1 parent 3ea8abb commit 5ad8b61
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 44 deletions.
6 changes: 5 additions & 1 deletion org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,11 @@ public boolean isEndTagClosed() {
* tag, eg: </a>) and false otherwise.
*/
public boolean isOrphanEndTag(String tagName) {
return isSameTag(tagName) && hasEndTag() && !hasStartTag();
return isSameTag(tagName) && isOrphanEndTag();
}

public boolean isOrphanEndTag() {
return hasEndTag() && !hasStartTag();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...ain/java/org/eclipse/lemminx/extensions/contentmodel/participants/XMLSyntaxErrorCode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ public static Range toLSPRange(XMLLocator location, XMLSyntaxErrorCode code, Obj
return XMLPositionUtility.selectEndTagName(offset, document);
case ETagRequired: {
String tag = getString(arguments[0]);
return XMLPositionUtility.selectChildEndTag(tag, offset, document);
return XMLPositionUtility.selectStartTagNameOfUnclosedElement(tag, offset, document);
}
case SemicolonRequiredInReference: {
EntityReferenceRange range = XMLPositionUtility.selectEntityReference(offset + 1, document, false);
Expand Down
40 changes: 28 additions & 12 deletions ...pse/lemminx/extensions/contentmodel/participants/codeactions/CloseStartTagCodeAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,9 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen
if (node != null && node.isElement()) {
int diagnosticEndOffset = document.offsetAt(diagnosticRange.getEnd());
DOMElement element = (DOMElement) node;
if (!element.hasStartTag()) {
if (element.isOrphanEndTag()) {
// </foo>
DOMElement parent = element.getParentElement();
if (parent != null && parent.getTagName() != null) {
// <a><b></c>
// Replace with 'b' closing tag
String tagName = parent.getTagName();
Range replaceRange = XMLPositionUtility.selectEndTagName(element);
CodeAction replaceTagAction = CodeActionFactory.replace("Replace with '" + tagName + "' closing tag",
replaceRange, tagName, document.getTextDocument(), diagnostic);
codeActions.add(replaceTagAction);
}
addReplaceCodeAction(element, diagnostic, codeActions);
} else {
// <foo>
boolean startTagClosed = element.isStartTagClosed();
Expand Down Expand Up @@ -99,7 +90,16 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen
endPosition, insertText, document.getTextDocument(), diagnostic);
codeActions.add(closeEndTagAction);
}

// Search orphean elements in the children
List<DOMNode> children = element.getChildren();
for (DOMNode child : children) {
if (child.isElement()) {
DOMElement childElement = (DOMElement) child;
if (childElement.isOrphanEndTag()) {
addReplaceCodeAction(childElement, diagnostic, codeActions);
}
}
}
} else {
// ex : <foo attr="
// Close with '/>
Expand Down Expand Up @@ -131,6 +131,22 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen
}
}

private static void addReplaceCodeAction(DOMElement element, Diagnostic diagnostic, List<CodeAction> codeActions) {
DOMElement parent = element.getParentElement();
if (parent != null && parent.getTagName() != null) {
// <a><b></c>
// Replace with 'b' closing tag
DOMDocument document = element.getOwnerDocument();
String replaceTagName = parent.getTagName();
Range replaceRange = XMLPositionUtility.selectEndTagName(element);
String tagName = element.getTagName();
CodeAction replaceTagAction = CodeActionFactory.replace(
"Replace '" + tagName + "' with '" + replaceTagName + "' closing tag", replaceRange, replaceTagName,
document.getTextDocument(), diagnostic);
codeActions.add(replaceTagAction);
}
}

/**
* Returns true if the given element has elements as children and false
* otherwise.
Expand Down
57 changes: 29 additions & 28 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/utils/XMLPositionUtility.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,46 +266,47 @@ private static int adjustOffsetForAttribute(int offset, DOMDocument document) {

// ------------ Element selection

public static Range selectChildEndTag(String childTag, int offset, DOMDocument document) {
DOMNode parent = document.findNodeAt(offset);
if (parent == null || !parent.isElement() || ((DOMElement) parent).getTagName() == null) {
public static Range selectStartTagNameOfUnclosedElement(String tagName, int offset, DOMDocument document) {
DOMNode node = document.findNodeAt(offset);
if (node == null || !node.isElement() || ((DOMElement) node).getTagName() == null) {
return null;
}

DOMNode curr = parent;
DOMNode child;
while (curr != null) {
child = findUnclosedChildNode(childTag, curr.getChildren());
if (child == null) {
curr = findUnclosedChildNode(curr.getChildren());
} else {
return createRange(child.getStart() + 1, child.getStart() + 1 + childTag.length(), document);
}
DOMElement parent = (DOMElement) node;
if (!parent.hasStartTag()) {
parent = parent.getParentElement();
}

String parentName = ((DOMElement) parent).getTagName();
return createRange(parent.getStart() + 2, parent.getStart() + 2 + parentName.length(), document);
}

private static DOMNode findUnclosedChildNode(List<DOMNode> children) {
for (DOMNode child : children) {
if (!child.isClosed()) {
return child;
}
DOMElement element = findUnclosedElement(tagName, parent);
if (element != null) {
return selectStartTagName(element);
}
return null;
}

private static DOMNode findUnclosedChildNode(String childTag, List<DOMNode> children) {
for (DOMNode child : children) {
if (child.isElement() && childTag != null && childTag.equals(((DOMElement) child).getTagName())
&& !child.isClosed()) {
return child;
private static DOMElement findUnclosedElement(String tagName, DOMElement parent) {
List<DOMNode> children = parent.getChildren();
for (int i = children.size() - 1; i >= 0; i--) {
DOMNode child = parent.getChild(i);
if (child.isElement()) {
DOMElement element = (DOMElement) child;
DOMElement element2 = findUnclosedElement(tagName, element);
if (element2 != null) {
return element2;
}
if (isUnclosedStartTag(element, tagName)) {
return element;
}
}
}
if (isUnclosedStartTag(parent, tagName)) {
return parent;
}
return null;
}

private static boolean isUnclosedStartTag(DOMElement element, String tagName) {
return (!element.isClosed() && element.hasStartTag() && element.isSameTag(tagName));
}

/**
* Returns the range of the root start tag (excludes the '<') of the given
* <code>document</code> and null otherwise.
Expand Down
18 changes: 16 additions & 2 deletions ...x/src/test/java/org/eclipse/lemminx/extensions/contentmodel/XMLSyntaxDiagnosticsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,24 @@ public void testETagRequiredWithReplace() throws Exception {
String xml = "<a>\r\n" + //
" <b>\r\n" + //
" </c>";
Diagnostic d = d(2, 4, 2, 5, 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 </b>")), // code action for close </b>
ca(d, te(2, 4, 2, 5, "b") // code action for replacing </c> with </b>
));
}

@Test
public void testETagRequired4() throws Exception {
String xml = "<foo>\r\n" + //
" <bar></\r\n" + //
"</foo>";
Diagnostic d = d(1, 3, 1, 6, XMLSyntaxErrorCode.ETagRequired);
testDiagnosticsFor(xml, d);
// testCodeActionsFor(xml, d, //
// ca(d, te(2, 6, 2, 6, "\r\n </b>")) // code action for close </b>
//);
}

/**
Expand Down

0 comments on commit 5ad8b61

Please sign in to comment.