IllegalArgumentException when getting Javadoc for nested class that has no Javadoc #69

A subclass with no Javadoc does not inherit method docs from superclass #70

Loosen the class name check, so it doesn't complain about canonical versus
non-canonical class name. Make it an assertion, so we only pay the cost
when running tests.

When scanning for inherited Javadoc, rework the "isEmpty" check.
Instead of bailing out early if the subclass has no Javadoc,
Wait until after the scan is complete.

Author: dnault

Diff for: therapi-runtime-javadoc-scribe/src/test/java/com/github/therapi/runtimejavadoc/JavadocAnnotationProcessorTest.java

@@ -548,6 +548,31 @@ public void nestedClassNameIsPreserved() throws Exception {
                 ClassJavadoc classJavadoc = expectJavadoc(c);
                 assertEquals(DOCUMENTED_RECORD + ".Nested", classJavadoc.getName());
             }
+
+            {
+                Class<?> c = classLoader.loadClass(DOCUMENTED_CLASS + "$NestedWithoutJavadoc");
+                expectNoJavadoc(c);
+            }
+
+            {
+                Class<?> c = classLoader.loadClass(DOCUMENTED_RECORD + "$NestedWithoutJavadoc");
+                expectNoJavadoc(c);
+            }
+        }
+    }
+
+    @Test
+    public void emptyNestedClassInheritsJavadoc() throws Exception {
+        try (CompilationClassLoader classLoader = compile(null)) {
+            Class<?> c = classLoader.loadClass(DOCUMENTED_CLASS + "$NestedSubclassWithoutJavadoc");
+            ClassJavadoc classJavadoc = expectJavadoc(c);
+
+            final String methodName = "frobulate";
+            Method m1 = c.getMethod(methodName, String.class, int.class);
+            Method m2 = c.getMethod(methodName, String.class, List.class);
+
+            assertMethodDescriptionMatches(m1, "Frobulate <code>a</code> by <code>b</code>");
+            assertMethodDescriptionMatches(m2, "Frobulate <code>a</code> by multiple oopsifizzle constants");
         }
     }
 
@@ -589,6 +614,7 @@ public void noCompanionGeneratedForUndocumentedClass() throws Exception {
         try (CompilationClassLoader classLoader = compile(null)) {
             expectNoJavadoc(classLoader.loadClass(UNDOCUMENTED));
             expectNoJavadoc(classLoader.loadClass(BLANK_COMMENTS));
+            expectNoJavadoc(classLoader.loadClass(BLANK_COMMENTS + "$NestedBlankBlankSubclass"));
         }
     }
 

Diff for: therapi-runtime-javadoc-scribe/src/test/resources/javasource/bar/BlankDocumentation.java

@@ -9,4 +9,7 @@ public class BlankDocumentation {
      */
     public void foo() {
     }
+
+    public static class NestedBlankBlankSubclass extends BlankDocumentation {
+    }
 }

Diff for: therapi-runtime-javadoc-scribe/src/test/resources/javasource/foo/DocumentedClass.java

@@ -128,4 +128,11 @@ public boolean equals(Object o) {
    */
   public static class Nested {
   }
+
+  public static class NestedWithoutJavadoc {
+  }
+
+  public static class NestedSubclassWithoutJavadoc extends DocumentedClass<Object> {
+  }
+
 }

Diff for: therapi-runtime-javadoc-scribe/src/test/resources/javasource/foo/DocumentedRecord.java

@@ -32,4 +32,7 @@ public String someMethod(boolean frobulate) {
      */
     public static class Nested {
     }
+
+    public static class NestedWithoutJavadoc {
+    }
 }

Diff for: therapi-runtime-javadoc/src/main/java/com/github/therapi/runtimejavadoc/ClassJavadoc.java

@@ -88,8 +88,7 @@ private ClassJavadoc(String name, Comment comment, Map<String, FieldJavadoc> fie
     }
 
     public static ClassJavadoc createEmpty(String qualifiedClassName) {
-        return new ClassJavadoc(qualifiedClassName, null, (List<FieldJavadoc>) null, null, null, null, null, null,
-                                null) {
+        return new ClassJavadoc(qualifiedClassName, null, (List<FieldJavadoc>) null, null, null, null, null, null, null) {
             @Override
             public boolean isEmpty() {
                 return true;
@@ -98,26 +97,29 @@ public boolean isEmpty() {
     }
 
     ClassJavadoc createEnhancedClassJavadoc(Class<?> clazz) {
-        if (!getName().equals(clazz.getCanonicalName())) {
-            throw new IllegalArgumentException("Class `" + clazz.getCanonicalName() + "` does not match class doc for `" + getName() + "`");
-        }
-
-        if (isEmpty()) {
-            return this;
-        }
+        assert getName().replace("$", ".").equals(clazz.getCanonicalName())
+                : "Class `" + clazz.getName() + "` does not match class doc for `" + getName() + "`";
 
         Map<String, ClassJavadoc> classJavadocCache = new HashMap<>();
 
         classJavadocCache.put(clazz.getCanonicalName(), this);
         getAllTypeAncestors(clazz).forEach(cls -> classJavadocCache.put(cls.getCanonicalName(), RuntimeJavadoc.getSkinnyClassJavadoc(cls)));
 
         Map<MethodSignature, MethodJavadoc> methodJavadocs = new LinkedHashMap<>();
-        Arrays.stream(clazz.getDeclaredMethods())
-              .forEach(method -> methodJavadocs.put(MethodSignature.from(method),
-                                                    RuntimeJavadoc.getJavadoc(method, classJavadocCache)));
+        Arrays.stream(clazz.getMethods())
+                .forEach(method -> {
+                    MethodJavadoc methodJavadoc = RuntimeJavadoc.getJavadoc(method, classJavadocCache);
+                    if (!methodJavadoc.isEmpty()) {
+                        methodJavadocs.put(MethodSignature.from(method), methodJavadoc);
+                    }
+                });
+
+        if (methodJavadocs.isEmpty()) {
+            return this; // didn't inherit anything.
+        }
 
         return new ClassJavadoc(getName(), getComment(), fields, enumConstants, methodJavadocs, constructors,
-                                getOther(), getSeeAlso(), recordComponents);
+                getOther(), getSeeAlso(), recordComponents);
     }
 
     public List<FieldJavadoc> getFields() {
@@ -172,13 +174,13 @@ ParamJavadoc findRecordComponent(String recordComponent) {
     @Override
     public String toString() {
         return "ClassJavadoc{"
-               + "name='" + getName() + '\''
-               + ", comment=" + getComment()
-               + ", fields=" + fields
-               + ", methods=" + methods
-               + ", constructors=" + constructors
-               + ", recordComponents=" + recordComponents
-               + ", seeAlso=" + getSeeAlso()
-               + ", other=" + getOther() + '}';
+                + "name='" + getName() + '\''
+                + ", comment=" + getComment()
+                + ", fields=" + fields
+                + ", methods=" + methods
+                + ", constructors=" + constructors
+                + ", recordComponents=" + recordComponents
+                + ", seeAlso=" + getSeeAlso()
+                + ", other=" + getOther() + '}';
     }
 }