Support maxLineWidth with invalid attribute values

JessicaJHee · angelozerr · commit e4292ef6b548 · 2022-11-09T22:05:57.000+01:00
Signed-off-by: Jessica He &lt;jhe@redhat.com&gt;
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsi/participants/XSIFormatterParticipant.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/xsi/participants/XSIFormatterParticipant.java
@@ -166,7 +166,7 @@ public boolean formatAttributeValue(DOMAttr attr, XMLFormatterDocumentNew format
 		XSISchemaLocationSplit split = XSISchemaLocationSplit.getSplit(formattingOptions);
 
 		if (split == XSISchemaLocationSplit.none || !XSISchemaModel.isXSISchemaLocationAttr(attr.getName(), attr)) {
-			if (formatterDocument.isMaxLineWidthSupported()) {
+			if (formatterDocument.isMaxLineWidthSupported() && attr.getValue() != null) {
 				parentConstraints
 						.setAvailableLineWidth(parentConstraints.getAvailableLineWidth() - attr.getValue().length());
 			}
@@ -226,7 +226,7 @@ public boolean formatAttributeValue(DOMAttr attr, XMLFormatterDocumentNew format
 				locationNum++;
 			}
 		}
-		if (formatterDocument.isMaxLineWidthSupported()) {
+		if (formatterDocument.isMaxLineWidthSupported() && attr.getValue() != null) {
 			parentConstraints
 					.setAvailableLineWidth(formatterDocument.getMaxLineWidth() - (attrValueStart - indentSpaceOffset)
 							- attr.getValue().length());
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMAttributeFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMAttributeFormatter.java
@@ -95,9 +95,10 @@ public void formatAttribute(DOMAttr attr, int prevOffset, boolean singleAttribut
 			if (isMaxLineWidthSupported() && parentConstraints.getAvailableLineWidth() < 0
 					&& !isSplitAttributes()) {
 				replaceLeftSpacesWithIndentation(indentLevel + 1, from, to, true, edits);
+				int attrValuelength = attr.getValue() != null ? attr.getValue().length() : 0;
 				parentConstraints.setAvailableLineWidth(
 						getMaxLineWidth() - getTabSize() * (indentLevel + 1) - attributeNamelength
-								- attr.getValue().length());
+								- attrValuelength);
 			} else {
 				// remove extra whitespaces between previous attribute
 				// attr0='name'[space][space][space]attr1='name' -->
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMElementFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMElementFormatter.java
@@ -103,7 +103,7 @@ private int formatStartTagElement(DOMElement element, XMLFormattingConstraints p
 			if ((parentStartCloseOffset != startTagOpenOffset
 					&& StringUtils.isWhitespace(formatterDocument.getText(), parentStartCloseOffset,
 							startTagOpenOffset))
-					|| (parentConstraints.getAvailableLineWidth() - width) < 0) {
+					|| ((parentConstraints.getAvailableLineWidth() - width) < 0 && isMaxLineWidthSupported())) {
 				replaceLeftSpacesWithIndentationPreservedNewLines(parentStartCloseOffset, startTagOpenOffset,
 						indentLevel, edits);
 				parentConstraints.setAvailableLineWidth(getMaxLineWidth());
@@ -307,7 +307,7 @@ private int formatEndTagElement(DOMElement element, XMLFormattingConstraints par
 			// before formatting: <a> example text <b> </b> [space][space]</a>
 			// after formatting: <a> example text <b> </b>\n</a>
 			DOMNode lastChild = element.getLastChild();
-			if (lastChild != null 
+			if (lastChild != null
 					&& (lastChild.isElement() || lastChild.isComment())
 					&& Character.isWhitespace(formatterDocument.getText().charAt(endTagOpenOffset - 1))
 					&& !isPreserveEmptyContent()) {
@@ -480,4 +480,8 @@ private int getMaxLineWidth() {
 	private int getTabSize() {
 		return formatterDocument.getSharedSettings().getFormattingSettings().getTabSize();
 	}
+
+	private boolean isMaxLineWidthSupported() {
+		return formatterDocument.isMaxLineWidthSupported();
+	}
 }
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/services/format/DOMTextFormatter.java
@@ -40,7 +40,6 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 		String text = formatterDocument.getText();
 		int availableLineWidth = parentConstraints.getAvailableLineWidth();
 		int indentLevel = parentConstraints.getIndentLevel();
-		int maxLineWidth = getMaxLineWidth();
 
 		int spaceStart = -1;
 		int spaceEnd = -1;
@@ -71,6 +70,7 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 				}
 				int contentEnd = i + 1;
 				if (isMaxLineWidthSupported()) {
+					int maxLineWidth = getMaxLineWidth();
 					availableLineWidth -= contentEnd - contentStart;
 					if (textNode.getStart() != contentStart && availableLineWidth >= 0
 							&& (isJoinContentLines() || !containsNewLine)) {
@@ -112,7 +112,7 @@ public void formatText(DOMText textNode, XMLFormattingConstraints parentConstrai
 		if (formatElementCategory != FormatElementCategory.IgnoreSpace && spaceEnd + 1 != text.length()) {
 			DOMElement parentElement = textNode.getParentElement();
 			// Don't format final spaces if text is at the end of the file
-			if ((!containsNewLine || isJoinContentLines()) && availableLineWidth > 0) {
+			if ((!containsNewLine || isJoinContentLines()) && (!isMaxLineWidthSupported() || availableLineWidth >= 0)) {
 				// Replace spaces with single space in the case of:
 				// 1. there is no new line
 				// 2. isJoinContentLines
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/settings/XMLFormattingOptions.java
@@ -157,7 +157,7 @@ private void initializeDefaultSettings() {
 		this.setJoinContentLines(false);
 		this.setEnabled(true);
 		this.setExperimental(false);
-		this.setMaxLineWidth(80);
+		this.setMaxLineWidth(0);
 		this.setSpaceBeforeEmptyCloseTag(true);
 		this.setPreserveEmptyContent(false);
 		this.setPreservedNewlines(DEFAULT_PRESERVER_NEW_LINES);
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xsi/XSIFormatterExperimentalTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/xsi/XSIFormatterExperimentalTest.java
@@ -79,6 +79,7 @@ public void xsiSchemaLocationSplitNoneWithDefaultMaxLineWidth() throws BadLocati
 		// None
 		settings = createSettings();
 		XSISchemaLocationSplit.setSplit(XSISchemaLocationSplit.none, settings.getFormattingSettings());
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings,
 				te(1, 58, 1, 59, "\r\n  "), //
 				te(1, 112, 1, 113, "\r\n  "), //
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterExperimentalTest.java
@@ -724,6 +724,7 @@ public void testCommentLongWrapJoinContentLines() throws BadLocationException {
 				"  comment comment comment comment comment comment comment comment --></a>";
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setJoinContentLines(true);
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(0, 3, 1, 2, " "), //
 				te(1, 78, 1, 79, "\n  "), //
@@ -743,6 +744,7 @@ public void testCommentLongTextContentWrap() throws BadLocationException {
 				"  comment comment comment comment comment comment comment -->\n" + //
 				"</a>";
 		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(0, 75, 0, 76, "\n  "), //
 				te(0, 152, 0, 153, "\n  "),
@@ -765,6 +767,7 @@ public void testCommentLongTextContentWrapNewLine() throws BadLocationException
 				"  comment comment comment comment comment comment comment -->\n" + //
 				"</a>";
 		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(1, 73, 1, 74, "\n  "), //
 				te(1, 150, 1, 151, "\n  "),
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterJoinCommentLinesTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterJoinCommentLinesTest.java
@@ -134,6 +134,7 @@ public void testJoinCommentLinesLongWrap() throws BadLocationException {
 				"  comment comment comment comment comment comment comment comment --></a>";
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setJoinCommentLines(true);
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(1, 78, 1, 79, "\n  "), //
 				te(1, 150, 1, 151, "\n  "));
@@ -154,6 +155,7 @@ public void testJoinCommentLinesLongWrapSingleWord() throws BadLocationException
 				"</a>";
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setJoinCommentLines(true);
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(0, 3, 1, 0, "\n  "), //
 				te(1, 62, 1, 63, "\n  "), //
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterMaxLineWithTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterMaxLineWithTest.java
@@ -194,7 +194,6 @@ public void splitWithAttributeNested() throws BadLocationException {
 				"        c=\"test\"> </b>" + System.lineSeparator() + //
 				"</a>";
 		assertFormat(content, expected, settings, //
-
 				te(0, 2, 0, 3, System.lineSeparator() + "    "), //
 				te(0, 13, 0, 14, System.lineSeparator() + "    "), //
 				te(0, 16, 0, 17, System.lineSeparator() + "        "), //
@@ -218,6 +217,80 @@ public void splitWithAttributeKeepSameLine() throws BadLocationException {
 		assertFormat(expected, expected, settings);
 	}
 
+	@Test
+	public void splitWithAttributeInvalid() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setTabSize(4);
+		settings.getFormattingSettings().setMaxLineWidth(10);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a bb=>    </a>";
+		String expected = "<a bb=> </a>";
+		assertFormat(content, expected, settings, //
+				te(0, 7, 0, 11, " "));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void splitWithAttributeInvalidSingleQuote() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setTabSize(4);
+		settings.getFormattingSettings().setMaxLineWidth(10);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a bb=\">    </a>";
+		String expected = "<a" + System.lineSeparator() + //
+				"    bb=\">    </a>";
+		assertFormat(content, expected, settings, //
+				te(0, 2, 0, 3, System.lineSeparator() + "    "));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void splitWithAttributeInvalidSpace() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setTabSize(4);
+		settings.getFormattingSettings().setMaxLineWidth(10);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a bb = >    </a>";
+		String expected = "<a bb=> </a>";
+		assertFormat(content, expected, settings, //
+				te(0, 5, 0, 6, ""), //
+				te(0, 7, 0, 8, ""), //
+				te(0, 9, 0, 13, " "));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void splitWithAttributeInvalidSpaceSingleQuote() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setTabSize(4);
+		settings.getFormattingSettings().setMaxLineWidth(10);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a bb = \" >    </a>";
+		String expected = "<a" + System.lineSeparator() + //
+				"    bb=\" >    </a>";
+		assertFormat(content, expected, settings, //
+				te(0, 2, 0, 3, System.lineSeparator() + "    "), //
+				te(0, 5, 0, 6, ""), //
+				te(0, 7, 0, 8, ""));
+		assertFormat(expected, expected, settings);
+	}
+
+	@Test
+	public void splitWithAttributeInvalidSpaceQuoted() throws BadLocationException {
+		SharedSettings settings = new SharedSettings();
+		settings.getFormattingSettings().setTabSize(4);
+		settings.getFormattingSettings().setMaxLineWidth(10);
+		settings.getFormattingSettings().setJoinContentLines(true);
+		String content = "<a bb = \" \" >    </a>";
+		String expected = "<a bb=\" \"> </a>";
+		assertFormat(content, expected, settings, //
+				te(0, 5, 0, 6, ""), //
+				te(0, 7, 0, 8, ""), //
+				te(0, 11, 0, 12, ""), //
+				te(0, 13, 0, 17, " "));
+		assertFormat(expected, expected, settings);
+	}
+
 	@Test
 	public void longText() throws BadLocationException {
 		SharedSettings settings = new SharedSettings();
diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveSpacesTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/services/format/experimental/XMLFormatterPreserveSpacesTest.java
@@ -132,6 +132,7 @@ public void preserveSpacesWithXsdStringWithMaxLineWidth() throws Exception {
 
 		SharedSettings settings = new SharedSettings();
 		settings.getFormattingSettings().setGrammarAwareFormatting(true);
+		settings.getFormattingSettings().setMaxLineWidth(80);
 		assertFormat(content, expected, settings, //
 				te(1, 50, 1, 51, "\r\n  "), //
 				te(1, 104, 1, 105, "\r\n  "), //

Original file line number	Diff line number	Diff line change
`@@ -166,7 +166,7 @@ public boolean formatAttributeValue(DOMAttr attr, XMLFormatterDocumentNew format`
`166`	`166`	`XSISchemaLocationSplit split = XSISchemaLocationSplit.getSplit(formattingOptions);`
`167`	`167`
`168`	`168`	`if (split == XSISchemaLocationSplit.none \|\| !XSISchemaModel.isXSISchemaLocationAttr(attr.getName(), attr)) {`
`169`		`- if (formatterDocument.isMaxLineWidthSupported()) {`
	`169`	`+ if (formatterDocument.isMaxLineWidthSupported() && attr.getValue() != null) {`
`170`	`170`	`parentConstraints`
`171`	`171`	`.setAvailableLineWidth(parentConstraints.getAvailableLineWidth() - attr.getValue().length());`
`172`	`172`	`}`
`@@ -226,7 +226,7 @@ public boolean formatAttributeValue(DOMAttr attr, XMLFormatterDocumentNew format`
`226`	`226`	`locationNum++;`
`227`	`227`	`}`
`228`	`228`	`}`
`229`		`- if (formatterDocument.isMaxLineWidthSupported()) {`
	`229`	`+ if (formatterDocument.isMaxLineWidthSupported() && attr.getValue() != null) {`
`230`	`230`	`parentConstraints`
`231`	`231`	`.setAvailableLineWidth(formatterDocument.getMaxLineWidth() - (attrValueStart - indentSpaceOffset)`
`232`	`232`	`- attr.getValue().length());`