SONAR-2018 Improve rule for detection of commented-out code

* Only one violation in one block of comment.
* Proper recognition of Javadocs.

5 files changed, 141 insertions, 45 deletions

diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/CommentedOutCodeLineCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/CommentedOutCodeLineCheck.java
new file mode 100644
index 00000000000..d4d42781297
--- /dev/null
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/check/CommentedOutCodeLineCheck.java
@@ -0,0 +1,126 @@
+/*
+ * Sonar, open source software quality management tool.
+ * Copyright (C) 2008-2011 SonarSource
+ * mailto:contact AT sonarsource DOT com
+ *
+ * Sonar is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * Sonar is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with Sonar; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
+ */
+package org.sonar.java.ast.check;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+
+import org.sonar.check.Priority;
+import org.sonar.check.Rule;
+import org.sonar.java.ast.visitor.AstUtils;
+import org.sonar.java.ast.visitor.JavaAstVisitor;
+import org.sonar.java.recognizer.JavaFootprint;
+import org.sonar.squid.api.CheckMessage;
+import org.sonar.squid.api.SourceFile;
+import org.sonar.squid.recognizer.CodeRecognizer;
+
+import com.google.common.collect.Sets;
+import com.puppycrawl.tools.checkstyle.api.DetailAST;
+import com.puppycrawl.tools.checkstyle.api.TextBlock;
+import com.puppycrawl.tools.checkstyle.api.TokenTypes;
+
+@Rule(key = "CommentedOutCodeLine", priority = Priority.MAJOR)
+public class CommentedOutCodeLineCheck extends JavaAstVisitor {
+
+  private static final double THRESHOLD = 0.9;
+
+  /**
+   * This list was taken from com.puppycrawl.tools.checkstyle.checks.javadoc.JavadocStyleCheck
+   */
+  private static final List<Integer> WANTED_TOKENS = Arrays.asList(
+      TokenTypes.INTERFACE_DEF,
+      TokenTypes.CLASS_DEF,
+      TokenTypes.ANNOTATION_DEF,
+      TokenTypes.ENUM_DEF,
+      TokenTypes.METHOD_DEF,
+      TokenTypes.CTOR_DEF,
+      TokenTypes.VARIABLE_DEF,
+      TokenTypes.ENUM_CONSTANT_DEF,
+      TokenTypes.ANNOTATION_FIELD_DEF,
+      TokenTypes.PACKAGE_DEF);
+
+  private final CodeRecognizer codeRecognizer;
+  private Set<TextBlock> comments;
+
+  public CommentedOutCodeLineCheck() {
+    codeRecognizer = new CodeRecognizer(THRESHOLD, new JavaFootprint());
+  }
+
+  @Override
+  public List<Integer> getWantedTokens() {
+    return WANTED_TOKENS;
+  }
+
+  @Override
+  public void visitFile(DetailAST ast) {
+    comments = Sets.newHashSet();
+
+    for (TextBlock comment : getFileContents().getCppComments().values()) {
+      comments.add(comment);
+    }
+
+    for (List<TextBlock> listOfComments : getFileContents().getCComments().values()) {
+      // This list contains not only comments in C style, but also Javadocs
+      for (TextBlock comment : listOfComments) {
+        comments.add(comment);
+      }
+    }
+  }
+
+  /**
+   * Documentation comments should be recognized only when placed
+   * immediately before class, interface, constructor, method, or field declarations.
+   */
+  @Override
+  public void visitToken(DetailAST ast) {
+    if (shouldHandle(ast)) {
+      TextBlock javadoc = getFileContents().getJavadocBefore(ast.getLineNo());
+      if (javadoc != null) {
+        comments.remove(javadoc);
+      }
+    }
+  }
+
+  private static boolean shouldHandle(DetailAST ast) {
+    if (AstUtils.isType(ast, TokenTypes.VARIABLE_DEF)) {
+      return AstUtils.isClassVariable(ast);
+    }
+    return true;
+  }
+
+  @Override
+  public void leaveFile(DetailAST ast) {
+    SourceFile sourceFile = (SourceFile) peekSourceCode();
+    for (TextBlock comment : comments) {
+      String[] lines = comment.getText();
+      for (int i = 0; i < lines.length; i++) {
+        if (codeRecognizer.isLineOfCode(lines[i])) {
+          CheckMessage message = new CheckMessage(this, "It's better to remove commented-out line of code.");
+          message.setLine(comment.getStartLineNo() + i);
+          sourceFile.log(message);
+          break;
+        }
+      }
+    }
+    comments = null;
+  }
+
+}
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/CommentVisitor.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/CommentVisitor.java
index 1be27168475..5fd989083c0 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/CommentVisitor.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/ast/visitor/CommentVisitor.java
@@ -51,7 +51,6 @@ public class CommentVisitor extends JavaAstVisitor {
   public void visitFile(DetailAST ast) {
     SourceFile file = (SourceFile) peekSourceCode();
     file.addNoSonarTagLines(getSource().getNoSonarTagLines());
-    file.addCommentedOutCodeLines(getSource().getCommentedOutCodeLines());
     file.setMeasure(Metric.HEADER_COMMENT_LINES, getSource().getMeasure(Metric.HEADER_COMMENT_LINES));
     file.setMeasure(Metric.COMMENTED_OUT_CODE_LINES, getSource().getMeasure(Metric.COMMENTED_OUT_CODE_LINES));
     file.setMeasure(Metric.COMMENT_LINES, getSource().getMeasure(Metric.COMMENT_LINES));
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CommentedOutCodeLineCheck.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CommentedOutCodeLineCheck.java
deleted file mode 100644
index 1e65f01c51b..00000000000
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/java/squid/check/CommentedOutCodeLineCheck.java
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Sonar, open source software quality management tool.
- * Copyright (C) 2008-2011 SonarSource
- * mailto:contact AT sonarsource DOT com
- *
- * Sonar is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or (at your option) any later version.
- *
- * Sonar is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with Sonar; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
- */
-package org.sonar.java.squid.check;
-
-import org.sonar.check.Priority;
-import org.sonar.check.Rule;
-import org.sonar.squid.api.CheckMessage;
-import org.sonar.squid.api.SourceFile;
-
-@Rule(key = "CommentedOutCodeLine", priority = Priority.MAJOR)
-public class CommentedOutCodeLineCheck extends SquidCheck {
-
-  @Override
-  public void visitFile(SourceFile sourceFile) {
-    for (Integer line : sourceFile.getCommentedOutCodeLines()) {
-      CheckMessage message = new CheckMessage(this, "It's better to remove commented-out line of code.");
-      message.setLine(line);
-      sourceFile.log(message);
-    }
-  }
-
-}
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java
index f2a93a76dbe..017ae33a215 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/SquidRuleRepository.java
@@ -28,6 +28,7 @@ import org.sonar.api.rules.AnnotationRuleParser;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleRepository;
 import org.sonar.java.ast.check.BreakCheck;
+import org.sonar.java.ast.check.CommentedOutCodeLineCheck;
 import org.sonar.java.ast.check.ContinueCheck;
 import org.sonar.java.ast.check.UndocumentedApiCheck;
 import org.sonar.java.bytecode.check.ArchitectureCheck;
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/squid/check/CommentedOutCodeLineCheckTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java
index a479d3a409c..b231567b3b8 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/squid/check/CommentedOutCodeLineCheckTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java
@@ -17,7 +17,7 @@
  * License along with Sonar; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02
  */
-package org.sonar.java.squid.check;
+package org.sonar.java.ast.check;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -48,10 +48,19 @@ public class CommentedOutCodeLineCheckTest {
   @Test
   public void testDetection() {
     CheckMessages checkMessages = new CheckMessages((SourceFile) squid.search("CommentedCode.java"));
-    checkMessages.assertNext().atLine(4);
-    checkMessages.assertNext().atLine(17);
-    checkMessages.assertNext().atLine(18);
-    checkMessages.assertNext().atLine(19);
+
+    checkMessages.assertNext().atLine(26);
+    checkMessages.assertNext().atLine(27);
+    checkMessages.assertNext().atLine(28);
+
+    checkMessages.assertNext().atLine(32);
+
+    checkMessages.assertNext().atLine(38);
+    checkMessages.assertNext().atLine(39);
+    checkMessages.assertNext().atLine(40);
+
+    checkMessages.assertNext().atLine(44);
+
     checkMessages.assertNoMore();
   }