SONAR-1901: "Undocumented API" is not defined as a rule in Sonar

* Add abstract class JavaAstCheck
* Change description of rule
* Check must not extend the PublicApiVisitor
* Add an optional new parameter to rule whose value would be a list of wildcard patterns

5 files changed, 100 insertions, 43 deletions

diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java
new file mode 100644
index 00000000000..048e39a0a00
--- /dev/null
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/PatternUtils.java
@@ -0,0 +1,36 @@
+package org.sonar.java;
+
+import org.apache.commons.lang.StringUtils;
+import org.sonar.api.utils.WildcardPattern;
+
+import com.google.common.collect.Lists;
+
+import java.util.List;
+
+public final class PatternUtils {
+
+  private PatternUtils() {
+  }
+
+  public static List<WildcardPattern> createMatchers(String pattern) {
+    List<WildcardPattern> matchers = Lists.newArrayList();
+    if (StringUtils.isNotEmpty(pattern)) {
+      String[] patterns = pattern.split(",");
+      for (String p : patterns) {
+        p = StringUtils.replace(p, ".", "/");
+        matchers.add(WildcardPattern.create(p));
+      }
+    }
+    return matchers;
+  }
+
+  public static boolean matches(String text, List<WildcardPattern> matchers) {
+    for (WildcardPattern matcher : matchers) {
+      if (matcher.match(text)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+}
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/JavaAstCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/JavaAstCheck.java
new file mode 100644
index 00000000000..502f7e3f71b
--- /dev/null
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/JavaAstCheck.java
@@ -0,0 +1,11 @@
+package org.sonar.java.ast.check;
+
+import org.sonar.java.ast.visitor.JavaAstVisitor;
+import org.sonar.squid.api.CodeCheck;
+
+public abstract class JavaAstCheck extends JavaAstVisitor implements CodeCheck {
+
+  public String getKey() {
+    return getClass().getSimpleName();
+  }
+}
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
index df08d0471a3..c508ad95388 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/UndocumentedApiCheck.java
@@ -1,32 +1,67 @@
 package org.sonar.java.ast.check;
 
+import org.apache.commons.lang.StringUtils;
+import org.sonar.api.utils.WildcardPattern;
 import org.sonar.check.IsoCategory;
 import org.sonar.check.Priority;
 import org.sonar.check.Rule;
+import org.sonar.check.RuleProperty;
+import org.sonar.java.PatternUtils;
 import org.sonar.java.ast.visitor.PublicApiVisitor;
 import org.sonar.squid.api.CheckMessage;
-import org.sonar.squid.api.CodeCheck;
+import org.sonar.squid.api.SourceClass;
 import org.sonar.squid.api.SourceCode;
 import org.sonar.squid.api.SourceFile;
+import org.sonar.squid.measures.Metric;
 
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 
-@Rule(key = "UndocumentedApi", name = "Undocumented API", isoCategory = IsoCategory.Usability, priority = Priority.MAJOR, description = "")
-public class UndocumentedApiCheck extends PublicApiVisitor implements CodeCheck {
+import java.util.List;
+
+@Rule(key = "UndocumentedApi", name = "Undocumented API", isoCategory = IsoCategory.Usability, priority = Priority.MAJOR, description = "<p>Check that each public class, interface, method and constructor has a Javadoc comment. " +
+    "The following public methods/constructors are not concerned by this rule :</p>" +
+    "<ul><li>Getter / Setter</li>" +
+    "<li>Method with @Override annotation</li>" +
+    "<li>Empty constructor</li></ul>")
+public class UndocumentedApiCheck extends JavaAstCheck {
+
+  @RuleProperty(description = "Optional. If this property is not defined, all classes should adhere to this constraint. Ex : **.api.**")
+  private String forClasses = new String();
+
+  private List<WildcardPattern> matchers;
+
+  @Override
+  public List<Integer> getWantedTokens() {
+    return PublicApiVisitor.TOKENS;
+  }
 
   @Override
   public void visitToken(DetailAST ast) {
-    if (isPublicApi(ast) && !isDocumentedApi(ast)) {
-      SourceCode currentResource = peekSourceCode();
-      SourceFile sourceFile = currentResource.getParent(SourceFile.class);
-      CheckMessage message = new CheckMessage(this, "Avoid undocumented API");
-      message.setLine(ast.getLineNo());
-      sourceFile.log(message);
+    SourceCode currentResource = peekSourceCode();
+    SourceClass sourceClass = peekParentClass();
+    if (PatternUtils.matches(sourceClass.getKey(), getMatchers())) {
+      if (currentResource.getDouble(Metric.PUBLIC_API) > 0 && currentResource.getDouble(Metric.PUBLIC_DOC_API) == 0) {
+        SourceFile sourceFile = currentResource.getParent(SourceFile.class);
+        CheckMessage message = new CheckMessage(this, "Avoid undocumented API");
+        message.setLine(ast.getLineNo());
+        sourceFile.log(message);
+      }
     }
   }
 
-  public String getKey() {
-    return getClass().getSimpleName();
+  private List<WildcardPattern> getMatchers() {
+    if (matchers == null) {
+      matchers = PatternUtils.createMatchers(StringUtils.defaultIfEmpty(forClasses, "**"));
+    }
+    return matchers;
+  }
+
+  public String getForClasses() {
+    return forClasses;
+  }
+
+  public void setForClasses(String forClasses) {
+    this.forClasses = forClasses;
   }
 
 }
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/PublicApiVisitor.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/PublicApiVisitor.java
index 92e3fd25112..625387ba0fb 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/PublicApiVisitor.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/PublicApiVisitor.java
@@ -35,7 +35,7 @@ public class PublicApiVisitor extends JavaAstVisitor {
 
   final static String OVERRIDE_ANNOTATION_KEYWORD = "Override";
 
-  private static final List<Integer> TOKENS = Arrays.asList(TokenTypes.CLASS_DEF, TokenTypes.INTERFACE_DEF, TokenTypes.METHOD_DEF,
+  public static final List<Integer> TOKENS = Arrays.asList(TokenTypes.CLASS_DEF, TokenTypes.INTERFACE_DEF, TokenTypes.METHOD_DEF,
       TokenTypes.CTOR_DEF, TokenTypes.ANNOTATION_DEF, TokenTypes.ANNOTATION_FIELD_DEF, TokenTypes.VARIABLE_DEF);
 
   public PublicApiVisitor() {
@@ -57,7 +57,7 @@ public class PublicApiVisitor extends JavaAstVisitor {
     }
   }
 
-  protected boolean isPublicApi(DetailAST ast) {
+  private boolean isPublicApi(DetailAST ast) {
     return isPublic(ast) && !isStaticFinalVariable(ast) && !isMethodWithOverrideAnnotation(ast) && !isEmptyDefaultConstructor(ast);
   }
 
@@ -94,7 +94,7 @@ public class PublicApiVisitor extends JavaAstVisitor {
     return (AstUtils.isScope(AstUtils.getScope(ast), Scope.PUBLIC) || AstUtils.isType(ast, TokenTypes.ANNOTATION_FIELD_DEF));
   }
 
-  protected boolean isDocumentedApi(DetailAST ast) {
+  private boolean isDocumentedApi(DetailAST ast) {
     return getFileContents().getJavadocBefore(ast.getLineNo()) != null;
   }
 
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
index 17f06cc5cd3..ee89cf2100f 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/bytecode/check/ArchitectureCheck.java
@@ -22,12 +22,12 @@ package org.sonar.java.bytecode.check;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.api.utils.WildcardPattern;
 import org.sonar.check.*;
+import org.sonar.java.PatternUtils;
 import org.sonar.java.bytecode.asm.AsmClass;
 import org.sonar.java.bytecode.asm.AsmEdge;
 import org.sonar.squid.api.CheckMessage;
 import org.sonar.squid.api.SourceFile;
 
-import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 
 import java.util.List;
@@ -88,7 +88,7 @@ public class ArchitectureCheck extends BytecodeCheck {
       String internalNameTargetClass = edge.getTargetAsmClass().getInternalName();
       if ( !internalNames.containsKey(internalNameTargetClass)) {
         String nameAsmClass = asmClass.getInternalName();
-        if (matches(nameAsmClass, getFromMatchers()) && matches(internalNameTargetClass, getToMatchers())) {
+        if (PatternUtils.matches(nameAsmClass, getFromMatchers()) && PatternUtils.matches(internalNameTargetClass, getToMatchers())) {
           logMessage(edge);
         }
       } else if (internalNames.get(internalNameTargetClass).getLine() == 0 && edge.getSourceLineNumber() != 0) {
@@ -105,41 +105,16 @@ public class ArchitectureCheck extends BytecodeCheck {
     internalNames.put(toClass, message);
   }
 
-  private boolean matches(String className, List<WildcardPattern> matchers) {
-    for (WildcardPattern matcher : matchers) {
-      if (matcher.match(className)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  private List<WildcardPattern> createMatchers(String pattern) {
-    List<WildcardPattern> matchers = Lists.newArrayList();
-    if (StringUtils.isNotEmpty(pattern)) {
-      String[] patterns = pattern.split(",");
-      for (String p : patterns) {
-        p = StringUtils.replace(p, ".", "/");
-        matchers.add(WildcardPattern.create(p));
-      }
-    }
-    return matchers;
-  }
-
   private List<WildcardPattern> getFromMatchers() {
     if (fromMatchers == null) {
-      if (StringUtils.isBlank(fromClasses)) {
-        fromMatchers = createMatchers("**");
-      } else {
-        fromMatchers = createMatchers(fromClasses);
-      }
+      fromMatchers = PatternUtils.createMatchers(StringUtils.defaultIfEmpty(fromClasses, "**"));
     }
     return fromMatchers;
   }
 
   private List<WildcardPattern> getToMatchers() {
     if (toMatchers == null) {
-      toMatchers = createMatchers(toClasses);
+      toMatchers = PatternUtils.createMatchers(toClasses);
     }
     return toMatchers;
   }