DTD losing content on format fixed

Fixes eclipse-lemminx#198
NikolasKomonen · Nov 7, 2018 · 379e05f · 379e05f
1 parent 4866e91
commit 379e05f
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 18 deletions.
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/XMLParser.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/XMLParser.java
@@ -252,19 +252,20 @@ public XMLDocument parse(TextDocument document) {
 				break;
 			}
 
-			case EndCommentTag: {
+			case EndDoctypeTag: {
 				curr.end = scanner.getTokenEnd();
 				curr.closed = true;
 				curr = curr.parent;
 				break;
 			}
-			case EndDoctypeTag: {
+
+			case EndCommentTag: {
 				curr.end = scanner.getTokenEnd();
 				curr.closed = true;
 				curr = curr.parent;
 				break;
 			}
-
+			
 			case Content: {
 				// FIXME: don't use getTokenText (substring) to know if the content is only
 				// spaces or line feed (scanner should know that).
@@ -279,6 +280,7 @@ public XMLDocument parse(TextDocument document) {
 				curr.addChild(textNode);
 				break;
 			}
+
 			default:
 			}
 			token = scanner.scan();

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/MultiLineStream.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/MultiLineStream.java
@@ -33,6 +33,10 @@ class MultiLineStream {
 		return ch == _WSP || ch == _TAB || ch == _NWL || ch == _LFD || ch == _CAR;
 	};
 
+	private static final Predicate<Integer> CHARACTER_PREDICATE = ch -> {
+		return ch != _WSP && ch != _TAB && ch != _NWL && ch != _LFD && ch != _CAR;
+	};
+
 	private final String source;
 	private final int len;
 	private int position;
@@ -217,6 +221,14 @@ public boolean skipWhitespace() {
 		return n > 0;
 	}
 
+	/**
+	 * Advances until it reaches a whitespace character
+	 */
+	public boolean advanceUntilWhitespace() {
+		int n = this.advanceWhileChar(CHARACTER_PREDICATE);
+		return n > 0;
+	}
+
 	public int advanceWhileChar(Predicate<Integer> condition) {
 		int posNow = this.position;
 		while (this.position < this.len && condition.test(peekChar())) {

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/Scanner.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/Scanner.java
@@ -20,10 +20,18 @@ public interface Scanner {
 
 	TokenType getTokenType();
 
+	/**
+	 * Starting offset position of the current token
+	 * @return int of token's start offset
+	 */
 	int getTokenOffset();
 
 	int getTokenLength();
 
+	/**
+	 * Ending offset position of the current token
+	 * @return int of token's end offset
+	 */
 	int getTokenEnd();
 
 	String getTokenText();

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/ScannerState.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/ScannerState.java
@@ -17,6 +17,7 @@
  */
 public enum ScannerState {
 	WithinContent, AfterOpeningStartTag, AfterOpeningEndTag, WithinProlog, WithinDoctype, WithinTag, WithinEndTag,
-	WithinComment, AfterAttributeName, BeforeAttributeValue, WithinCDATA, AfterClosingCDATATag, StartCDATATag, AfterPrologOpen, PrologOrPI, WithinPI
+	WithinComment, AfterAttributeName, BeforeAttributeValue, WithinCDATA, AfterClosingCDATATag, StartCDATATag, AfterPrologOpen, PrologOrPI, 
+	WithinPI
 
 }
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/XMLScanner.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/XMLScanner.java
@@ -120,10 +120,17 @@ TokenType internalScan() {
 				state = ScannerState.WithinContent;
 				return finishToken(offset, TokenType.EndDoctypeTag);
 			}
-			stream.advanceUntilCharOrNewTag(_RAN); // >
-			if (stream.peekChar() == _LAN) { // <
-				state = ScannerState.WithinContent;
+			stream.skipWhitespace();
+			if (stream.peekChar() == _RAN) { // >
+				return finishToken(offset, TokenType.Doctype);
+			}
+			stream.advanceUntilWhitespace();
+			stream.skipWhitespace();
+			if(stream.advanceIfChar(_OSB)) { // [
+				stream.advanceUntilChar(_CSB); // ]
+				stream.advance(1);
 			}
+			stream.advanceUntilChar(_RAN); // >
 			return finishToken(offset, TokenType.Doctype);
 		case PrologOrPI:
 			if (stream.advanceIfChars(_QMA, _RAN)) { // ?>
@@ -348,6 +355,11 @@ public TokenType getTokenType() {
 	}
 
 	@Override
+	/**
+	 * Starting offset position of the current token
+	 * 
+	 * @return Starting offset position of the current token
+	 */
 	public int getTokenOffset() {
 		return tokenOffset;
 	}
@@ -358,6 +370,11 @@ public int getTokenLength() {
 	}
 
 	@Override
+	/**
+	 * Ending offset position of the current token
+	 * 
+	 * @return Ending offset position of the current token
+	 */
 	public int getTokenEnd() {
 		return stream.pos();
 	}

diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/services/XMLFormatterTest.java
@@ -376,24 +376,96 @@ public void testElementContentNormalized() throws BadLocationException {
 	@Test
 	public void testDTDFormatting() throws BadLocationException {
 		String content = 
-			"<!DOCTYPE web-app PUBLIC" + lineSeparator() +
-			"\"-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN\"" + lineSeparator() +
-			"\"http://java.sun.com/dtd/web-app_2_3.dtd\" >" + lineSeparator() + 
-		   	lineSeparator() +
-		    "<web-app>" + lineSeparator() +
- 			"  <display-name>Servlet 2.3 Web Application</display-name>" + lineSeparator() +
+			"<!DOCTYPE web-app PUBLIC\r\n" + 
+			"\"-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN\"\r\n" +
+			"\"http://java.sun.com/dtd/web-app_2_3.dtd\" >\r\n" + 
+		    "<web-app>\r\n" + 
+ 			"  <display-name>Servlet 2.3 Web Application</display-name>\r\n" + 
 		    "</web-app>";
 		String expected = 
-			"<!DOCTYPE web-app PUBLIC" + lineSeparator() +
-			"\"-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN\"" + lineSeparator() +
-			"\"http://java.sun.com/dtd/web-app_2_3.dtd\" >" + lineSeparator() + 
-			"<web-app>" + lineSeparator() +
-			"  <display-name>Servlet 2.3 Web Application</display-name>" + lineSeparator() +
+			"<!DOCTYPE web-app PUBLIC\r\n" + 
+			"\"-//Sun Microsystems, Inc.//DTD Web Application 2.3//EN\"\r\n" + 
+			"\"http://java.sun.com/dtd/web-app_2_3.dtd\" >\r\n" + 
+			"<web-app>\r\n" + 
+			"  <display-name>Servlet 2.3 Web Application</display-name>\r\n" + 
 			"</web-app>";
 		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
 		format(content, expected, formattingOptions);
 	}
 
+	@Test
+	public void testDTDFormattingInternal() throws BadLocationException {
+		String content = 
+			"<?xml version=\"1.0\"?>\r\n" +
+			"\r\n" +
+			"\r\n" +
+			"<!DOCTYPE student [\r\n" +
+			"  <!ELEMENT student (surname,id)>\r\n" + 
+			"  <!ELEMENT surname (#PCDATA)>\r\n" + 
+			"  <!ELEMENT id (#PCDATA)>\r\n" + 
+			"]>\r\n" + 
+			"\r\n" + 
+			"<student>\r\n" + 
+			"\r\n" + 
+			"\r\n" + 
+			"  <surname>Smith</surname>\r\n" + 
+			"  <id>567896</id>\r\n" + 
+			"</student>";
+		String expected = 
+			"<?xml version=\"1.0\"?>\r\n" + 
+			"<!DOCTYPE student [\r\n" +
+			"  <!ELEMENT student (surname,id)>\r\n" +
+			"  <!ELEMENT surname (#PCDATA)>\r\n" +
+			"  <!ELEMENT id (#PCDATA)>\r\n" + 
+			"]>\r\n" +
+			"<student>\r\n" +
+			"  <surname>Smith</surname>\r\n" + 
+			"  <id>567896</id>\r\n" +
+			"</student>";
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		format(content, expected, formattingOptions);
+	}
+
+	@Test
+	public void testDTDFormattingExternal() throws BadLocationException {
+		String content = 
+			"<?xml version=\"1.0\"?>\r\n" + 
+			"<!DOCTYPE SYSTEM \"test.xsd>\r\n" + 
+			"<student>\r\n" + 
+			"  <surname>Smith</surname>\r\n" + 
+			"  <id>567896</id>\r\n" + 
+			"</student>";
+		String expected = 
+			"<?xml version=\"1.0\"?>\r\n" + 
+			"<!DOCTYPE SYSTEM \"test.xsd>\r\n" + 
+			"<student>\r\n" + 
+			"  <surname>Smith</surname>\r\n" + 
+			"  <id>567896</id>\r\n" + 
+			"</student>";
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		format(content, expected, formattingOptions);
+	}
+
+	@Test
+	public void testDTDFormattingEmptyContent() throws BadLocationException {
+		String content = 
+			"<?xml version=\"1.0\"?>\r\n" + 
+			"<!DOCTYPE >\r\n" + 
+			"<student>\r\n" + 
+			"  <surname>Smith</surname>\r\n" + 
+			"  <id>567896</id>\r\n" + 
+			"</student>";
+		String expected = 
+			"<?xml version=\"1.0\"?>\r\n" + 
+			"<!DOCTYPE >\r\n" + 
+			"<student>\r\n" + 
+			"  <surname>Smith</surname>\r\n" +
+			"  <id>567896</id>\r\n" + 
+			"</student>";
+		XMLFormattingOptions formattingOptions = createDefaultFormattingOptions();
+		format(content, expected, formattingOptions);
+	}
+
 	@Test
 	public void testContentFormatting1() throws BadLocationException {
 		String content =