Preference to convert single quote attribute values to double quote o…

…n format. Fixes eclipse#263, eclipse#294 Signed-off-by: Nikolas <[email protected]>
NikolasKomonen · Feb 21, 2019 · 2201878 · 2201878
1 parent 39f9420
commit 2201878
Show file tree

Hide file tree

Showing 10 changed files with 312 additions and 28 deletions.
diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMAttr.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMAttr.java
@@ -25,7 +25,9 @@ public class DOMAttr extends DOMNode implements org.w3c.dom.Attr {
 
 	private DOMNode nodeAttrValue;
 
-	private String value;
+	private String quotelessValue;//Value without quotes
+
+	private String originalValue;//Exact value from document
 
 	private final DOMNode ownerElement;
 
@@ -110,7 +112,7 @@ public DOMElement getOwnerElement() {
 	 */
 	@Override
 	public String getValue() {
-		return value;
+		return quotelessValue;
 	}
 
 	/*
@@ -157,23 +159,63 @@ public DOMNode getNodeAttrName() {
 		return nodeAttrName;
 	}
 
+	/**
+	 * Get original attribute value from the document.
+	 * 
+	 * This will include quotations (", ').
+	 * @return attribute value with quotations if it had them.
+	 */
+	public String getOriginalValue() {
+		return originalValue;
+	}
+
 	public void setValue(String value, int start, int end) {
-		this.value = getValue(value);
+		this.originalValue = value;
+		this.quotelessValue = getQuotelessValue(value);
 		this.nodeAttrValue = start != -1 ? new AttrNameOrValue(start, end, this) : null;
 	}
 
-	private static String getValue(String value) {
+	/**
+	 * Returns a String of 'value' without surrounding quotes if it had them.
+	 * @param value
+	 * @return
+	 */
+	public static String getQuotelessValue(String value) {
 		if (value == null) {
 			return null;
 		}
 		if (value.isEmpty()) {
 			return value;
 		}
-		int start = value.charAt(0) == '\"' ? 1 : 0;
-		int end = value.charAt(value.length() - 1) == '\"' ? value.length() - 1 : value.length();
+		char quoteValue = value.charAt(0);
+		int start = quoteValue == '\"' || quoteValue == '\'' ? 1 : 0;
+		quoteValue = value.charAt(value.length() - 1);
+		int end = quoteValue == '\"' || quoteValue == '\'' ? value.length() - 1 : value.length();
 		return value.substring(start, end);
 	}
 
+	/**
+	 * Checks if 'value' has matching surrounding quotations.
+	 * @param value
+	 * @return
+	 */
+	public static boolean isQuoted(String value) {
+		if (value == null) {
+			return false;
+		}
+		if (value.isEmpty()) {
+			return false;
+		}
+		char quoteValueStart = value.charAt(0);
+		boolean start = quoteValueStart == '\"' || quoteValueStart == '\'' ? true : false;
+		if(start == false) {
+			return false;
+		}
+		char quoteValueEnd = value.charAt(value.length() - 1);
+		boolean end = (quoteValueEnd == '\"' || quoteValueEnd == '\'') && quoteValueEnd == quoteValueStart ? true : false;
+		return end;
+	}
+
 	public DOMNode getNodeAttrValue() {
 		return nodeAttrValue;
 	}
@@ -201,7 +243,7 @@ public int hashCode() {
 		final int prime = 31;
 		int result = 1;
 		result = prime * result + ((name == null) ? 0 : name.hashCode());
-		result = prime * result + ((value == null) ? 0 : value.hashCode());
+		result = prime * result + ((quotelessValue == null) ? 0 : quotelessValue.hashCode());
 		return result;
 	}
 
@@ -219,10 +261,10 @@ public boolean equals(Object obj) {
 				return false;
 		} else if (!name.equals(other.name))
 			return false;
-		if (value == null) {
-			if (other.value != null)
+		if (quotelessValue == null) {
+			if (other.quotelessValue != null)
 				return false;
-		} else if (!value.equals(other.value))
+		} else if (!quotelessValue.equals(other.quotelessValue))
 			return false;
 		return true;
 	}

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMParser.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMParser.java
@@ -189,6 +189,14 @@ public DOMDocument parse(TextDocument document, URIResolverExtensionManager reso
 				break;
 			}
 
+			case DelimiterAssign: {
+				if(attr != null) {
+					//Sets the value to the '=' position in case there is no AttributeValue
+					attr.setValue(null, scanner.getTokenOffset(), scanner.getTokenEnd());
+				}
+				break;
+			}
+
 			case AttributeValue: {
 				String value = scanner.getTokenText();
 				if (curr.hasAttributes() && attr != null) {

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
@@ -316,7 +316,7 @@ TokenType internalScan() {
 				}
 				state = ScannerState.WithinTag;
 				hasSpaceAfterTag = false;
-				return finishToken(offset, TokenType.AttributeValue);
+				return finishToken(offset, TokenType.AttributeName);
 			}
 			int ch = stream.peekChar();
 			if (ch == _SQO || ch == _DQO || ch == _SIQ) { // " || " || '

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLFormatter.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLFormatter.java
@@ -119,12 +119,12 @@ private void format(DOMNode node, int level, int end, XMLBuilder xml) {
 						List<DOMAttr> attributes = element.getAttributeNodes();
 						if (attributes.size() == 1) {
 							DOMAttr singleAttribute = attributes.get(0);
-							xml.addSingleAttribute(singleAttribute.getName(), singleAttribute.getValue());
+							xml.addSingleAttribute(singleAttribute.getName(), singleAttribute.getOriginalValue());
 						} else {
 							int attributeIndex = 0;
 							for (DOMAttr attr : attributes) {
 								String attributeName = attr.getName();
-								xml.addAttribute(attributeName, attr.getValue(), attributeIndex, level, tag);
+								xml.addAttribute(attributeName, attr.getOriginalValue(), attributeIndex, level, tag);
 								attributeIndex++;
 							}
 						}

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/settings/XMLFormattingOptions.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/settings/XMLFormattingOptions.java
@@ -10,6 +10,8 @@
  */
 package org.eclipse.lsp4xml.settings;
 
+import java.util.Objects;
+
 import org.eclipse.lsp4j.FormattingOptions;
 
 /**
@@ -27,6 +29,14 @@ public class XMLFormattingOptions extends FormattingOptions {
 	private static final String JOIN_CONTENT_LINES = "joinContentLines";
 	private static final String ENABLED = "enabled";
 	private static final String SPACE_BEFORE_EMPTY_CLOSE_TAG = "spaceBeforeEmptyCloseTag";
+	private static final String QUOTATIONS = "quotations";
+
+	// Values for QUOTATIONS
+	public static final String DOUBLE_QUOTES_VALUE = "doubleQuotes";
+	public static final String SINGLE_QUOTES_VALUE = "singleQuotes";
+	enum Quotations {
+		doubleQuotes, singleQuotes
+	}	
 
 	public XMLFormattingOptions() {
 		this(false);
@@ -53,6 +63,7 @@ public void initializeDefaultSettings() {
 		this.setJoinContentLines(false);
 		this.setEnabled(true);
 		this.setSpaceBeforeEmptyCloseTag(true);
+		this.setQuotations(DOUBLE_QUOTES_VALUE);
 	}
 
 	public XMLFormattingOptions(int tabSize, boolean insertSpaces, boolean initializeDefaultSettings) {
@@ -168,6 +179,46 @@ public boolean isSpaceBeforeEmptyCloseTag() {
 		}
 	}
 
+	public void setQuotations(final String quotations) {
+		this.putString(XMLFormattingOptions.QUOTATIONS, quotations);
+	}
+
+	/**
+	 * Returns the value of the format.quotations preference.
+	 * 
+	 * If invalid or null, the default is {@link XMLFormattingOptions#DOUBLE_QUOTES_VALUE}.
+	 */
+	public String getQuotations() {
+		final String value = this.getString(XMLFormattingOptions.QUOTATIONS);
+		if ((value != null) && isValidQuotations()) {
+			return value;
+		} else {
+			this.setQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE);
+			return DOUBLE_QUOTES_VALUE;// default
+		}
+	}
+
+	/**
+	 * If the quotations preference is a valid option.
+	 * 
+	 * Keep up to date with new preferences.
+	 * @return
+	 */
+	private boolean isValidQuotations() {
+		final String value = this.getString(XMLFormattingOptions.QUOTATIONS);
+		return SINGLE_QUOTES_VALUE.equals(value) || DOUBLE_QUOTES_VALUE.equals(value);
+	}
+
+	/**
+	 * Checks if {@code quotation} equals the current value for {@code format.quotations}.
+	 * @param quotation 
+	 * @return
+	 */
+	public boolean isQuotations(String quotation) {
+		String value = getQuotations();
+		return Objects.equals(value, quotation);
+	}
+
 	public XMLFormattingOptions merge(FormattingOptions formattingOptions) {
 		formattingOptions.entrySet().stream().forEach(entry -> {
 			String key = entry.getKey();

diff --git a/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java b/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/utils/XMLBuilder.java
@@ -12,6 +12,7 @@
 
 import static org.eclipse.lsp4xml.utils.StringUtils.normalizeSpace;
 
+import org.eclipse.lsp4xml.dom.DOMAttr;
 import org.eclipse.lsp4xml.dom.DOMComment;
 import org.eclipse.lsp4xml.dom.DOMDocumentType;
 import org.eclipse.lsp4xml.dom.DOMNode;
@@ -32,7 +33,7 @@ public class XMLBuilder {
 
 	public XMLBuilder(XMLFormattingOptions formattingOptions, String whitespacesIndent, String lineDelimiter) {
 		this.whitespacesIndent = whitespacesIndent;
-		this.formattingOptions = formattingOptions;
+		this.formattingOptions = formattingOptions != null ? formattingOptions : new XMLFormattingOptions(true);
 		this.lineDelimiter = lineDelimiter;
 		this.xml = new StringBuilder();
 	}
@@ -92,17 +93,34 @@ public XMLBuilder selfCloseElement() {
 		return this;
 	}
 
+	/**
+	 * Used when only one attribute is being added to a node.
+	 * 
+	 * It will not perform any linefeeds and only basic indentation.
+	 * 
+	 * @param name attribute name
+	 * @param value attribute value
+	 * @return
+	 */
 	public XMLBuilder addSingleAttribute(String name, String value) {
 		xml.append(" ");
-		xml.append(name);
-		xml.append("=\"");
-		if (value != null) {
-			xml.append(value);
-		}
-		xml.append("\"");
+		addAttributeContents(name, value);
+
 		return this;
 	}
 
+	/**
+	 * Used when you are knowingly adding multiple attributes.
+	 * 
+	 * It will do linefeeds and indentation.
+	 * 
+	 * @param name
+	 * @param value
+	 * @param index
+	 * @param level
+	 * @param tagName
+	 * @return
+	 */
 	public XMLBuilder addAttribute(String name, String value, int index, int level, String tagName) {
 		if (isSplitAttributes()) {
 			linefeed();
@@ -111,13 +129,33 @@ public XMLBuilder addAttribute(String name, String value, int index, int level,
 			xml.append(" ");
 		}
 
+		addAttributeContents(name, value);
+		return this;
+	}
+
+	/**
+	 * Builds the attribute name, '=', and value.
+	 * 
+	 * Always puts quotes around values.
+	 */
+	private void addAttributeContents(String name, String value) {
 		xml.append(name);
-		xml.append("=\"");
+		xml.append("=");
+		if(DOMAttr.isQuoted(value)) {
+			if(value.charAt(0) == '\'' && formattingOptions.isQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE) ||
+			   value.charAt(0) == '\"' && formattingOptions.isQuotations(XMLFormattingOptions.SINGLE_QUOTES_VALUE)) {
+				value = DOMAttr.getQuotelessValue(value);
+			} else {
+				xml.append(value);
+				return;
+			}		
+		}
+		String quote = formattingOptions.isQuotations(XMLFormattingOptions.DOUBLE_QUOTES_VALUE) ? "\"" : "'";
+		xml.append(quote);
 		if (value != null) {
 			xml.append(value);
 		}
-		xml.append("\"");
-		return this;
+		xml.append(quote);	
 	}
 
 	public XMLBuilder linefeed() {

diff --git a/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/dom/parser/XMLScannerTest.java b/org.eclipse.lsp4xml/src/test/java/org/eclipse/lsp4xml/dom/parser/XMLScannerTest.java
@@ -545,11 +545,11 @@ public void testName34() {
 		assertOffsetAndToken(4, TokenType.Whitespace);
 		assertOffsetAndToken(5, TokenType.AttributeName);
 		assertOffsetAndToken(8, TokenType.DelimiterAssign);
-		assertOffsetAndToken(9, TokenType.AttributeValue);
+		assertOffsetAndToken(9, TokenType.AttributeName);
 		assertOffsetAndToken(12, TokenType.Whitespace);
 		assertOffsetAndToken(13, TokenType.AttributeName);
 		assertOffsetAndToken(16, TokenType.DelimiterAssign);
-		assertOffsetAndToken(17, TokenType.AttributeValue);
+		assertOffsetAndToken(17, TokenType.AttributeName);
 		assertOffsetAndToken(24, TokenType.StartTagClose);
 	}
 
@@ -562,7 +562,7 @@ public void testName35() {
 		assertOffsetAndToken(4, TokenType.Whitespace);
 		assertOffsetAndToken(5, TokenType.AttributeName);
 		assertOffsetAndToken(8, TokenType.DelimiterAssign);
-		assertOffsetAndToken(9, TokenType.AttributeValue);
+		assertOffsetAndToken(9, TokenType.AttributeName);
 		assertOffsetAndToken(12, TokenType.StartTagSelfClose);
 	}
 

diff --git a/...l/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSyntaxDiagnosticsTest.java b/...l/src/test/java/org/eclipse/lsp4xml/extensions/contentmodel/XMLSyntaxDiagnosticsTest.java
@@ -257,7 +257,7 @@ public void testPseudoAttrNameExpected() throws Exception {
 	@Test
 	public void testQuoteRequiredInXMLDecl() throws Exception {
 		String xml = "<?xml version= encoding=\"UTF-8\"?>";
-		testDiagnosticsFor(xml, d(0, 15, 0, 23, XMLSyntaxErrorCode.QuoteRequiredInXMLDecl));
+		testDiagnosticsFor(xml, d(0, 13, 0, 14, XMLSyntaxErrorCode.QuoteRequiredInXMLDecl));
 	}
 
 	@Test