SONAR-3093 Java: "comments" metric should include "commented out lines of code"

6 files changed, 65 insertions, 10 deletions

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 5fd989083c0..44ae96d8a42 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
@@ -28,6 +28,10 @@ import org.sonar.squid.measures.Metric;
 import com.puppycrawl.tools.checkstyle.api.DetailAST;
 import com.puppycrawl.tools.checkstyle.api.TokenTypes;
 
+/**
+ * SONAR-3093: Note that this visitor saves {@link Metric#COMMENT_LINES} into {@link org.sonar.squid.api.SourceCode}
+ * as sum of {@link Metric#COMMENT_LINES} and {@link Metric#COMMENTED_OUT_CODE_LINES} from {@link org.sonar.squid.test.Source}.
+ */
 public class CommentVisitor extends JavaAstVisitor {
 
   private static final List<Integer> WANTED_TOKENS = Arrays.asList(TokenTypes.RCURLY);
@@ -41,9 +45,10 @@ public class CommentVisitor extends JavaAstVisitor {
   public void visitToken(DetailAST ast) {
     int startAtLine = peekSourceCode().getStartAtLine();
     int endAtLine = peekSourceCode().getEndAtLine();
-    peekSourceCode().setMeasure(Metric.COMMENTED_OUT_CODE_LINES,
-        getSource().getMeasure(Metric.COMMENTED_OUT_CODE_LINES, startAtLine, endAtLine));
-    peekSourceCode().setMeasure(Metric.COMMENT_LINES, getSource().getMeasure(Metric.COMMENT_LINES, startAtLine, endAtLine));
+    int commentedOutCodeLines = getSource().getMeasure(Metric.COMMENTED_OUT_CODE_LINES, startAtLine, endAtLine);
+    int commentLines = getSource().getMeasure(Metric.COMMENT_LINES, startAtLine, endAtLine);
+    peekSourceCode().setMeasure(Metric.COMMENTED_OUT_CODE_LINES, commentedOutCodeLines);
+    peekSourceCode().setMeasure(Metric.COMMENT_LINES, commentLines + commentedOutCodeLines);
     peekSourceCode().setMeasure(Metric.COMMENT_BLANK_LINES, getSource().getMeasure(Metric.COMMENT_BLANK_LINES, startAtLine, endAtLine));
   }
 
@@ -52,8 +57,10 @@ public class CommentVisitor extends JavaAstVisitor {
     SourceFile file = (SourceFile) peekSourceCode();
     file.addNoSonarTagLines(getSource().getNoSonarTagLines());
     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));
+    int commentedOutCodeLines = getSource().getMeasure(Metric.COMMENTED_OUT_CODE_LINES);
+    int commentLines = getSource().getMeasure(Metric.COMMENT_LINES);
+    file.setMeasure(Metric.COMMENTED_OUT_CODE_LINES, commentedOutCodeLines);
+    file.setMeasure(Metric.COMMENT_LINES, commentLines + commentedOutCodeLines);
     file.setMeasure(Metric.COMMENT_BLANK_LINES, getSource().getMeasure(Metric.COMMENT_BLANK_LINES));
   }
 
diff --git a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridge.java b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridge.java
index 0d596dda816..a854d477422 100644
--- a/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridge.java
+++ b/plugins/sonar-squid-java-plugin/src/main/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridge.java
@@ -60,7 +60,6 @@ public final class CopyBasicMeasuresBridge extends Bridge {
     copy(squidCode, sonarResource, Metric.PUBLIC_API, CoreMetrics.PUBLIC_API);
     copy(squidCode, sonarResource, Metric.COMPLEXITY, CoreMetrics.COMPLEXITY);
     copy(squidCode, sonarResource, Metric.STATEMENTS, CoreMetrics.STATEMENTS);
-    copy(squidCode, sonarResource, Metric.COMMENTED_OUT_CODE_LINES, CoreMetrics.COMMENTED_OUT_CODE_LINES);
   }
 
   private void copy(SourceCode squidResource, Resource sonarResource, Metric squidMetric, org.sonar.api.measures.Metric sonarMetric) {
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java
index cc5c7588c4f..58c58cf9255 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/check/CommentedOutCodeLineCheckTest.java
@@ -19,6 +19,9 @@
  */
 package org.sonar.java.ast.check;
 
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.java.CheckMessages;
@@ -47,7 +50,8 @@ public class CommentedOutCodeLineCheckTest {
 
   @Test
   public void testDetection() {
-    CheckMessages checkMessages = new CheckMessages((SourceFile) squid.search("CommentedCode.java"));
+    SourceFile sourceFile = (SourceFile) squid.search("CommentedCode.java");
+    CheckMessages checkMessages = new CheckMessages(sourceFile);
 
     checkMessages.assertNext().atLine(26).withMessage("It's better to remove commented-out line of code.");
     checkMessages.assertNext().atLine(27);
@@ -66,6 +70,9 @@ public class CommentedOutCodeLineCheckTest {
     checkMessages.assertNext().atLine(69);
 
     checkMessages.assertNoMore();
+
+    assertThat(sourceFile.getInt(Metric.COMMENT_LINES), is(40)); // TODO but in fact 46, so without fake-JSNI
+    assertThat(sourceFile.getInt(Metric.COMMENT_BLANK_LINES), is(16));
   }
 
 }
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/visitor/CommentVisitorTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/visitor/CommentVisitorTest.java
index 7a430efbc9a..55c3d0cb6cf 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/visitor/CommentVisitorTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/java/ast/visitor/CommentVisitorTest.java
@@ -64,7 +64,7 @@ public class CommentVisitorTest {
   @Test
   public void testCommentedCode() {
     SourceProject res = scan("/metrics/commentedCode/CommentedCode.java");
-    assertEquals(4, res.getInt(Metric.COMMENT_LINES));
+    assertEquals(8, res.getInt(Metric.COMMENT_LINES));
     assertEquals(4, res.getInt(Metric.COMMENTED_OUT_CODE_LINES));
     assertEquals(25, res.getInt(Metric.LINES));
     assertEquals(7, res.getInt(Metric.LINES_OF_CODE));
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridgeTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridgeTest.java
new file mode 100644
index 00000000000..25ed16e993a
--- /dev/null
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/plugins/squid/bridges/CopyBasicMeasuresBridgeTest.java
@@ -0,0 +1,42 @@
+/*
+ * 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.plugins.squid.bridges;
+
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyDouble;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+
+import org.junit.Test;
+import org.sonar.api.measures.CoreMetrics;
+import org.sonar.api.resources.Resource;
+
+public class CopyBasicMeasuresBridgeTest extends BridgeTestCase {
+
+  /**
+   * SONAR-3093
+   */
+  @Test
+  public void shouldNotCopyCommentedOutCodeLines() {
+    verify(context, never()).saveMeasure(any(Resource.class), eq(CoreMetrics.COMMENTED_OUT_CODE_LINES), anyDouble());
+  }
+
+}
diff --git a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/squid/SquidUserGuideTest.java b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/squid/SquidUserGuideTest.java
index 56e0ae64bdb..79c5ab47fea 100644
--- a/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/squid/SquidUserGuideTest.java
+++ b/plugins/sonar-squid-java-plugin/src/test/java/org/sonar/squid/SquidUserGuideTest.java
@@ -77,9 +77,9 @@ public class SquidUserGuideTest {
     assertEquals(12268, project.getInt(Metric.STATEMENTS));
     assertEquals(8475, project.getInt(Metric.COMPLEXITY));
     assertEquals(4668, project.getInt(Metric.BRANCHES));
-    assertEquals(21114, project.getInt(Metric.COMMENT_LINES));
+    assertEquals(21184, project.getInt(Metric.COMMENT_LINES));
     assertEquals(9995, project.getInt(Metric.COMMENT_BLANK_LINES));
-    assertEquals(17838, project.getInt(Metric.COMMENT_LINES_WITHOUT_HEADER));
+    assertEquals(17908, project.getInt(Metric.COMMENT_LINES_WITHOUT_HEADER));
     assertEquals(0.40, project.getDouble(Metric.COMMENT_LINES_DENSITY), 0.01);
 
   }