Fixes unclosed element end tag in parser

Fixes eclipse#269

Signed-off-by: Nikolas Komonen <[email protected]>

org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMParser.java

@@ -66,10 +66,17 @@ public DOMDocument parse(TextDocument document, URIResolverExtensionManager reso
  while (token != TokenType.EOS) {
  switch (token) {
  case StartTagOpen: {
- if(curr.parent != null) {
+ if(!curr.isClosed() && curr.parent != null) {
+ //The next node's parent is not closed at this point
+ //so the node's parent (curr) will have its end position updated
+ //to a newer end position.
  curr.end = scanner.getTokenOffset();
  }
- if(curr.isDoctype() && curr.parent != null) {
+ if((curr.isClosed()) || curr.isDoctype()) {
+ //The next node is being considered is a child of 'curr'
+ //and if 'curr' is already closed then 'curr' was not updated properly.
+ //Or if we get a Doctype node then we know it was not closed and 'curr'
+ //wasn't updated properly.
  curr = curr.parent;
  }
  DOMElement child = xmlDocument.createElement(scanner.getTokenOffset(), scanner.getTokenEnd());
@@ -136,7 +143,7 @@ public DOMDocument parse(TextDocument document, URIResolverExtensionManager reso
  }
  curr.end = scanner.getTokenEnd();
  } else {
- // element open tag not found (ex: <root>) add a fake element which have just
+ // element open tag not found (ex: <root>) add a fake element which only has an
  // end tag (no start tag).
  DOMElement element = xmlDocument.createElement(scanner.getTokenOffset() - 2, scanner.getTokenEnd());
  element.endTagOpenOffset = endTagOpenOffset;
@@ -247,6 +254,14 @@ public DOMDocument parse(TextDocument document, URIResolverExtensionManager reso
  }
 
  case StartCommentTag: {
+ if(xmlDocument.isDTD()) {
+ if(!curr.isDoctype()) {
+ curr = curr.parent;
+ }
+ }
+ else if((curr.isClosed())) {
+ curr = curr.parent;
+ }
  DOMComment comment = xmlDocument.createComment(scanner.getTokenOffset(), text.length());
  if(curr.parent != null && curr.parent.isDoctype()) {
  curr.parent.addChild(comment);

org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/XMLScanner.java

@@ -843,9 +843,6 @@ else if(c == -1) {
  }
 
 
-
-
-
  stream.advance(1);
  state = isInsideDTDContent ? ScannerState.DTDWithinContent : ScannerState.WithinContent;
  return finishToken(offset, TokenType.Unknown, errorMessage);

org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/extensions/contentmodel/participants/XMLSyntaxErrorCode.java

@@ -127,7 +127,7 @@ public static Range toLSPRange(XMLLocator location, XMLSyntaxErrorCode code, Obj
  return XMLPositionUtility.selectAttributeValueByGivenValueAt(attrValue, offset, document);
  }
  case ETagUnterminated:
- return XMLPositionUtility.selectEndTag(offset - 1, document);
+ return XMLPositionUtility.selectPreviousEndTag(offset - 1, document);
  case CustomETag:
  return XMLPositionUtility.selectEndTag(offset, document);
  case ETagRequired: {

org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLPositionUtility.java

@@ -305,20 +305,19 @@ public static Range selectFirstNonWhitespaceText(int offset, DOMDocument documen
  /**
  * Finds the offset of the first tag it comes across behind the given offset.
  * 
- * This excludes the tag it starts in if offset is within a tag.
+ * This includes the tag it starts in if offset is within a tag's content.
+ * 
+ * eg:
+ * <a> <b> | </b> </a> , will give 'b'
+ * <a> <b|> </b> </a> , will give 'a'
  */
  public static Range selectPreviousEndTag(int offset, DOMDocument document) {
  // boolean firstBracket = false;
  int i = offset;
  char c = document.getText().charAt(i);
  while (i >= 0) {
  if (c == '>') {
- // if(firstBracket) {
- return selectStartTag(i, document);
- // }
- // else {
- // firstBracket = true;
- // }
+ return selectEndTag(i, document);
  }
  i--;
  c = document.getText().charAt(i);

org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/dom/DOMParserForInternalDTDTest.java

@@ -800,6 +800,7 @@ public void dtdNotationComplete() {
  }
 
 
+
  private static DOMDocument createDOMDocument(String xml) {
  return DOMParser.getInstance().parse(xml, "uri", null);
  }

org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/dom/DOMParserTest.java

@@ -359,6 +359,34 @@ public void testContentTextHasTag() {
  assertDocument("<html> eek </html>", html);
  }
 
+ @Test
+ public void testUnclosedEndTagWithTrailingElement() {
+ DOMNode root = createElement("root", 0, 29, 36, true);
+ DOMNode elementA = createElement("a", 7, 17, 20, true);
+ DOMNode elementB = createElement("b", 21, 24, 28, true);
+ DOMText content = createTextNode("Content", 10, 17, true);
+
+ root.addChild(elementA);
+ root.addChild(elementB);
+ elementA.addChild(content);
+
+ assertDocument("<root> <a>Content</a <b></b> </root>", root);
+ }
+
+ @Test
+ public void testUnclosedEndTagWithTrailingComment() {
+ DOMNode root = createElement("root", 0, 38, 45, true);
+ DOMNode elementA = createElement("a", 7, 17, 20, true);
+ DOMNode comment = createCommentNode(" comment ", 21, 37, true);
+ DOMText content = createTextNode("Content", 10, 17, true);
+
+ root.addChild(elementA);
+ root.addChild(comment);
+ elementA.addChild(content);
+
+ assertDocument("<root> <a>Content</a <!-- comment --> </root>", root);
+ }
+
  @Test
  public void elementOffsets() {
  DOMDocument document = DOMParser.getInstance().parse("<a></a>", null, null);
@@ -697,6 +725,22 @@ public void testUnrecognizedDTDTagName() {
  DOMDocument document = DOMParser.getInstance().parse(dtd, "name.dtd", null);
  compareTrees(doctype, document.getChild(0));
  }
+
+ @Test
+ public void testExternalDTDCommentBeforeDecl() {
+ String dtd = "<!-- c --> <!ELEMENT png PUBLIC \"JPG 1.0\" \"image/gif\" BAD>";
+
+ DOMNode doctype = createDoctypeNode(0, 58, null, null, null, null, null, null, null, null, null, null);
+ doctype.closed = true;
+ DOMComment comment = createCommentNode(" c ", 0, 10, true);
+ DTDElementDecl element = createElementDecl(11, 58, 21, 24, null, null, null, null, 25, 57);
+ element.closed = true;
+ doctype.addChild(comment);
+ doctype.addChild(element);
+
+ DOMDocument document = DOMParser.getInstance().parse(dtd, "name.dtd", null);
+ compareTrees(doctype, document.getChild(0));
+ }
 
  // --------------------------------------------------------------------------------
  // Tools

org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java

@@ -210,6 +210,21 @@ public void testSplitAttributesProlog() throws BadLocationException {
  format(content, expected, formattingOptions);
  }
 
+ @Test
+ public void testUnclosedEndTagBracketTrailingElement() throws BadLocationException {
+ String content = 
+ "<root>" + lineSeparator() +
+ " <a> content </a" + lineSeparator() + 
+ " <b></b>" + lineSeparator() +
+ "</root>";
+ String expected = 
+ "<root>" + lineSeparator() +
+ " <a> content </a" + lineSeparator() + 
+ " <b></b>" + lineSeparator() +
+ "</root>";
+ format(content, expected);
+ }
+
  @Test
  public void testComment() throws BadLocationException {
  String content = "<!-- CommentText --><a>Val</a>";
@@ -293,6 +308,23 @@ public void testJoinCommentLines() throws BadLocationException {
  format(content, expected, formattingOptions);
  }
 
+ @Test
+ public void testUnclosedEndTagTrailingComment() throws BadLocationException {
+ String content = 
+ "<root>" + lineSeparator() +
+ " <a> content </a" + lineSeparator() + 
+ " <!-- comment -->" + lineSeparator() +
+ " </root>";
+ String expected = 
+ "<root>" + lineSeparator() +
+ " <a> content </a" + lineSeparator() + 
+ " <!-- comment -->" + lineSeparator() +
+ "</root>";
+ XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+ formattingOptions.setJoinCommentLines(true);
+ format(content, expected, formattingOptions);
+ }
+
  @Test
  public void testJoinCommentLinesNested() throws BadLocationException {
  String content =