Merge changed violations

Author: oowekyala

lib/pmdtester/builders/diff_report/violations.rb

@@ -37,7 +37,15 @@ def build_violation_table_body(doc, key, value)
   end
 
   def build_violation_table_row(doc, key, pmd_violation)
-    doc.tr(class: pmd_violation.branch == PmdTester::BASE ? 'b' : 'a') do
+    klass = if pmd_violation.changed
+              'd'
+            elsif pmd_violation.branch == PmdTester::BASE
+              'b'
+            else
+              'a'
+            end
+
+    doc.tr(class: klass) do
       build_table_anchor_column(doc, 'A', increment_violation_index)
 
       violation = pmd_violation.attrs

lib/pmdtester/builders/diff_report_builder.rb

@@ -82,7 +82,8 @@ def build_violations_summary_row(doc)
                           base: @report_diff.base_violations_size,
                           patch: @report_diff.patch_violations_size,
                           removed: @report_diff.removed_violations_size,
-                          added: @report_diff.new_violations_size
+                          added: @report_diff.new_violations_size,
+                          changed: @report_diff.changed_violations_size
                         })
     end
 
@@ -127,7 +128,7 @@ def build_summary_row(doc, row_data)
         doc.td(class: 'a') { doc.text row_data[:patch] }
         doc.td(class: 'c') do
           if row_data.key?(:removed) && row_data.key?(:added)
-            build_table_content_for(doc, row_data[:removed], row_data[:added])
+            build_table_content_for(doc, row_data[:removed], row_data[:added], row_data[:changed])
           elsif row_data.key?(:diff_execution_time)
             doc.text row_data[:diff_execution_time]
           end

lib/pmdtester/builders/html_report_builder.rb

@@ -47,10 +47,14 @@ def copy_css(report_dir)
       FileUtils.copy_entry(CSS_SRC_DIR, css_dest_dir)
     end
 
-    def build_table_content_for(doc, removed_size, new_size)
-      doc.font(color: 'red') { doc.text "-#{removed_size}" }
+    def build_table_content_for(doc, removed_size, new_size, changed_size=nil)
+      doc.span(class: 'removed') { doc.text "-#{removed_size}" }
+      if changed_size
+        doc.text ' | '
+        doc.span(class: 'changed') { doc.text "~#{changed_size}" }
+      end
       doc.text ' | '
-      doc.font(color: 'green') { doc.text "+#{new_size}" }
+      doc.span(class: 'added') { doc.text "+#{new_size}" }
     end
   end
 end

lib/pmdtester/pmd_configerror.rb

@@ -52,6 +52,10 @@ def msg
       @attrs['msg']
     end
 
+    def changed
+      false
+    end
+
     def eql?(other)
       rulename.eql?(other.rulename) && msg.eql?(other.msg)
     end

lib/pmdtester/pmd_error.rb

@@ -55,6 +55,10 @@ def msg
       @attrs['msg']
     end
 
+    def changed
+      false
+    end
+
     def eql?(other)
       filename.eql?(other.filename) && msg.eql?(other.msg) &&
         @text.eql?(other.text)

lib/pmdtester/pmd_violation.rb

@@ -49,15 +49,33 @@ class PmdViolation
     attr_reader :attrs
     attr_accessor :text
 
+    # means it was in both branches but changed messages
+    attr_accessor :changed
+
     def initialize(attrs, branch)
       @attrs = attrs
       @branch = branch
+      @changed = false
       @text = ''
     end
 
-    def eql?(other)
+    def is_same_modulo_message?(other)
       @attrs['beginline'].eql?(other.attrs['beginline']) &&
-        @attrs['rule'].eql?(other.attrs['rule']) &&
+          @attrs['rule'].eql?(other.attrs['rule'])
+    end
+
+    def try_merge?(other)
+      if branch != BASE && @branch != other.branch && is_same_modulo_message?(other)
+        @changed = true
+        @attrs['oldMessage'] = other.text
+        true
+      else
+        false
+      end
+    end
+
+    def eql?(other)
+      is_same_modulo_message?(other) &&
         @text.eql?(other.text)
     end
 

lib/pmdtester/report_diff.rb

@@ -10,6 +10,7 @@ class ReportDiff
     attr_accessor :base_violations_size
     attr_accessor :patch_violations_size
     attr_accessor :new_violations_size
+    attr_accessor :changed_violations_size
     attr_accessor :removed_violations_size
     attr_accessor :violation_diffs_size
 
@@ -88,28 +89,28 @@ def diffs_exist?
     def calculate_violations(base_violations, patch_violations)
       @base_violations_size = base_violations.violations_size
       @patch_violations_size = patch_violations.violations_size
-      violation_diffs = build_diffs(base_violations.violations, patch_violations.violations)
-      @violation_diffs = violation_diffs
-      @new_violations_size, @removed_violations_size = get_diffs_size(violation_diffs)
-      @violation_diffs_size = @new_violations_size + @removed_violations_size
+
+      @violation_diffs = build_diffs(base_violations.violations, patch_violations.violations)
+      @violation_diffs = merge_changed_violations(@violation_diffs)
+
+      @new_violations_size, @changed_violations_size, @removed_violations_size = get_diffs_size(@violation_diffs)
+      @violation_diffs_size = @violation_diffs.size
     end
 
     def calculate_errors(base_errors, patch_errors)
       @base_errors_size = base_errors.errors_size
       @patch_errors_size = patch_errors.errors_size
-      error_diffs = build_diffs(base_errors.errors, patch_errors.errors)
-      @error_diffs = error_diffs
-      @new_errors_size, @removed_errors_size = get_diffs_size(error_diffs)
-      @error_diffs_size = @new_errors_size + @removed_errors_size
+      @error_diffs = build_diffs(base_errors.errors, patch_errors.errors)
+      @new_errors_size, _, @removed_errors_size = get_diffs_size(@error_diffs)
+      @error_diffs_size = @error_diffs.size
     end
 
     def calculate_configerrors(base_configerrors, patch_configerrors)
       @base_configerrors_size = base_configerrors.size
       @patch_configerrors_size = patch_configerrors.size
-      configerrors_diffs = build_diffs(base_configerrors.errors, patch_configerrors.errors)
-      @configerrors_diffs = configerrors_diffs
-      @new_configerrors_size, @removed_configerrors_size = get_diffs_size(configerrors_diffs)
-      @configerrors_diffs_size = @new_configerrors_size + @removed_configerrors_size
+      @configerrors_diffs = build_diffs(base_configerrors.errors, patch_configerrors.errors)
+      @new_configerrors_size, _, @removed_configerrors_size = get_diffs_size(@configerrors_diffs)
+      @configerrors_diffs_size = @configerrors_diffs.size
     end
 
     def calculate_details(base_info, patch_info)
@@ -130,7 +131,10 @@ def calculate_details(base_info, patch_info)
     end
 
     def build_diffs(base_hash, patch_hash)
+      # Keys are filenames
+      # Values are lists of violations/errors
       diffs = base_hash.merge(patch_hash) do |_key, base_value, patch_value|
+        # make the difference of values
         (base_value | patch_value) - (base_value & patch_value)
       end
 
@@ -139,15 +143,33 @@ def build_diffs(base_hash, patch_hash)
       end
     end
 
+    # @param diff_violations a hash { filename => list[violation]}, containing those that changed
+    def merge_changed_violations(diff_violations)
+      diff_violations.each do |fname, different|
+        diff_violations[fname] = different.dup.delete_if { |v|
+          v.branch == BASE &&
+              # try_merge will set v2.changed = true if it succeeds
+              different.any? { |v2| v2.try_merge?(v) }
+        }
+      end
+    end
+
     def get_diffs_size(diffs_hash)
       new_size = 0
+      changed_size = 0
       removed_size = 0
       diffs_hash.each_value do |value|
         value.each do |item|
-          item.branch.eql?(BASE) ? removed_size += 1 : new_size += 1
+          if item.changed
+            changed_size += 1
+          elsif item.branch.eql?(BASE)
+            removed_size += 1
+          else
+            new_size += 1
+          end
         end
       end
-      [new_size, removed_size]
+      [new_size, changed_size, removed_size]
     end
 
     def introduce_new_errors?

resources/css/maven-theme.css

@@ -94,6 +94,16 @@ table.bodyTable th, table.bodyTable td {
   font-size: 1em;
 }
 
+span.added {
+  color: green;
+}
+span.removed {
+  color: red;
+}
+span.changed {
+  color: darkorange;
+}
+
 table.bodyTable tr.a {
   background-color: #af8;
 }