Range formatting inserts <null> when formatting inside DOCTYPE element

Fixes #682 Signed-off-by: azerr <[email protected]>
eclipse-lemminx · Jun 4, 2020 · 605bd0a · 605bd0a
1 parent 84d2689
commit 605bd0a
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 14 deletions.
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/XMLFormatter.java
@@ -112,13 +112,17 @@ private void setupRangeFormatting(Range range) throws BadLocationException {
 			String fullText = this.textDocument.getText();
 			String rangeText = fullText.substring(this.startOffset, this.endOffset);
 
-			this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, this.textDocument.getUri(), null, false);
+			boolean withinDTDContent = this.fullDomDocument.isWithinInternalDTD(startOffset);
+			String uri = this.textDocument.getUri();
+			if (withinDTDContent) {
+				uri += ".dtd";
+			}
+			this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, uri, null, false);
 
 			if (containsTextWithinStartTag()) {
 				adjustOffsetToStartTag();
 				rangeText = fullText.substring(this.startOffset, this.endOffset);
-				this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, this.textDocument.getUri(), null,
-						false);
+				this.rangeDomDocument = DOMParser.getInstance().parse(rangeText, uri, null, false);
 			}
 
 			this.xmlBuilder = new XMLBuilder(this.sharedSettings, "",
@@ -161,8 +165,8 @@ private void setupFullFormatting(Range range) throws BadLocationException {
 			this.rangeDomDocument = this.fullDomDocument;
 
 			Position startPosition = textDocument.positionAt(startOffset);
-			this.xmlBuilder = new XMLBuilder(this.sharedSettings,
-					"", textDocument.lineDelimiter(startPosition.getLine()));
+			this.xmlBuilder = new XMLBuilder(this.sharedSettings, "",
+					textDocument.lineDelimiter(startPosition.getLine()));
 		}
 
 		private void enlargePositionToGutters(Position start, Position end) throws BadLocationException {
@@ -271,9 +275,9 @@ private void format(DOMNode node) throws BadLocationException {
 			}
 
 			if (node.getNodeType() != DOMNode.DOCUMENT_NODE) {
-				boolean doLineFeed = !node.getOwnerDocument().isDTD() &&
-						!(node.isComment() && ((DOMComment) node).isCommentSameLineEndTag()) &&
-						(!node.isText() || (!((DOMText) node).isWhitespace() && ((DOMText) node).hasSiblings()));
+				boolean doLineFeed = !node.getOwnerDocument().isDTD()
+						&& !(node.isComment() && ((DOMComment) node).isCommentSameLineEndTag())
+						&& (!node.isText() || (!((DOMText) node).isWhitespace() && ((DOMText) node).hasSiblings()));
 
 				if (this.indentLevel > 0 && doLineFeed) {
 					// add new line + indent
@@ -633,7 +637,8 @@ private List<? extends TextEdit> getFormatTextEdit() throws BadLocationException
 					this.xmlBuilder.trimFinalNewlines();
 				}
 
-				if (this.sharedSettings.getFormattingSettings().isInsertFinalNewline() && !this.xmlBuilder.isLastLineEmptyOrWhitespace()) {
+				if (this.sharedSettings.getFormattingSettings().isInsertFinalNewline()
+						&& !this.xmlBuilder.isLastLineEmptyOrWhitespace()) {
 					this.xmlBuilder.linefeed();
 				}
 			}
@@ -692,11 +697,9 @@ public XMLFormatter(XMLExtensionsRegistry extensionsRegistry) {
 	 * @param sharedSettings settings containing formatting preferences
 	 * @return List containing a TextEdit with formatting changes
 	 */
-	public List<? extends TextEdit> format(TextDocument textDocument, Range range,
-			SharedSettings sharedSettings) {
+	public List<? extends TextEdit> format(TextDocument textDocument, Range range, SharedSettings sharedSettings) {
 		try {
-			XMLFormatterDocument formatterDocument = new XMLFormatterDocument(textDocument, range,
-					sharedSettings);
+			XMLFormatterDocument formatterDocument = new XMLFormatterDocument(textDocument, range, sharedSettings);
 			return formatterDocument.format();
 		} catch (BadLocationException e) {
 			LOGGER.log(Level.SEVERE, "Formatting failed due to BadLocation", e);

diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/XMLFormatterTest.java
@@ -283,6 +283,21 @@ public void rangeSelectWithinText() throws BadLocationException {
 		format(content, expected);
 	}
 
+	@Test
+	public void rangeSelectEntity() throws BadLocationException {
+		String content = "<?xml version='1.0' standalone='no'?>\r\n" +  //
+				"<!DOCTYPE root-element [\r\n" + //
+				"    |<!ENTITY local \"LOCALLY DECLARED ENTITY\">|\r\n" +  //
+				"]>";
+
+		String expected = "<?xml version='1.0' standalone='no'?>\r\n" +  //
+				"<!DOCTYPE root-element [\r\n" + //
+				"<!ENTITY local \"LOCALLY DECLARED ENTITY\">\r\n" +  //
+				"]>";
+
+		format(content, expected);
+	}
+
 	@Test
 	public void testProlog() throws BadLocationException {
 		String content = "<?xml version=   \"1.0\"       encoding=\"UTF-8\"  ?>\r\n";
@@ -2471,7 +2486,6 @@ public void dontEnforceDoubleQuoteStyle() throws BadLocationException {
 		format(content, expected, settings);
 	}
 
-
 	@Test
 	public void testTemplate() throws BadLocationException {
 		String content = "";