Entity documentation has no value for entities declared with SYSTEM OR

PUBLIC Fixes eclipse#741 Signed-off-by: azerr <[email protected]>
angelozerr · May 29, 2020 · 41b1c37 · 41b1c37
1 parent f2a90dd
commit 41b1c37
Show file tree

Hide file tree

Showing 16 changed files with 209 additions and 55 deletions.
diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DTDEntityDecl.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DTDEntityDecl.java
@@ -114,7 +114,7 @@ public String getNotationName() {
  */
  @Override
  public String getPublicId() {
- return publicId != null ? publicId.getParameter() : null;
+ return publicId != null ? publicId.getParameterWithoutFirstAndLastChar() : null;
  }
 
  public void setPublicId(int start, int end) {
@@ -128,7 +128,7 @@ public void setPublicId(int start, int end) {
  */
  @Override
  public String getSystemId() {
- return systemId != null ? systemId.getParameter() : null;
+ return systemId != null ? systemId.getParameterWithoutFirstAndLastChar() : null;
  }
 
  public void setSystemId(int start, int end) {

diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DTDNotationDecl.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/dom/DTDNotationDecl.java
@@ -54,15 +54,15 @@ void setPublicId(int start, int end) {
  }
 
  public String getPublicId() {
- return publicId != null ? publicId.getParameter() : null;
+ return publicId != null ? publicId.getParameterWithoutFirstAndLastChar() : null;
  }
 
  void setSystemId(int start, int end) {
  systemId = addNewParameter(start, end);
  }
 
  public String getSystemId() {
- return systemId != null ? systemId.getParameter() : null;
+ return systemId != null ? systemId.getParameterWithoutFirstAndLastChar() : null;
  }
 
  @Override

diff --git a/....lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/contentmodel/CMDTDDocument.java b/....lemminx/src/main/java/org/eclipse/lemminx/extensions/dtd/contentmodel/CMDTDDocument.java
@@ -28,6 +28,7 @@
 import org.apache.xerces.impl.dtd.DTDGrammar;
 import org.apache.xerces.impl.dtd.XMLDTDLoader;
 import org.apache.xerces.xni.Augmentations;
+import org.apache.xerces.xni.XMLResourceIdentifier;
 import org.apache.xerces.xni.XMLString;
 import org.apache.xerces.xni.XNIException;
 import org.apache.xerces.xni.grammars.Grammar;
@@ -101,12 +102,28 @@ private static class ScannedDTDEntityDecl extends DTDEntityDecl {
 
  private final String entityName;
  private final String value;
+
  private final DTDDeclParameter nameParameter;
 
+ private final String publicId;
+
+ private final String systemId;
+
  public ScannedDTDEntityDecl(String entityName, String value, ScannedEntity scannedEntity) {
+ this(entityName, value, null, scannedEntity);
+ }
+
+ public ScannedDTDEntityDecl(String name, XMLResourceIdentifier identifier, ScannedEntity scannedEntity) {
+ this(name, null, identifier, scannedEntity);
+ }
+
+ private ScannedDTDEntityDecl(String entityName, String value, XMLResourceIdentifier identifier,
+ ScannedEntity scannedEntity) {
  super(-1, -1);
  this.entityName = entityName;
  this.value = value;
+ this.publicId = identifier != null ? identifier.getPublicId() : null;
+ this.systemId = identifier != null ? identifier.getLiteralSystemId() : null;
  this.nameParameter = createNameParameter(entityName, scannedEntity);
  }
 
@@ -130,6 +147,16 @@ public String getNotationName() {
  return value;
  }
 
+ @Override
+ public String getSystemId() {
+ return systemId;
+ }
+
+ @Override
+ public String getPublicId() {
+ return publicId;
+ }
+
  private static DTDDeclParameter createNameParameter(String name, ScannedEntity scannedEntity) {
  String systemId = scannedEntity.entityLocation.getExpandedSystemId();
  int lineNumber = scannedEntity.lineNumber - 1;
@@ -170,7 +197,8 @@ private static int getEntityNameStartColumnNumber(String entityName, ScannedEnti
  char[] ch = scannedEntity.ch;
  int wordIndex = entityName.length(); //
  int startEntityNameIndex = -1;
- // Loop for characters from the end of the entity (>) to search the entity name start offset
+ // Loop for characters from the end of the entity (>) to search the entity name
+ // start offset
  // <!ENTITY name .....> |
  for (int i = endEntityIndex; i >= 0; i--) {
  char c = ch[i];
@@ -292,6 +320,17 @@ public void internalEntityDecl(String name, XMLString text, XMLString nonNormali
  }
  }
 
+ @Override
+ public void externalEntityDecl(String name, XMLResourceIdentifier identifier, Augmentations augs)
+ throws XNIException {
+ super.externalEntityDecl(name, identifier, augs);
+ try {
+ entities.add(new ScannedDTDEntityDecl(name, identifier, fEntityManager.getCurrentEntity()));
+ } catch (Exception e) {
+ LOGGER.log(Level.SEVERE, "Error while extracting information for the external entity '" + name + "'", e);
+ }
+ }
+
  @Override
  public void startContentModel(String elementName, Augmentations augs) throws XNIException {
  if (hierarchiesMap == null) {

diff --git a/...inx/src/main/java/org/eclipse/lemminx/extensions/entities/EntitiesDocumentationUtils.java b/...inx/src/main/java/org/eclipse/lemminx/extensions/entities/EntitiesDocumentationUtils.java
@@ -12,6 +12,7 @@
  */
 package org.eclipse.lemminx.extensions.entities;
 
+import org.eclipse.lemminx.dom.DTDEntityDecl;
 import org.eclipse.lsp4j.MarkupContent;
 import org.eclipse.lsp4j.MarkupKind;
 
@@ -64,6 +65,19 @@ public String getValue() {
  private EntitiesDocumentationUtils() {
  }
 
+ public static MarkupContent getDocumentation(String entityName, String entityValue, EntityOriginType type,
+ boolean markdown) {
+ return getDocumentation(entityName, entityValue, null, null, null, type, markdown);
+ }
+
+ public static MarkupContent getDocumentation(DTDEntityDecl entity, EntityOriginType type, boolean markdown) {
+ String systemID = entity.getSystemId();
+ String publicID = entity.getPublicId();
+ String targetURI = entity.getNameParameter().getTargetURI();
+ return getDocumentation(entity.getName(), entity.getNotationName(), systemID, publicID, targetURI, type,
+ markdown);
+ }
+
  /**
  * Returns the entity documentation.
  * 
@@ -74,8 +88,8 @@ private EntitiesDocumentationUtils() {
  * false otherwise.
  * @return the entity documentation.
  */
- public static MarkupContent getDocumentation(String entityName, String entityValue, EntityOriginType type,
- boolean markdown) {
+ public static MarkupContent getDocumentation(String entityName, String entityValue, String systemID,
+ String publicID, String targetURI, EntityOriginType type, boolean markdown) {
  StringBuilder documentation = new StringBuilder();
 
  // Title
@@ -88,15 +102,32 @@ public static MarkupContent getDocumentation(String entityName, String entityVal
  documentation.append("**");
  }
 
- if (entityValue != null && !entityValue.isEmpty()) {
- addParameter("Value", entityValue, documentation, markdown);
- }
+ addParameter("Value", entityValue, documentation, markdown);
  addParameter("Type", type.getLabel(), documentation, markdown);
+ addParameter("Public ID", publicID, documentation, markdown);
+ addParameter("System ID", systemID, documentation, markdown);
+ if (targetURI != null) {
+ documentation.append(System.lineSeparator());
+ if (markdown) {
+ documentation.append(" * ");
+ }
+ documentation.append("Source: ");
+ if (markdown) {
+ documentation.append("[");
+ documentation.append(getFileName(targetURI));
+ documentation.append("]");
+ documentation.append("(");
+ }
+ documentation.append(targetURI);
+ if (markdown) {
+ documentation.append(")");
+ }
+ }
  return new MarkupContent(markdown ? MarkupKind.MARKDOWN : MarkupKind.PLAINTEXT, documentation.toString());
  }
 
  private static void addParameter(String name, String value, StringBuilder documentation, boolean markdown) {
- if (value != null) {
+ if (value != null && !value.isEmpty()) {
  documentation.append(System.lineSeparator());
  if (markdown) {
  documentation.append(" * ");
@@ -113,4 +144,20 @@ private static void addParameter(String name, String value, StringBuilder docume
  }
  }
 
+ /**
+ * Returns the file name from the given schema URI
+ * 
+ * @param schemaURI the schema URI
+ * @return the file name from the given schema URI
+ */
+ private static String getFileName(String schemaURI) {
+ int index = schemaURI.lastIndexOf('/');
+ if (index == -1) {
+ index = schemaURI.lastIndexOf('\\');
+ }
+ if (index == -1) {
+ return schemaURI;
+ }
+ return schemaURI.substring(index + 1, schemaURI.length());
+ }
 }
diff --git a/...a/org/eclipse/lemminx/extensions/entities/participants/EntitiesCompletionParticipant.java b/...a/org/eclipse/lemminx/extensions/entities/participants/EntitiesCompletionParticipant.java
@@ -17,6 +17,7 @@
 
 import org.eclipse.lemminx.dom.DOMDocument;
 import org.eclipse.lemminx.dom.DOMDocumentType;
+import org.eclipse.lemminx.dom.DTDEntityDecl;
 import org.eclipse.lemminx.extensions.contentmodel.model.CMDocument;
 import org.eclipse.lemminx.extensions.contentmodel.model.ContentModelManager;
 import org.eclipse.lemminx.extensions.entities.EntitiesDocumentationUtils;
@@ -30,6 +31,7 @@
 import org.eclipse.lsp4j.CompletionItem;
 import org.eclipse.lsp4j.CompletionItemKind;
 import org.eclipse.lsp4j.InsertTextFormat;
+import org.eclipse.lsp4j.MarkupContent;
 import org.eclipse.lsp4j.MarkupKind;
 import org.eclipse.lsp4j.Range;
 import org.eclipse.lsp4j.TextEdit;
@@ -78,8 +80,9 @@ private static void collectLocalEntityProposals(DOMDocument document, Range enti
  Entity entity = (Entity) entities.item(i);
  if (entity.getNodeName() != null) {
  // provide completion for the locally declared entity
- fillCompletion(entity.getNodeName(), entity.getNotationName(), EntityOriginType.LOCAL, entityRange,
- markdown, response);
+ MarkupContent documentation = EntitiesDocumentationUtils.getDocumentation((DTDEntityDecl) entity,
+ EntityOriginType.LOCAL, markdown);
+ fillCompletion(entity.getNodeName(), documentation, entityRange, response);
  }
  }
  }
@@ -103,8 +106,9 @@ private static void collectExternalEntityProposals(DOMDocument document, Range e
  for (Entity entity : entities) {
  if (entity.getNodeName() != null) {
  // provide completion for the external declared entity
- fillCompletion(entity.getNodeName(), entity.getNotationName(), EntityOriginType.EXTERNAL,
- entityRange, markdown, response);
+ MarkupContent documentation = EntitiesDocumentationUtils.getDocumentation((DTDEntityDecl) entity,
+ EntityOriginType.EXTERNAL, markdown);
+ fillCompletion(entity.getNodeName(), documentation, entityRange, response);
  }
  }
  }
@@ -123,13 +127,14 @@ private static void collectExternalEntityProposals(DOMDocument document, Range e
  private void collectPredefinedEntityProposals(Range entityRange, boolean markdown, ICompletionResponse response) {
  PredefinedEntity[] entities = PredefinedEntity.values();
  for (PredefinedEntity entity : entities) {
- fillCompletion(entity.getName(), entity.getValue(), EntityOriginType.PREDEFINED, entityRange, markdown,
- response);
+ MarkupContent documentation = EntitiesDocumentationUtils.getDocumentation(entity.getName(),
+ entity.getValue(), EntityOriginType.PREDEFINED, markdown);
+ fillCompletion(entity.getName(), documentation, entityRange, response);
  }
  }
 
- private static void fillCompletion(String name, String entityValue, EntityOriginType type, Range entityRange,
- boolean markdown, ICompletionResponse response) {
+ private static void fillCompletion(String name, MarkupContent documentation, Range entityRange,
+ ICompletionResponse response) {
  String entityName = "&" + name + ";";
  CompletionItem item = new CompletionItem();
  item.setLabel(entityName);
@@ -138,7 +143,7 @@ private static void fillCompletion(String name, String entityValue, EntityOrigin
  String insertText = entityName;
  item.setFilterText(insertText);
  item.setTextEdit(new TextEdit(entityRange, insertText));
- item.setDocumentation(EntitiesDocumentationUtils.getDocumentation(name, entityValue, type, markdown));
+ item.setDocumentation(documentation);
  response.addCompletionItem(item);
  }
 

diff --git a/...n/java/org/eclipse/lemminx/extensions/entities/participants/EntitiesHoverParticipant.java b/...n/java/org/eclipse/lemminx/extensions/entities/participants/EntitiesHoverParticipant.java
@@ -136,8 +136,7 @@ private static MarkupContent searchInLocalEntities(String entityName, Range enti
  DTDEntityDecl entity = (DTDEntityDecl) entities.item(i);
  if (entityName.equals(entity.getName())) {
  boolean markdown = request.canSupportMarkupKind(MarkupKind.MARKDOWN);
- return EntitiesDocumentationUtils.getDocumentation(entity.getName(), entity.getNotationName(),
- EntityOriginType.LOCAL, markdown);
+ return EntitiesDocumentationUtils.getDocumentation(entity, EntityOriginType.LOCAL, markdown);
  }
  }
  return null;
@@ -164,8 +163,7 @@ private static MarkupContent searchInExternalEntities(String entityName, Range e
  DTDEntityDecl entity = (DTDEntityDecl) ent;
  if (entityName.equals(entity.getName())) {
  boolean markdown = request.canSupportMarkupKind(MarkupKind.MARKDOWN);
- return EntitiesDocumentationUtils.getDocumentation(entity.getName(), entity.getNotationName(),
- EntityOriginType.EXTERNAL, markdown);
+ return EntitiesDocumentationUtils.getDocumentation(entity, EntityOriginType.EXTERNAL, markdown);
  }
  }
  }

diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/dom/DOMParserForInternalDTDTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/dom/DOMParserForInternalDTDTest.java
@@ -514,8 +514,8 @@ public void entityDeclWithNameClosedAndParameters() {
  assertEquals("%", entityDecl.getPercent());
  assertEquals("eName", entityDecl.getNodeName());
  assertEquals("PUBLIC", entityDecl.getKind());
- assertEquals("\"publicId\"", entityDecl.getPublicId());
- assertEquals("\"systemId\"", entityDecl.getSystemId());
+ assertEquals("publicId", entityDecl.getPublicId());
+ assertEquals("systemId", entityDecl.getSystemId());
 
  // <foo />element
  assertTrue(actual.getChild(1).isElement());
@@ -544,7 +544,7 @@ public void entityDeclWithNameClosedAndSomeParameters() {
  assertEquals("%", entityDecl.getPercent());
  assertEquals("eName", entityDecl.getNodeName());
  assertEquals("PUBLIC", entityDecl.getKind());
- assertEquals("\"publicId\"", entityDecl.getPublicId());
+ assertEquals("publicId", entityDecl.getPublicId());
  assertEquals(null, entityDecl.getSystemId());
 
  // <foo />element
@@ -772,8 +772,8 @@ public void dtdNotationComplete() {
  assertEquals(65, elementDecl.getEnd());
  assertEquals("Name", elementDecl.getName());
  assertEquals("PUBLIC", elementDecl.getKind());
- assertEquals("\"PublicID\"", elementDecl.getPublicId());
- assertEquals("\"SystemID\"", elementDecl.getSystemId());
+ assertEquals("PublicID", elementDecl.getPublicId());
+ assertEquals("SystemID", elementDecl.getSystemId());
 
  assertTrue(elementDecl.isClosed());
  }
@@ -805,7 +805,7 @@ public void dtdNotationSYSTEMUnrecognizedParameter() {
  assertEquals(65, elementDecl.getEnd());
  assertEquals("Name", elementDecl.getName());
  assertEquals("SYSTEM", elementDecl.getKind());
- assertEquals("\"PublicID\"", elementDecl.getSystemId());
+ assertEquals("PublicID", elementDecl.getSystemId());
  assertEquals("\"SystemID\"", elementDecl.getUnrecognized());
 
  assertTrue(elementDecl.isClosed());

diff --git a/...emminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java b/...emminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDocumentLinkTest.java
@@ -27,29 +27,29 @@ public class DTDDocumentLinkTest {
  @Test
  public void docTypeSYSTEM() throws BadLocationException {
  String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\r\n" + //
- "<!DOCTYPE root-element SYSTEM \"../dtd/base.dtd\" [\r\n" + //
+ "<!DOCTYPE root-element SYSTEM \"../dtd/entities/base.dtd\" [\r\n" + //
  "\r\n" + //
  "]>\r\n" + //
  "<root-element />";
  XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml",
- dl(r(1, 31, 1, 46), "src/test/resources/dtd/base.dtd"));
+ dl(r(1, 31, 1, 55), "src/test/resources/dtd/entities/base.dtd"));
  }
 
  @Test
  public void docTypePUBLIC() throws BadLocationException {
  String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\r\n" + //
- "<!DOCTYPE root-element PUBLIC \"ABCD\" \"../dtd/base.dtd\" [\r\n" + //
+ "<!DOCTYPE root-element PUBLIC \"ABCD\" \"../dtd/entities/base.dtd\" [\r\n" + //
  "\r\n" + //
  "]>\r\n" + //
  "<root-element />";
  XMLAssert.testDocumentLinkFor(xml, "src/test/resources/xml/base.xml",
- dl(r(1, 38, 1, 53), "src/test/resources/dtd/base.dtd"));
+ dl(r(1, 38, 1, 62), "src/test/resources/dtd/entities/base.dtd"));
  }
 
  @Test
  public void noLinks() throws BadLocationException {
  String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\r\n" + //
- "<!DOCTYPE root-element \"ABCD\" \"../dtd/base.dtd\" [\r\n" + // here it misses PUBLIC
+ "<!DOCTYPE root-element \"ABCD\" \"../dtd/entities/base.dtd\" [\r\n" + // here it misses PUBLIC
  "\r\n" + //
  "]>\r\n" + //
  "<root-element />";

diff --git a/...c/test/java/org/eclipse/lemminx/extensions/entities/EntitiesCompletionExtensionsTest.java b/...c/test/java/org/eclipse/lemminx/extensions/entities/EntitiesCompletionExtensionsTest.java
@@ -128,7 +128,7 @@ public void none() throws BadLocationException {
  @Test
  public void external() throws BadLocationException {
  String xml = "<?xml version=\"1.0\" encoding=\"utf-8\" ?>\r\n" + //
- "<!DOCTYPE root-element SYSTEM \"src/test/resources/dtd/base.dtd\" [\r\n" + //
+ "<!DOCTYPE root-element SYSTEM \"src/test/resources/dtd/entities/base.dtd\" [\r\n" + //
  " <!ENTITY mdash \"&#x2014;\">\r\n" + //
  "]>\r\n" + //
  "<root-element>\r\n" + //
@@ -146,7 +146,7 @@ public void bug_vscode_xml_262() throws BadLocationException {
  // See
  // https:/redhat-developer/vscode-xml/issues/262#issuecomment-634716408
  String xml = "<?xml version=\"1.0\" encoding=\"utf-8\"?>\r\n" + //
- "<!DOCTYPE alex-update-sequence SYSTEM \"src/test/resources/dtd/bug_vscode-xml_262.dtd\" [<!-- {{{ -->\r\n"
+ "<!DOCTYPE alex-update-sequence SYSTEM \"src/test/resources/dtd/entities/bug_vscode-xml_262.dtd\" [<!-- {{{ -->\r\n"
  + //
  "]>\r\n" + //
  "<!-- }}} -->\r\n" + //