Issue #611: rewrote Javadoc HTML escaping, skip comment-less tags, si…

…mplified type param skipping.

	Change on 2015/09/21 by tball <tball@google.com>
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=103570844

translator/src/main/java/com/google/devtools/j2objc/ast/Javadoc.java

@@ -14,38 +14,25 @@
 
 package com.google.devtools.j2objc.ast;
 
-import org.eclipse.jdt.core.dom.ASTNode;
-import org.eclipse.jdt.core.dom.AbstractTypeDeclaration;
-import org.eclipse.jdt.core.dom.FieldDeclaration;
-import org.eclipse.jdt.core.dom.MethodDeclaration;
-import org.eclipse.jdt.core.dom.PackageDeclaration;
-
 import java.util.List;
 
 /**
  * Node for a javadoc-style comment.
  */
 public class Javadoc extends Comment {
 
-  public enum OwnerType {
-    FIELD, METHOD, PACKAGE, TYPE, OTHER
-  }
-
   private ChildList<TagElement> tags = ChildList.create(TagElement.class, this);
-  private final OwnerType ownerType;
 
   public Javadoc(org.eclipse.jdt.core.dom.Javadoc jdtNode) {
     super(jdtNode);
     for (Object tag : jdtNode.tags()) {
       tags.add((TagElement) TreeConverter.convert(tag));
     }
-    ownerType = ownerType(jdtNode);
   }
 
   public Javadoc(Javadoc other) {
     super(other);
     tags.copyFrom(other.getTags());
-    this.ownerType = other.ownerType;
   }
 
   @Override
@@ -74,25 +61,4 @@ protected void acceptInner(TreeVisitor visitor) {
   public Javadoc copy() {
     return new Javadoc(this);
   }
-
-  public OwnerType getOwnerType() {
-    return ownerType;
-  }
-
-  private OwnerType ownerType(org.eclipse.jdt.core.dom.Javadoc jdtNode) {
-    ASTNode parentNode = jdtNode.getParent();
-    if (parentNode instanceof PackageDeclaration) {
-      return OwnerType.PACKAGE;
-    } else if (parentNode instanceof AbstractTypeDeclaration) {
-      return OwnerType.TYPE;
-    } else if (parentNode instanceof MethodDeclaration) {
-      return OwnerType.METHOD;
-    } else if (parentNode instanceof FieldDeclaration) {
-      return OwnerType.FIELD;
-    }
-
-    // Other includes AnnotationTypeMemberDeclaration, and any future
-    // Javadoc types that aren't supported.
-    return OwnerType.OTHER;
-  }
 }

translator/src/main/java/com/google/devtools/j2objc/gen/JavadocGenerator.java

@@ -26,8 +26,10 @@
 import org.eclipse.jdt.core.dom.IVariableBinding;
 
 import java.text.BreakIterator;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 
 /**
  * Generates Javadoc comments.
@@ -39,6 +41,17 @@ public class JavadocGenerator extends AbstractSourceGenerator {
   // True when a <pre> tag is in a Javadoc tag, but not the closing </pre>.
   boolean spanningPreTag = false;
 
+  // All escapes are defined at "http://dev.w3.org/html5/html-author/charref".
+  private static final Map<Character, String> htmlEntities = new HashMap<>();
+  static {
+    htmlEntities.put('"',  "&quot;");
+    htmlEntities.put('\'', "&apos;");
+    htmlEntities.put('<',  "&lt;");
+    htmlEntities.put('>',  "&gt;");
+    htmlEntities.put('&',  "&amp;");
+    htmlEntities.put('@',  "&commat;");
+ }
+
   private JavadocGenerator(SourceBuilder builder) {
     super(builder);
   }
@@ -110,14 +123,10 @@ private void printDocLine(String line) {
   private String printTag(TagElement tag) {
     String tagName = tag.getTagName();
     if (tagName != null) {
-      // Remove param tags from class comments.
+      // Remove @param tags for parameterized types, such as "@param <T> the type".
       // TODO(tball): update when (if) Xcode supports Objective C type parameter documenting.
-      if (tagName.equals(TagElement.TAG_PARAM)) {
-        TreeNode parent = tag.getParent();
-        if (parent instanceof Javadoc
-            && ((Javadoc) parent).getOwnerType() == Javadoc.OwnerType.TYPE) {
-          return "";
-        }
+      if (tagName.equals(TagElement.TAG_PARAM) && hasTypeParam(tag.getFragments())) {
+        return "";
       }
 
       // Xcode 7 compatible tags.
@@ -128,7 +137,9 @@ private String printTag(TagElement tag) {
           || tagName.equals(TagElement.TAG_SINCE)
           || tagName.equals(TagElement.TAG_THROWS)
           || tagName.equals(TagElement.TAG_VERSION)) {
-        return String.format("%s %s", tagName, printTagFragments(tag.getFragments()).trim());
+        // Skip
+        String comment = printTagFragments(tag.getFragments()).trim();
+        return comment.isEmpty() ? "" : String.format("%s %s", tagName, comment);
       }
 
       if (tagName.equals(TagElement.TAG_DEPRECATED)) {
@@ -166,13 +177,21 @@ private String printTag(TagElement tag) {
         if (spanningPreTag) {
           return text;
         }
-//      return HtmlEscapers.htmlEscaper().escape(text);
-        return text;
+        return escapeHtmlText(text);
       }
     }
     return printTagFragments(tag.getFragments());
   }
 
+  private boolean hasTypeParam(List<TreeNode> fragments) {
+    // The format for a @param tag with a type parameter is:
+    // [ "<", Name, ">", comment ].
+    return fragments.size() >= 3
+        && "<".equals(fragments.get(0).toString())
+        && (fragments.get(1) instanceof SimpleName)
+        && ">".equals(fragments.get(2).toString());
+  }
+
   private String printTagFragments(List<TreeNode> fragments) {
     if (fragments.isEmpty()) {
       return "";
@@ -248,4 +267,17 @@ private String escapeCodeText(String text) {
   private String escapeDocText(String text) {
     return escapeCodeText(text.replace("@", "@@").replace("/*", "/\\*"));
   }
+
+  private String escapeHtmlText(String text) {
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < text.length(); i++) {
+      Character c = text.charAt(i);
+      if (htmlEntities.containsKey(c)) {
+        sb.append(htmlEntities.get(c));
+      } else {
+        sb.append(c);
+      }
+    }
+    return sb.toString();
+  }
 }

translator/src/test/java/com/google/devtools/j2objc/gen/JavadocGeneratorTest.java

@@ -63,20 +63,22 @@ public void testLinkTag() throws IOException {
     assertTranslation(translation, "@brief See <code>Test.bar</code>.");
   }
 
-  // TODO(tball): enable when we can use Guava's HtmlEscapers, or write custom escaping.
-//  public void testLiteralTag() throws IOException {
-//    String translation = translateSourceFile(
-//        "/** Class javadoc for {@literal <Test>}. */ class Test {}", "Test", "Test.h");
-//    assertTranslation(translation, "@brief Class javadoc for &lt;Test&gt;.");
-//  }
+  public void testLiteralTag() throws IOException {
+    String translation = translateSourceFile(
+        "/** Class javadoc for {@literal <Test>}. */ class Test {}", "Test", "Test.h");
+    assertTranslation(translation, "@brief Class javadoc for &lt;Test&gt;.");
+  }
 
   // Javadoc supports @param tags on classes, to document type parameters. Since there's
   // no equivalent in Objective C, these tags need to be removed.
-  public void testClassParamTagRemoval() throws IOException {
+  public void testTypeParamTagRemoval() throws IOException {
     String translation = translateSourceFile(
         "/** Class javadoc for Test.\n"
         + " * @param <T> the test name\n"
-        + " */ class Test <T> {}", "Test", "Test.h");
+        + " */ class Test <T> {\n"
+        + "  /** Method javadoc.\n"
+        + "   * @param <T> the type to be returned.\n"
+        + "   */ T test() { return null; }}", "Test", "Test.h");
     assertTranslation(translation, "@brief Class javadoc for Test.");
     assertNotInTranslation(translation, "@param");
     assertNotInTranslation(translation, "<T>");
@@ -137,4 +139,23 @@ public void testReservedParamName() throws IOException {
     assertTranslation(translation, "@param outArg Unused.");
     assertTranslation(translation, "@param description_ Unused.");
   }
+
+  // Verify that tags without following text are skipped, such as "@param\n".
+  public void testSkipEmptyTags() throws IOException {
+    String translation = translateSourceFile(
+        "/** Class javadoc for Test.\n"
+        + " * @see\n"
+        + " */ class Test { \n"
+        + "/** Method javadoc.\n"
+        + "  * @param \n"
+        + "  * @return\n"
+        + "  * @throws\n"
+        + "  */\n"
+        + "boolean test(int foo) { return false; } }", "Test", "Test.h");
+    assertTranslation(translation, "@brief Class javadoc for Test.");
+    assertNotInTranslation(translation, "@since");
+    assertNotInTranslation(translation, "@param");
+    assertNotInTranslation(translation, "@return");
+    assertNotInTranslation(translation, "@throws");
+  }
 }