RNG attribute completion doesn't generate the proper prefix if the

namespace is not declared Fixes eclipse#1489 Signed-off-by: azerr <[email protected]>
angelozerr · Feb 22, 2023 · fdfdc29 · fdfdc29
1 parent d30d667
commit fdfdc29
Show file tree

Hide file tree

Showing 9 changed files with 15,449 additions and 26 deletions.
diff --git a/...mminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGAttributeDeclaration.java b/...mminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGAttributeDeclaration.java
@@ -14,6 +14,7 @@
 import java.util.Collection;
 import java.util.List;
 
+import org.eclipse.lemminx.dom.DOMElement;
 import org.eclipse.lemminx.extensions.contentmodel.model.CMAttributeDeclaration;
 import org.eclipse.lemminx.extensions.contentmodel.model.CMElementDeclaration;
 import org.eclipse.lemminx.services.extensions.ISharedSettingsRequest;
@@ -38,12 +39,35 @@ public class CMRelaxNGAttributeDeclaration implements CMAttributeDeclaration {
 
  private final CMRelaxNGElementDeclaration cmElement;
  private final AttributePattern pattern;
+
+ private String prefix;
+
+ private boolean computedPrefix;
+
  private boolean required;
  private List<String> values;
 
  public CMRelaxNGAttributeDeclaration(CMRelaxNGElementDeclaration element, AttributePattern pattern) {
  this.cmElement = element;
  this.pattern = pattern;
+ this.computedPrefix = false;
+ }
+
+ @Override
+ public String getPrefix() {
+ String namespaceURI = getNamespace();
+ if (namespaceURI == null) {
+ return null;
+ }
+ if (computedPrefix) {
+ return prefix;
+ }
+ DOMElement element = ((CMRelaxNGElementDeclaration) getOwnerElementDeclaration()).getDOMElement();
+ if (element != null) {
+ prefix = element.getPrefix(namespaceURI);
+ }
+ computedPrefix = true;
+ return prefix;
  }
 
  @Override
@@ -60,7 +84,7 @@ public String getNamespace() {
  public CMElementDeclaration getOwnerElementDeclaration() {
  return cmElement;
  }
- 
+
  Name getJingName() {
  NameClass nameClass = pattern.getNameClass();
  if (nameClass instanceof SimpleNameClass) {

diff --git a/org.eclipse.lemminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGDocument.java b/org.eclipse.lemminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGDocument.java
@@ -193,7 +193,7 @@ private static Position createValidPosition(int line, int character) {
  }
 
  private DOMRange getDeclaredTypeRange(DOMNode originNode, Locator locator) {
- DOMNode node = findNode(locator);
+ DOMNode node = findNodeAt(locator);
  if (node == null) {
  return null;
  }
@@ -207,7 +207,7 @@ private DOMRange getDeclaredTypeRange(DOMNode originNode, Locator locator) {
  return null;
  }
 
- private DOMDocument getDocument(String systemId) {
+ public DOMDocument getDocument(String systemId) {
  DOMDocument document = documents.get(systemId);
  if (document != null) {
  return document;
@@ -236,7 +236,7 @@ public String getDocumentation(Locator locator) {
  }
 
  public String getDocumentation(Locator locator, String value) {
- DOMNode node = findNode(locator);
+ DOMNode node = findNodeAt(locator);
  if (node == null) {
  return null;
  }
@@ -289,7 +289,7 @@ private DOMElement getDocumentationElement(DOMElement element, String value) {
  return null;
  }
 
- DOMNode findNode(Locator locator) {
+ DOMNode findNodeAt(Locator locator) {
  if (locator == null) {
  return null;
  }

diff --git a/...lemminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGElementDeclaration.java b/...lemminx/src/main/java/com/thaiopensource/relaxng/pattern/CMRelaxNGElementDeclaration.java
@@ -60,6 +60,8 @@ public class CMRelaxNGElementDeclaration implements CMElementDeclaration {
 
  private Set<String> possibleRequiredElementNames;
 
+ private DOMElement domElement;
+
  CMRelaxNGElementDeclaration(CMRelaxNGDocument document, ElementPattern pattern) {
  this.cmDocument = document;
  this.pattern = pattern;
@@ -81,9 +83,9 @@ public String getNamespace() {
 
  @Override
  public String getPrefix(String namespaceURI) {
- DOMNode node = cmDocument.findNode(pattern.getLocator());
- if (node != null && node.isElement()) {
- return ((DOMElement) node).getPrefix(namespaceURI);
+ DOMElement element = getDOMElement();
+ if (element != null) {
+ return element.getPrefix(namespaceURI);
  }
  return null;
  }
@@ -96,6 +98,17 @@ private Name getJingName() {
  return null;
  }
 
+ public DOMElement getDOMElement() {
+ if (domElement != null) {
+ return domElement;
+ }
+ DOMNode node = cmDocument.findNodeAt(pattern.getLocator());
+ if (node != null && node.isElement()) {
+ domElement = (DOMElement) node;
+ }
+ return domElement;
+ }
+
  @Override
  public Collection<CMAttributeDeclaration> getAttributes() {
  if (attributes == null) {

diff --git a/...c/main/java/org/eclipse/lemminx/extensions/contentmodel/model/CMAttributeDeclaration.java b/...c/main/java/org/eclipse/lemminx/extensions/contentmodel/model/CMAttributeDeclaration.java
@@ -24,6 +24,8 @@
  */
 public interface CMAttributeDeclaration {
 
+ String getPrefix();
+
  /**
  * Returns the declared attribute local name.
  * 

diff --git a/...lipse/lemminx/extensions/contentmodel/participants/ContentModelCompletionParticipant.java b/...lipse/lemminx/extensions/contentmodel/participants/ContentModelCompletionParticipant.java
@@ -39,6 +39,7 @@
 import org.eclipse.lemminx.services.extensions.completion.ICompletionResponse;
 import org.eclipse.lemminx.uriresolver.CacheResourceDownloadingException;
 import org.eclipse.lemminx.utils.StringUtils;
+import org.eclipse.lemminx.utils.XMLPositionUtility;
 import org.eclipse.lsp4j.CompletionItem;
 import org.eclipse.lsp4j.CompletionItemKind;
 import org.eclipse.lsp4j.MarkupContent;
@@ -302,30 +303,68 @@ public void onAttributeName(boolean generateValue, ICompletionRequest request, I
  private void fillAttributesWithCMAttributeDeclarations(DOMElement parentElement, Range fullRange,
  CMElementDeclaration elementDeclaration, boolean canSupportSnippet, boolean generateValue,
  ICompletionRequest request, ICompletionResponse response) {
-
+ if (parentElement == null) {
+ return;
+ }
  Collection<CMAttributeDeclaration> attributes = elementDeclaration.getAttributes();
  if (attributes == null) {
  return;
  }
  for (CMAttributeDeclaration attributeDeclaration : attributes) {
- if (parentElement != null) {
- String prefix = parentElement.getPrefix(attributeDeclaration.getNamespace());
- String attrName = attributeDeclaration.getName(prefix);
- if (!parentElement.hasAttribute(attrName)) {
- CompletionItem item = new AttributeCompletionItem(attrName, canSupportSnippet, fullRange,
- generateValue,
- attributeDeclaration.getDefaultValue(), attributeDeclaration.getEnumerationValues(),
- request.getSharedSettings());
- if (request.isResolveDocumentationSupported()) {
- addResolveData(request, item, AttributeNameCompletionResolver.PARTICIPANT_ID);
- } else {
- MarkupContent documentation = XMLGenerator.createMarkupContent(attributeDeclaration,
- elementDeclaration,
- request);
- item.setDocumentation(documentation);
+ String prefix = null;
+ boolean declareNamespace = false;
+ String namespaceURI = attributeDeclaration.getNamespace();
+ if (!StringUtils.isEmpty(namespaceURI)) {
+ // The schema defines the attribute with a namespace (ex :
+ // http://www.w3.org/1999/xlink)
+ // Try to get the prefix from the XML document (ex: <docbook
+ // xlink="http://www.w3.org/1999/xlink")
+ prefix = parentElement.getPrefix(namespaceURI);
+ if (prefix == null) {
+ // The namespace is not declared in the XML document, get the prefered prefeix
+ // defined in the grammar.
+ prefix = attributeDeclaration.getPrefix();
+ // The xlink="http://www.w3.org/1999/xlink" needs to be inserted in the root
+ // document element.
+ declareNamespace = true;
+ }
+ }
+ String attrName = attributeDeclaration.getName(prefix);
+ if (!parentElement.hasAttribute(attrName)) {
+ CompletionItem item = new AttributeCompletionItem(attrName, canSupportSnippet, fullRange,
+ generateValue,
+ attributeDeclaration.getDefaultValue(), attributeDeclaration.getEnumerationValues(),
+ request.getSharedSettings());
+ if (declareNamespace) {
+ // Insert with additional text edit, the declaration of the namespace (ex :
+ // xlink="http://www.w3.org/1999/xlink")
+ DOMDocument document = parentElement.getOwnerDocument();
+ int offset = document.getDocumentElement().getOffsetBeforeCloseOfStartTag();
+ Range range = XMLPositionUtility.createRange(offset, offset, document);
+
+ StringBuilder insertText = new StringBuilder();
+ insertText.append(' ');
+ insertText.append("xmlns:");
+ insertText.append(prefix);
+ insertText.append("=\"");
+ insertText.append(namespaceURI);
+ insertText.append("\"");
+ char currentChar = document.getText().charAt(offset);
+ char previousChar = document.getText().charAt(offset - 1);
+ if (currentChar == '>' && previousChar == ' ') {
+ insertText.append(' ');
  }
- response.addCompletionAttribute(item);
+ item.setAdditionalTextEdits(Collections.singletonList(new TextEdit(range, insertText.toString())));
+ }
+ if (request.isResolveDocumentationSupported()) {
+ addResolveData(request, item, AttributeNameCompletionResolver.PARTICIPANT_ID);
+ } else {
+ MarkupContent documentation = XMLGenerator.createMarkupContent(attributeDeclaration,
+ elementDeclaration,
+ request);
+ item.setDocumentation(documentation);
  }
+ response.addCompletionAttribute(item);
  }
  }
  }

diff --git a/.../main/java/org/eclipse/lemminx/extensions/dtd/contentmodel/CMDTDAttributeDeclaration.java b/.../main/java/org/eclipse/lemminx/extensions/dtd/contentmodel/CMDTDAttributeDeclaration.java
@@ -35,14 +35,19 @@ public CMDTDAttributeDeclaration(CMDTDElementDeclaration elementDecl) {
  this.elementDecl = elementDecl;
  }
 
+ @Override
+ public String getPrefix() {
+ return super.name.prefix;
+ }
+
  @Override
  public String getLocalName() {
  return super.name.localpart;
  }
 
  @Override
  public String getNamespace() {
- return null;
+ return super.name.uri;
  }
 
  @Override

diff --git a/.../main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDAttributeDeclaration.java b/.../main/java/org/eclipse/lemminx/extensions/xsd/contentmodel/CMXSDAttributeDeclaration.java
@@ -49,6 +49,11 @@ public CMXSDAttributeDeclaration(CMXSDElementDeclaration cmElement, XSAttributeU
  this.attributeUse = attributeUse;
  }
 
+ @Override
+ public String getPrefix() {
+ return null;
+ }
+
  @Override
  public String getLocalName() {
  return getAttrDeclaration().getName();

diff --git a/...rg/eclipse/lemminx/extensions/relaxng/xml/completion/XMLCompletionBasedOnRelaxNGTest.java b/...rg/eclipse/lemminx/extensions/relaxng/xml/completion/XMLCompletionBasedOnRelaxNGTest.java
@@ -20,6 +20,8 @@
 import static org.eclipse.lemminx.XMLAssert.c;
 import static org.eclipse.lemminx.XMLAssert.te;
 
+import java.util.Arrays;
+
 import org.eclipse.lemminx.XMLAssert;
 import org.eclipse.lemminx.commons.BadLocationException;
 import org.eclipse.lemminx.extensions.contentmodel.BaseFileTempTest;
@@ -155,6 +157,47 @@ public void completionInElementWithXMLNSAndDefinedNS() throws BadLocationExcepti
  c("child", te(2, 0, 2, 1, "<child myvx:type=\"\"></child>"), "<child"));
  }
 
+ @Test
+ public void completionOnAttributes() throws BadLocationException {
+ // completion on <|
+ String xml = "<?xml-model href=\"docbook.rng\"?>\r\n"
+ + "<book xmlns=\"http://docbook.org/ns/docbook\">\r\n"
+ + " <acknowledgements |></acknowledgements> \r\n"
+ + "</book>";
+ testCompletionFor(xml, //
+ 30, //
+ c("role", te(2, 19, 2, 19, "role=\"\""), "role"), //
+ c("xlink:actuate", te(2, 19, 2, 19, "xlink:actuate=\"\""),
+ Arrays.asList(te(1, 43, 1, 43, " xmlns:xlink=\"http://www.w3.org/1999/xlink\"")), //
+ "xlink:actuate"));
+ }
+
+ @Test
+ public void completionOnAttributesWithPrefix() throws BadLocationException {
+ // completion on <|
+ String xml = "<?xml-model href=\"docbook.rng\"?>\r\n"
+ + "<book xmlns=\"http://docbook.org/ns/docbook\">\r\n"
+ + " <acknowledgements |></acknowledgements> \r\n"
+ + "</book>";
+ testCompletionFor(xml, //
+ 30, //
+ c("xlink:actuate", te(2, 19, 2, 19, "xlink:actuate=\"\""),
+ Arrays.asList(te(1, 43, 1, 43, " xmlns:xlink=\"http://www.w3.org/1999/xlink\"")), //
+ "xlink:actuate"));
+
+ xml = "<?xml-model href=\"docbook.rng\"?>\r\n"
+ + "<book xmlns=\"http://docbook.org/ns/docbook\" >\r\n"
+ + " <acknowledgements |></acknowledgements> \r\n"
+ + "</book>";
+ testCompletionFor(xml, //
+ 30, //
+ c("xlink:actuate", te(2, 19, 2, 19, "xlink:actuate=\"\""),
+ Arrays.asList(te(1, 44, 1, 44, " xmlns:xlink=\"http://www.w3.org/1999/xlink\" ")), //
+ "xlink:actuate"));
+ }
+
+ // role,xml:id,version,xml:lang,xml:base,remap,xreflabel,revisionflag,dir,arch,audience,condition,conformance,os,revision,security,userlevel,vendor,wordsize,annotations,linkend,xlink:href,xlink:type,xlink:role,xlink:arcrole,xlink:title,xlink:show,xlink:actuate,label,status
+
  private static void testCompletionFor(String value, Integer expectedCount, CompletionItem... expectedItems)
  throws BadLocationException {
  XMLAssert.testCompletionFor(new XMLLanguageService(), value, null, null, "src/test/resources/relaxng/test.xml",