Skip to content

Commit

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

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr
angelozerr committed Sep 23, 2020
1 parent 3ea8abb commit 4de695b
Show file tree
Hide file tree
Showing 7 changed files with 288 additions and 109 deletions.
19 changes: 15 additions & 4 deletions 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 @@ -397,12 +397,23 @@ public boolean isEndTagClosed() {
* Returns true if the given element is an orphan end tag (which has no start
* tag, eg: </a>) and false otherwise.
*
* @param tagName the end tag name.
* @return true if the given element is an orphan end tag (which has no start
* tag, eg: </a>) and false otherwise.
*/
public boolean isOrphanEndTag(String tagName) {
return isSameTag(tagName) && hasEndTag() && !hasStartTag();
public boolean isOrphanEndTag() {
return hasEndTag() && !hasStartTag();
}

/**
* Returns true if the given element is an orphan end tag (which has no start
* tag, eg: </a>) of the given tag name and false otherwise.
*
* @param tagName the end tag name.
* @return true if the given element is an orphan end tag (which has no start
* tag, eg: </a>) of the given tag name and false otherwise.
*/
public boolean isOrphanEndTagOf(String tagName) {
return isSameTag(tagName) && isOrphanEndTag();
}

@Override
Expand All @@ -423,7 +434,7 @@ public DOMElement getOrphanEndElement(int offset, String tagName) {
for (DOMNode child : children) {
if (child.isElement()) {
DOMElement childElement = (DOMElement) child;
if (childElement.isOrphanEndTag(tagName)) {
if (childElement.isOrphanEndTagOf(tagName)) {
return childElement;
}
}
Expand Down
2 changes: 1 addition & 1 deletion org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DOMNode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ public DOMElement getOrphanEndElement(int offset, String tagName) {
}
// emp| </employe>
DOMElement nextElement = (DOMElement) next;
if (nextElement.isOrphanEndTag(tagName)) {
if (nextElement.isOrphanEndTagOf(tagName)) {
return nextElement;
}
return null;
Expand Down
4 changes: 4 additions & 0 deletions org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/parser/XMLScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,10 @@ boolean hasNextElementName() {
stream.advanceWhileChar(ELEMENT_NAME_PREDICATE);
return true;
}

public static boolean isStartElementName(Integer ch) {
return START_ELEMENT_NAME_PREDICATE.test(ch);
}

/**
* Returns true if the current token is an attribute name and false otherwise.
Expand Down
3 changes: 2 additions & 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 @@ -141,6 +141,7 @@ public static Range toLSPRange(XMLLocator location, XMLSyntaxErrorCode code, Obj
return XMLPositionUtility.selectAttributeNameFromGivenNameAt(attrName, offset, document);
}
case MarkupEntityMismatch:
return XMLPositionUtility.selectRootStartTag(document);
case ElementUnterminated: {
String text = document.getText();
if (offset < text.length()) {
Expand Down Expand Up @@ -216,7 +217,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
243 changes: 171 additions & 72 deletions ...pse/lemminx/extensions/contentmodel/participants/codeactions/CloseStartTagCodeAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import org.eclipse.lemminx.dom.DOMElement;
import org.eclipse.lemminx.dom.DOMNode;
import org.eclipse.lemminx.dom.LineIndentInfo;
import org.eclipse.lemminx.dom.parser.XMLScanner;
import org.eclipse.lemminx.services.extensions.ICodeActionParticipant;
import org.eclipse.lemminx.services.extensions.IComponentProvider;
import org.eclipse.lemminx.settings.SharedSettings;
import org.eclipse.lemminx.utils.DOMUtils;
import org.eclipse.lemminx.utils.XMLPositionUtility;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.Diagnostic;
Expand All @@ -42,95 +44,192 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen
int startOffset = document.offsetAt(diagnosticRange.getStart());
DOMNode node = document.findNodeAt(startOffset);
if (node != null && node.isElement()) {
String text = document.getText();
int diagnosticEndOffset = document.offsetAt(diagnosticRange.getEnd());
DOMElement element = (DOMElement) node;
if (!element.hasStartTag()) {
// </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);
}
} else {
// <foo>
boolean startTagClosed = element.isStartTagClosed();
char c = document.getText().charAt(diagnosticEndOffset - 1);
if (c != '/') {
if (startTagClosed) {
// ex : <foo attr="" >
// // Close with '</$tag>
String tagName = element.getTagName();
if (tagName != null) {
String label = "</" + tagName + ">";
String insertText = label;
Position endPosition = null;
if (!element.hasChildNodes()) {
int endOffset = element.getStartTagCloseOffset() + 1;
endPosition = document.positionAt(endOffset);
} else {
String text = document.getText();
// the element have some children(Text node, Element node, etc)
int endOffset = element.getLastChild().getEnd() - 1;
if (endOffset < text.length()) {
// remove whitespaces
char ch = text.charAt(endOffset);
while (Character.isWhitespace(ch)) {
endOffset--;
ch = text.charAt(endOffset);
}
boolean startTagClosed = element.isStartTagClosed();
boolean hasSlash = isCharAt(text, diagnosticEndOffset - 1, '/');
if (!hasSlash) {
// Here we are not in the case of <foo attr="" /
if (startTagClosed) {
// ex : <foo attr="" >
// // Close with '</foo>'
String tagName = element.getTagName();
String label = "</" + tagName + ">";
String insertText = label;
Position startPosition = null;
Position endPosition = null;

// Search orphan elements in the children
List<DOMNode> children = element.getChildren();
for (DOMNode child : children) {
if (child.isElement()) {
DOMElement childElement = (DOMElement) child;
if (childElement.getTagName() == null) {
startPosition = document.positionAt(childElement.getStart());
endPosition = document.positionAt(childElement.getEnd());
CodeAction closeEndTagAction = CodeActionFactory.replace("Replace '<' with '" + label + "'",
new Range(startPosition, endPosition), insertText,
document.getTextDocument(), diagnostic);
codeActions.add(closeEndTagAction);
} else if (childElement.isOrphanEndTag()) {
CodeAction replaceTagAction = createReplaceTagNameCodeAction(childElement, element,
diagnostic);
if (replaceTagAction != null) {
codeActions.add(replaceTagAction);
}
}
}
}
if (element.hasEndTag()) {
// ex : <foo> </foo
// --> the '</foo' must be replaced with '</foo>'
startPosition = document.positionAt(element.getEndTagOpenOffset());
endPosition = document.positionAt(element.getEnd());
} else if (!element.hasChildNodes() || DOMUtils.containsTextOnly(element)) {
// Check if we are in this following cases :
// - <foo></
// - <foo> </
// --> if we are in those cases, the '</' must be replaced with '</foo>'.
int startIndex = element.getStartTagCloseOffset() + 1;
if (startIndex < text.length()) {
// Remove all whitespaces
char ch = text.charAt(startIndex);
while (Character.isWhitespace(ch)) {
startIndex++;
ch = text.charAt(startIndex);
}
//
int endIndex = startIndex;
if (isCharAt(text, endIndex, '<')) {
startIndex = endIndex;
endIndex++;
if (isCharAt(text, endIndex, '/')) {
endIndex++;
}
endOffset++;
endPosition = document.positionAt(endOffset);
if (hasElements(element)) {
// The element have element node as children
// the </foo> must be inserted with a new line and indent
LineIndentInfo indentInfo = document
.getLineIndentInfo(diagnosticRange.getStart().getLine());
insertText = indentInfo.getLineDelimiter() + indentInfo.getWhitespacesIndent()
+ insertText;
if (endIndex >= text.length()
|| !XMLScanner.isStartElementName(text.codePointAt(endIndex))) {
startPosition = document.positionAt(startIndex);
endPosition = document.positionAt(endIndex);
}
}
CodeAction closeEndTagAction = CodeActionFactory.insert("Close with '" + label + "'",
endPosition, insertText, document.getTextDocument(), diagnostic);
codeActions.add(closeEndTagAction);
}
}

} else {
// ex : <foo attr="
// Close with '/>
CodeAction autoCloseAction = CodeActionFactory.insert("Close with '/>'",
diagnosticRange.getEnd(), "/>", document.getTextDocument(), diagnostic);
codeActions.add(autoCloseAction);
// // Close with '></$tag>
String tagName = element.getTagName();
if (tagName != null) {
String insertText = "></" + tagName + ">";
CodeAction closeEndTagAction = CodeActionFactory.insert(
"Close with '" + insertText + "'", diagnosticRange.getEnd(), insertText,
document.getTextDocument(), diagnostic);
codeActions.add(closeEndTagAction);
// - <foo><
// - <foo> <
// - <foo></
// - <foo> </
if (endPosition == null) {
if (!element.hasChildNodes()) {
// ex : <foo><bar></foo>
int endOffset = element.getStartTagCloseOffset() + 1;
endPosition = document.positionAt(endOffset);
} else {
// the element have some children(Text node, Element node, etc)
int endOffset = element.getLastChild().getEnd() - 1;
if (endOffset < text.length()) {
// remove whitespaces
char ch = text.charAt(endOffset);
while (Character.isWhitespace(ch)) {
endOffset--;
ch = text.charAt(endOffset);
}
}
endOffset++;
endPosition = document.positionAt(endOffset);
}
if (hasElements(element)) {
// The element have element node as children
// the </foo> must be inserted with a new line and indent
LineIndentInfo indentInfo = document
.getLineIndentInfo(diagnosticRange.getStart().getLine());
insertText = indentInfo.getLineDelimiter() + indentInfo.getWhitespacesIndent()
+ insertText;
}
}
}
CodeAction closeEndTagAction = null;
if (startPosition == null) {
closeEndTagAction = CodeActionFactory.insert("Close with '" + label + "'", endPosition,
insertText, document.getTextDocument(), diagnostic);
} else {
closeEndTagAction = CodeActionFactory.replace("Close with '" + label + "'",
new Range(startPosition, endPosition), insertText, document.getTextDocument(),
diagnostic);

}
codeActions.add(closeEndTagAction);

if (!startTagClosed) {
// Close with '>
CodeAction closeAction = CodeActionFactory.insert("Close with '>'", diagnosticRange.getEnd(),
">", document.getTextDocument(), diagnostic);
codeActions.add(closeAction);
} else {
// ex : <foo attr="
// Close with '/>'
CodeAction autoCloseAction = CodeActionFactory.insert("Close with '/>'",
diagnosticRange.getEnd(), "/>", document.getTextDocument(), diagnostic);
codeActions.add(autoCloseAction);
// // Close with '></foo>'
String tagName = element.getTagName();
if (tagName != null) {
String insertText = "></" + tagName + ">";
CodeAction closeEndTagAction = CodeActionFactory.insert("Close with '" + insertText + "'",
diagnosticRange.getEnd(), insertText, document.getTextDocument(), diagnostic);
codeActions.add(closeEndTagAction);
}
}
}

if (!startTagClosed) {
// Close with '>'
CodeAction closeAction = CodeActionFactory.insert("Close with '>'", diagnosticRange.getEnd(), ">",
document.getTextDocument(), diagnostic);
codeActions.add(closeAction);
}
}
} catch (BadLocationException e) {
// do nothing
}
}

private static boolean isCharAt(String text, int offset, char ch) {
if (text.length() <= offset) {
return false;
}
return text.charAt(offset) == ch;
}

private static boolean isStartElementName(String text, int offset) {
if (text.length() <= offset) {
return false;
}
int ch = text.codePointAt(offset);
return XMLScanner.isStartElementName(ch);
}

/**
* Create a code action which replace the tag name of the given element with the
* tag name of the parent element.
*
* @param element
* @param parent
* @param diagnostic
* @param codeActions
* @return
*/
private static CodeAction createReplaceTagNameCodeAction(DOMElement element, DOMElement parent,
Diagnostic diagnostic) {
// <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();
String replaceText = replaceTagName;
if (!element.isEndTagClosed()) {
replaceText = replaceText + ">";
}
return CodeActionFactory.replace("Replace '" + tagName + "' with '" + replaceTagName + "' closing tag",
replaceRange, replaceText, document.getTextDocument(), diagnostic);
}

/**
* Returns true if the given element has elements as children and false
* otherwise.
Expand All @@ -142,7 +241,7 @@ public void doCodeAction(Diagnostic diagnostic, Range range, DOMDocument documen
*/
private static boolean hasElements(DOMElement element) {
for (DOMNode node : element.getChildren()) {
if (node.isElement()) {
if (node.isElement() && ((DOMElement) node).getTagName() != null) {
return true;
}
}
Expand Down
Loading

0 comments on commit 4de695b

Please sign in to comment.