You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2020/08/30 16:21:12 UTC

[lucene-solr] branch LUCENE-9215 updated: LUCENE-9215: more extensive javadocs checks by default

This is an automated email from the ASF dual-hosted git repository.

rmuir pushed a commit to branch LUCENE-9215
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/LUCENE-9215 by this push:
     new 2c365f1  LUCENE-9215: more extensive javadocs checks by default
2c365f1 is described below

commit 2c365f15768af6f4b0835344117ea3e25e054892
Author: Robert Muir <rm...@apache.org>
AuthorDate: Sun Aug 30 12:20:23 2020 -0400

    LUCENE-9215: more extensive javadocs checks by default
    
    Add "parameter" which, in addition to the checks done by "method",
    validates that there is an @param tag for each formal parameter of a
    method/constructor.
    
    This is a good win for an API, documenting the parameters in this way
    plugs in nicely into IDEs and users can just hover over things to pop up
    precisely what they need to see.
    
    Some modules only require just a few fixes to lock this in, so we should
    fix the easy wins at least (I fixed lucene/memory since it literally
    only had ONE problem).
---
 .../org/apache/lucene/gradle/MissingDoclet.java    | 43 ++++++++++++++++++++--
 gradle/documentation/render-javadoc.gradle         | 31 ++++++++++++----
 .../apache/lucene/index/memory/MemoryIndex.java    |  1 +
 3 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/buildSrc/src/main/java/org/apache/lucene/gradle/MissingDoclet.java b/buildSrc/src/main/java/org/apache/lucene/gradle/MissingDoclet.java
index 413b925..75510ad 100644
--- a/buildSrc/src/main/java/org/apache/lucene/gradle/MissingDoclet.java
+++ b/buildSrc/src/main/java/org/apache/lucene/gradle/MissingDoclet.java
@@ -25,10 +25,14 @@ import java.util.Set;
 
 import javax.lang.model.element.Element;
 import javax.lang.model.element.ElementKind;
+import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.ModuleElement;
+import javax.lang.model.element.VariableElement;
 import javax.lang.model.util.Elements;
 import javax.tools.Diagnostic;
 
+import com.sun.source.doctree.DocCommentTree;
+import com.sun.source.doctree.ParamTree;
 import com.sun.source.util.DocTrees;
 
 import jdk.javadoc.doclet.Doclet;
@@ -38,7 +42,7 @@ import jdk.javadoc.doclet.StandardDoclet;
 
 /**
  * Checks for missing javadocs, where missing also means "only whitespace" or "license header".
- * Has option --missing-level (package, class, method) so that we can improve over time.
+ * Has option --missing-level (package, class, method, parameter) so that we can improve over time.
  * Has option --missing-ignore to ignore individual elements (such as split packages). 
  *   It isn't recursive, just ignores exactly the elements you tell it.
  *   This should be removed when packaging is fixed to no longer be split across JARs.
@@ -46,10 +50,15 @@ import jdk.javadoc.doclet.StandardDoclet;
  *   Matches package names exactly: so you'll need to list subpackages separately.
  */
 public class MissingDoclet extends StandardDoclet {
+  // checks that modules and packages have documentation
   private static final int PACKAGE = 0;
+  // + checks that classes, interfaces, enums, and annotation types have documentation
   private static final int CLASS = 1;
+  // + checks that methods, constructors, fields, and enumerated constants have documentation
   private static final int METHOD = 2;
-  int level = METHOD;
+  // + checks that @param tags are present for any method/constructor parameters
+  private static final int PARAMETER = 3;
+  int level = PARAMETER;
   Reporter reporter;
   DocletEnvironment docEnv;
   DocTrees docTrees;
@@ -69,7 +78,7 @@ public class MissingDoclet extends StandardDoclet {
 
       @Override
       public String getDescription() {
-        return "level to enforce for missing javadocs: [package, class, method]";
+        return "level to enforce for missing javadocs: [package, class, method, parameter]";
       }
 
       @Override
@@ -99,6 +108,8 @@ public class MissingDoclet extends StandardDoclet {
           case "method":
             level = METHOD;
             return true;
+          case "parameter":
+            level = PARAMETER;
           default:
             return false;
         }
@@ -285,7 +296,7 @@ public class MissingDoclet extends StandardDoclet {
     }
     // check that this element isn't on our ignore list. This is only used as a workaround for "split packages".
     // ignoring a package isn't recursive (on purpose), we still check all the classes, etc. inside it.
-    // we just need to cope with the fact module-info.java isn't there because it is split across multiple jars.
+    // we just need to cope with the fact package-info.java isn't there because it is split across multiple jars.
     if (ignored.contains(element.toString())) {
       return;
     }
@@ -304,6 +315,30 @@ public class MissingDoclet extends StandardDoclet {
         error(element, "comment is really a license");
       }
     }
+    if (level >= PARAMETER && tree != null) {
+      checkParameters(element, tree);
+    }
+  }
+  
+  /** Checks there is a corresponding "param" tag for each method parameter */
+  private void checkParameters(Element element, DocCommentTree tree) {
+    if (element instanceof ExecutableElement) {
+      // record each @param that we see
+      Set<String> seenParameters = new HashSet<>();
+      for (var tag : tree.getBlockTags()) {
+        if (tag instanceof ParamTree) {
+          var name = ((ParamTree)tag).getName().getName().toString();
+          seenParameters.add(name);
+        }
+      }
+      // now compare the method's formal parameter list against it
+      for (var param : ((ExecutableElement)element).getParameters()) {
+        var name = ((VariableElement)param).getSimpleName().toString();
+        if (!seenParameters.contains(name)) {
+          error(element, "missing javadoc @param for parameter '" + name + "'");
+        }
+      }
+    }
   }
   
   /** logs a new error for the particular element */
diff --git a/gradle/documentation/render-javadoc.gradle b/gradle/documentation/render-javadoc.gradle
index 66413b7..680b600 100644
--- a/gradle/documentation/render-javadoc.gradle
+++ b/gradle/documentation/render-javadoc.gradle
@@ -114,6 +114,25 @@ configure([
   }
 }
 
+configure([
+    project(":lucene:analysis:icu"),
+    project(":lucene:analysis:morfologik"),
+    project(":lucene:analysis:phonetic"),
+    project(":lucene:analysis:stempel"),
+    project(":lucene:classification"),
+    project(":lucene:demo"),
+    project(":lucene:expressions"),
+    project(":lucene:facet"),
+    project(":lucene:join"),
+    project(":lucene:spatial3d"),
+    project(":lucene:suggest"),
+  ]) {
+  project.tasks.withType(RenderJavadocTask) {
+    // TODO: fix missing @param tags
+    javadocMissingLevel = "method"
+  }
+}
+
 configure(project(":lucene:analysis:icu")) {
   project.tasks.withType(RenderJavadocTask) {
     // TODO: clean up split packages
@@ -126,6 +145,8 @@ configure(project(":lucene:analysis:icu")) {
 
 configure(project(":lucene:backward-codecs")) {
   project.tasks.withType(RenderJavadocTask) {
+    // TODO: fix missing @param tags
+    javadocMissingLevel = "method"
     // TODO: clean up split packages
     javadocMissingIgnore = [
         "org.apache.lucene.codecs",
@@ -219,7 +240,7 @@ configure(project(":lucene:core")) {
   }
 }
 
-configure(project(":solr")) {
+configure(project(":solr").allprojects) {
   project.tasks.withType(RenderJavadocTask) {
     // TODO: fix missing javadocs
     javadocMissingLevel = "package"
@@ -268,12 +289,6 @@ configure(project(":solr:solrj")) {
   }
 }
 
-configure(allprojects.findAll { it.path.startsWith(':solr') }) {
-  project.tasks.withType(RenderJavadocTask) {
-    javadocMissingLevel = "package"
-  }
-}
-
 configure(project(":solr:test-framework")) {
   project.tasks.withType(RenderJavadocTask) {
     // TODO: clean up split packages
@@ -366,7 +381,7 @@ class RenderJavadocTask extends DefaultTask {
 
   // default is to require full javadocs
   @Input
-  String javadocMissingLevel = "method"
+  String javadocMissingLevel = "parameter"
 
   // anything in these packages is checked with level=method. This allows iteratively fixing one package at a time.
   @Input
diff --git a/lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java b/lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java
index dacee10..705e0cc 100644
--- a/lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java
+++ b/lucene/memory/src/java/org/apache/lucene/index/memory/MemoryIndex.java
@@ -629,6 +629,7 @@ public class MemoryIndex {
 
   /**
    * Set the Similarity to be used for calculating field norms
+   * @param similarity instance with custom {@link Similarity#computeNorm} implementation
    */
   public void setSimilarity(Similarity similarity) {
     if (frozen)