[FIXED JENKINS-11376] Don't show multiple warnings for duplicated block.

jenkinsci · Aug 24, 2012 · 4ebdbbd · 4ebdbbd
1 parent 3afe0a3
commit 4ebdbbd
Show file tree

Hide file tree

Showing 11 changed files with 158 additions and 31 deletions.
diff --git a/src/main/java/hudson/plugins/dry/DryTabDetail.java b/src/main/java/hudson/plugins/dry/DryTabDetail.java
@@ -38,5 +38,10 @@ public DryTabDetail(final AbstractBuild<?, ?> owner, final DetailFactory detailF
     public String getWarnings() {
         return "dry-warnings.jelly";
     }
+
+    @Override
+    public String getDetails() {
+        return "dry-details.jelly";
+    }
 }
 
diff --git a/src/main/java/hudson/plugins/dry/parser/DuplicateCode.java b/src/main/java/hudson/plugins/dry/parser/DuplicateCode.java
@@ -43,6 +43,12 @@ public class DuplicateCode extends AbstractAnnotation {
     private final Set<DuplicateCode> links = new HashSet<DuplicateCode>();
     /** The duplicate source code fragment. */
     private String sourceCode;
+    /**
+     * Marks this duplication as derived. Derived duplications will not be shown in the user interface.
+     *
+     * @since 2.31
+     */
+    private boolean isDerived;
 
     /**
      * Creates a new instance of {@link DuplicateCode}.
@@ -64,6 +70,38 @@ public DuplicateCode(final Priority priority, final int firstLine, final int num
         setFileName(fileName);
     }
 
+    /**
+     * Creates a new instance of {@link DuplicateCode}.
+     *
+     * @param priority
+     *            the priority of the warning
+     * @param firstLine
+     *            the starting line of the duplication
+     * @param numberOfLines
+     *            total number of duplicate lines
+     * @param fileName
+     *            name of the file that contains the duplication
+     * @param isDerived
+     *            determines if this duplication is derived. Derived
+     *            duplications will not be shown in the user interface
+     */
+    public DuplicateCode(final Priority priority, final int firstLine, final int numberOfLines, final String fileName, final boolean isDerived) {
+        this(priority, firstLine, numberOfLines, fileName);
+
+        this.isDerived = isDerived;
+    }
+
+    /**
+     * Returns whether this duplication as derived. Derived duplications will
+     * not be shown in the user interface.
+     *
+     * @return <code>true</code> if this duplication as derived, false if it is
+     *         the master duplication
+     */
+    public boolean isDerived() {
+        return isDerived;
+    }
+
     @Override
     public String getType() {
         return Messages.DRY_Warning_Type();

diff --git a/src/main/java/hudson/plugins/dry/parser/cpd/CpdParser.java b/src/main/java/hudson/plugins/dry/parser/cpd/CpdParser.java
@@ -110,12 +110,14 @@ private Collection<DuplicateCode> convert(final List<Duplication> duplications,
 
         for (Duplication duplication : duplications) {
             List<DuplicateCode> codeBlocks = new ArrayList<DuplicateCode>();
+            boolean isDerived = false;
             for (SourceFile file : duplication.getFiles()) {
                 // TODO: check why PMD reports a length + 1
-                DuplicateCode annotation = new DuplicateCode(getPriority(duplication.getLines()), file.getLine(), duplication.getLines(), file.getPath());
+                DuplicateCode annotation = new DuplicateCode(getPriority(duplication.getLines()), file.getLine(), duplication.getLines(), file.getPath(), isDerived);
                 annotation.setSourceCode(duplication.getCodeFragment());
                 annotation.setModuleName(moduleName);
-                codeBlocks.add(annotation);
+                codeBlocks.add(annotation);
+                isDerived = true;
             }
             for (DuplicateCode block : codeBlocks) {
                 block.linkTo(codeBlocks);

diff --git a/src/main/java/hudson/plugins/dry/parser/simian/SimianParser.java b/src/main/java/hudson/plugins/dry/parser/simian/SimianParser.java
@@ -109,10 +109,12 @@ private Collection<DuplicateCode> convert(final List<Set> duplications, final St
 
         for (Set duplication : duplications) {
             List<DuplicateCode> codeBlocks = new ArrayList<DuplicateCode>();
+            boolean isDerived = false;
             for (Block file : duplication.getBlocks()) {
-                DuplicateCode annotation = new DuplicateCode(getPriority(duplication.getLineCount()), file.getStartLineNumber(), duplication.getLineCount(), file.getSourceFile());
+                DuplicateCode annotation = new DuplicateCode(getPriority(duplication.getLineCount()), file.getStartLineNumber(), duplication.getLineCount(), file.getSourceFile(), isDerived);
                 annotation.setModuleName(moduleName);
                 codeBlocks.add(annotation);
+                isDerived = true;
             }
             for (DuplicateCode block : codeBlocks) {
                 block.linkTo(codeBlocks);

diff --git a/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details.jelly b/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details.jelly
@@ -0,0 +1,29 @@
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
+  xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form"
+  xmlns:i="jelly:fmt" xmlns:local="local" xmlns:u="/util">
+  <st:header name="Content-Type" value="text/html;charset=UTF-8" />
+  <j:forEach var="warning" items="${annotations.sortedAnnotations}">
+    <j:if test="${!warning.isDerived()}">
+      <table class="pane" id="details">
+        <tr>
+          <td class="pane-header">
+            <u:sourceLink it="${it}" warning="${warning}" />
+            ,
+            ${warning.type}, ${%Priority}: ${warning.priority.localizedString}
+            <j:if test="${h.size2(warning.category) > 0}">
+              , ${%Category}: ${warning.category}
+            </j:if>
+          </td>
+        </tr>
+        <tr>
+          <td style="white-space: normal">
+            <p>
+              <b>${warning.message}</b>
+            </p>
+            <p>${warning.toolTip}</p>
+          </td>
+        </tr>
+      </table>
+    </j:if>
+  </j:forEach>
+</j:jelly>
diff --git a/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details_de.properties b/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details_de.properties
@@ -0,0 +1,2 @@
+Priority=Priorität
+Category=Kategorie
diff --git a/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details_ja.properties b/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-details_ja.properties
@@ -0,0 +1,2 @@
+Priority=\u91cd\u8981\u5ea6
+Category=\u30ab\u30c6\u30b4\u30ea\u30fc
diff --git a/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-warnings.jelly b/src/main/resources/hudson/plugins/dry/DryTabDetail/dry-warnings.jelly
@@ -10,43 +10,47 @@
       <td class="pane-header">${%Duplicated in}</td>
     </tr>
     <tbody>
-      <j:forEach var="warning" items="${it.container.annotations}">
-        <tr>
-          <td class="pane">
-            <j:choose>
-              <j:when test="${warning.size() &lt; 40}">
-                <div tooltip="${warning.formattedSourceCode}" nodismiss="">
-                   <u:sourceLink it="${it}" warning="${warning}"/>
-                </div>
-              </j:when>
-              <j:otherwise>
-                   <u:sourceLink it="${it}" warning="${warning}"/>
-              </j:otherwise>
-            </j:choose>
-          </td>
-          <td class="pane">
+      <j:forEach var="warning" items="${it.container.sortedAnnotations}">
+        <j:if test="${!warning.isDerived()}">
+          <tr>
+            <td class="pane">
+              <j:choose>
+                <j:when test="${warning.size() &lt; 40}">
+                  <div tooltip="${warning.formattedSourceCode}"
+                    nodismiss="">
+                    <u:sourceLink it="${it}" warning="${warning}" />
+                  </div>
+                </j:when>
+                <j:otherwise>
+                  <u:sourceLink it="${it}" warning="${warning}" />
+                </j:otherwise>
+              </j:choose>
+            </td>
+            <td class="pane">
               ${warning.numberOfLines}
-          </td>
-          <td class="pane">
-            <j:set var="linkCount" value="${0}" />
+            </td>
+            <td class="pane">
+              <j:set var="linkCount" value="${0}" />
               <j:forEach var="link" items="${warning.links}">
                 <j:choose>
                   <j:when test="${warning.size() &lt; 40}">
-                    <div tooltip="${warning.formattedSourceCode}" nodismiss="">
+                    <div tooltip="${warning.formattedSourceCode}"
+                      nodismiss="">
                       <dry:link />
                     </div>
                   </j:when>
                   <j:otherwise>
-                      <j:if test="${linkCount > 0}">
-                        <br/>
-                      </j:if>
-                      <dry:link />
+                    <j:if test="${linkCount > 0}">
+                      <br />
+                    </j:if>
+                    <dry:link />
                   </j:otherwise>
                 </j:choose>
-                    <j:set var="linkCount" value="${linkCount + 1}" />
+                <j:set var="linkCount" value="${linkCount + 1}" />
               </j:forEach>
-          </td>
-        </tr>
+            </td>
+          </tr>
+        </j:if>
       </j:forEach>
     </tbody>
   </table>

diff --git a/src/test/java/hudson/plugins/dry/parser/AbstractDuplicationParserTest.java b/src/test/java/hudson/plugins/dry/parser/AbstractDuplicationParserTest.java
@@ -0,0 +1,30 @@
+package hudson.plugins.dry.parser;
+
+import static org.junit.Assert.*;
+
+import java.util.Collection;
+
+/**
+ * Base class for duplication tests.
+ *
+ * @author Ulli Hafner
+ */
+public class AbstractDuplicationParserTest {
+    /**
+     * Verifies that the number of derived duplications is correct.
+     *
+     * @param annotations
+     *            the annotations to check
+     * @param expected
+     *            the expected number of derived duplications
+     */
+    protected void verifyDerivedDuplications(final Collection<DuplicateCode> annotations, final int expected) {
+        int derivedCount = 0;
+        for (DuplicateCode duplicateCode : annotations) {
+            if (duplicateCode.isDerived()) {
+                derivedCount++;
+            }
+        }
+        assertEquals("Wrong number of derived duplications",  expected, derivedCount);
+    }
+}
diff --git a/src/test/java/hudson/plugins/dry/parser/cpd/CpdParserTest.java b/src/test/java/hudson/plugins/dry/parser/cpd/CpdParserTest.java
@@ -1,6 +1,7 @@
 package hudson.plugins.dry.parser.cpd;
 
 import static org.junit.Assert.*;
+import hudson.plugins.dry.parser.AbstractDuplicationParserTest;
 import hudson.plugins.dry.parser.DuplicateCode;
 
 import java.io.InputStream;
@@ -15,7 +16,7 @@
 /**
  *  Tests the extraction of PMD's CPD analysis results.
  */
-public class CpdParserTest {
+public class CpdParserTest extends AbstractDuplicationParserTest {
     /** First line in publisher. */
     private static final int PUBLISHER_LINE = 69;
     /** First line in reporter. */
@@ -92,6 +93,7 @@ public void scanFileWithTwoDuplications() throws InvocationTargetException {
         Collection<DuplicateCode> annotations = parseFile(fileName);
 
         assertEquals(ERROR_MESSAGE, 4, annotations.size());
+        verifyDerivedDuplications(annotations, 2);
     }
 
     /**
@@ -112,6 +114,8 @@ public void scanFileWithOneDuplication() throws InvocationTargetException {
         DuplicateCode first = iterator.next();
         DuplicateCode second = iterator.next();
 
+        verifyDerivedDuplications(annotations, 1);
+
         if (first.getPrimaryLineNumber() == REPORTER_LINE) {
             assertDuplication(REPORTER_LINE, REPORTER, first);
             assertDuplication(PUBLISHER_LINE, PUBLISHER, second);

diff --git a/src/test/java/hudson/plugins/dry/parser/simian/SimianParserTest.java b/src/test/java/hudson/plugins/dry/parser/simian/SimianParserTest.java
@@ -1,6 +1,7 @@
 package hudson.plugins.dry.parser.simian;
 
 import static org.junit.Assert.*;
+import hudson.plugins.dry.parser.AbstractDuplicationParserTest;
 import hudson.plugins.dry.parser.DuplicateCode;
 
 import java.io.InputStream;
@@ -13,7 +14,7 @@
 /**
  *  Tests the extraction of PMD's CPD analysis results.
  */
-public class SimianParserTest {
+public class SimianParserTest extends AbstractDuplicationParserTest {
     private static final String MATRIX_RUN = "c:/java/hudson/matrix/MatrixRun.java";
     private static final String MAVEN_BUILD = "c:/java/hudson/maven/MavenBuild.java";
 
@@ -71,6 +72,8 @@ public void scanFileWithOneDuplicationInOneFile() throws InvocationTargetExcepti
         Iterator<DuplicateCode> iterator = annotations.iterator();
         assertDuplication(93, 98, MAVEN_BUILD, iterator.next());
         assertDuplication(76, 81, MAVEN_BUILD, iterator.next());
+
+        verifyDerivedDuplications(annotations, 1);
     }
 
     /**
@@ -90,6 +93,8 @@ public void scanFileWithOneDuplicationInTwoFiles() throws InvocationTargetExcept
         Iterator<DuplicateCode> iterator = annotations.iterator();
         assertDuplication(92, 97, MAVEN_BUILD, iterator.next());
         assertDuplication(61, 66, MATRIX_RUN, iterator.next());
+
+        verifyDerivedDuplications(annotations, 1);
     }
 
     /**
@@ -124,6 +129,8 @@ public void scanFileWithTwoDuplications() throws InvocationTargetException {
         warning = iterator.next();
         assertDuplication(76, 81, MAVEN_BUILD, warning);
         assertEquals("Wrong other file", MAVEN_BUILD, getFileName(warning));
+
+        verifyDerivedDuplications(annotations, 2);
     }
 
     private String getFileName(final DuplicateCode warning) {
@@ -149,6 +156,8 @@ public void scanFileWithOneDuplicationInFourFiles() throws InvocationTargetExcep
         assertDuplication(21, 26, "c:/java/foo2.java", iterator.next());
         assertDuplication(31, 36, "c:/java/foo3.java", iterator.next());
         assertDuplication(41, 46, "c:/java/foo4.java", iterator.next());
+
+        verifyDerivedDuplications(annotations, 3);
     }
 
     /**