Merge branch 'pr-74' into master

Merge violations that have just changed messages #74

Author: adangel

.rubocop.yml

@@ -20,6 +20,10 @@ Metrics/MethodLength:
   Exclude:
     - 'lib/pmdtester/parsers/options.rb'
 
+Metrics/ClassLength:
+  Exclude:
+    - '**/*'
+
 Metrics/BlockLength:
   Exclude:
     - 'lib/pmdtester/parsers/options.rb'

History.md

@@ -14,6 +14,7 @@ Typeresolution is now supported by two new tags in the project-list.xml file:
 *   [#68](https://github.com/pmd/pmd-regression-tester/pull/68): Don't generate a dynamic ruleset if not needed
 *   [#69](https://github.com/pmd/pmd-regression-tester/pull/69): Detect single rules with auto-gen-config
 *   [#70](https://github.com/pmd/pmd-regression-tester/pull/70): Add link to PR on github in HTML report
+*   [#74](https://github.com/pmd/pmd-regression-tester/pull/74): Merge violations that have just changed messages
 
 ## External Contributions
 

lib/pmdtester/builders/diff_report/violations.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'differ'
+
 # Contains methods to write out html for the violations.
 # This mixin is used by DiffReportBuilder.
 module DiffReportBuilderViolations
@@ -25,7 +27,7 @@ def build_violation_table(doc, key, value)
   end
 
   def build_violation_table_head(doc)
-    build_table_head(doc, '', 'Priority', 'Rule', 'Message', 'Line')
+    build_table_head(doc, '', 'Rule', 'Message', 'Line')
   end
 
   def build_violation_table_body(doc, key, value)
@@ -36,42 +38,61 @@ def build_violation_table_body(doc, key, value)
     end
   end
 
-  def build_violation_table_row(doc, key, pmd_violation)
-    doc.tr(class: pmd_violation.branch == PmdTester::BASE ? 'b' : 'a') do
+  def build_violation_table_row(doc, key, violation)
+    doc.tr(class: get_css_class(violation)) do
       build_table_anchor_column(doc, 'A', increment_violation_index)
 
-      violation = pmd_violation.attrs
-
-      # The priority of the rule
-      doc.td violation['priority']
-
       # The rule that trigger the violation
       doc.td do
-        doc.a(href: (violation['externalInfoUrl']).to_s) { doc.text violation['rule'] }
+        doc.a(href: violation.info_url.to_s) { doc.text violation.rule_name }
       end
 
       # The violation message
-      doc.td "\n" + pmd_violation.text + "\n"
-
-      # The begin line of the violation
-      line = violation['beginline']
+      if violation.changed && violation.message != violation.old_message
+        doc.td { diff_fragments(doc, violation) }
+      else
+        doc.td violation.text
+      end
 
       # The link to the source file
       doc.td do
         link = get_link_to_source(violation, key)
-        doc.a(href: link.to_s) { doc.text line }
+        doc.a(href: link.to_s) { doc.text display_line(violation) }
       end
     end
   end
 
+  def diff_fragments(doc, violation)
+    diff = Differ.diff_by_word(violation.old_message, violation.message)
+    doc << diff.format_as(:html)
+  end
+
+  def display_line(violation)
+    if violation.changed && violation.old_line && violation.old_line != violation.line
+      "#{violation.old_line} => #{violation.line}"
+    else
+      violation.line
+    end
+  end
+
   def get_link_to_source(violation, key)
     l_str = @project.type == 'git' ? 'L' : 'l'
-    line_str = "##{l_str}#{violation['beginline']}"
+    line_str = "##{l_str}#{violation.line}"
     @project.get_webview_url(key) + line_str
   end
 
   def increment_violation_index
     @violation_index ||= 0 # init with 0
     @violation_index += 1
   end
+
+  def get_css_class(pmd_violation)
+    if pmd_violation.changed
+      'd'
+    elsif pmd_violation.branch == PmdTester::BASE
+      'b'
+    else
+      'a'
+    end
+  end
 end

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/builders/summary_report_builder.rb

@@ -127,7 +127,7 @@ def build_projects_table_head(doc)
           doc.th 'project name'
           doc.th 'project branch/tag'
           doc.th 'removed|new errors size'
-          doc.th 'removed|new violations size'
+          doc.th 'removed|changed|new violations size'
           doc.th 'removed|new configerrors size'
         end
       end
@@ -142,12 +142,12 @@ def build_projects_table_body(doc)
             end
             doc.td project.tag
             doc.td do
-              build_table_content_for(doc, project.removed_errors_size,
-                                      project.new_errors_size)
+              build_table_content_for(doc, project.removed_errors_size, project.new_errors_size)
             end
             doc.td do
               build_table_content_for(doc, project.removed_violations_size,
-                                      project.new_violations_size)
+                                      project.new_violations_size,
+                                      project.changed_violations_size)
             end
             doc.td do
               build_table_content_for(doc, project.removed_configerrors_size,

lib/pmdtester/parsers/pmd_report_document.rb

@@ -31,9 +31,9 @@ def start_element(name, attrs = [])
       when 'file'
         remove_work_dir!(attrs['name'])
         @current_violations = []
-        @current_filename = attrs['name']
+        @current_filename = attrs['name'].freeze
       when 'violation'
-        @current_violation = PmdViolation.new(attrs, @branch_name)
+        @current_violation = PmdViolation.new(attrs, @branch_name, @current_filename)
       when 'error'
         remove_work_dir!(attrs['filename'])
         remove_work_dir!(attrs['msg'])
@@ -81,7 +81,7 @@ def match_filter_set?(violation)
 
       @filter_set.each do |filter_rule_ref|
         ruleset_attr = violation.attrs['ruleset'].delete(' ').downcase + '.xml'
-        rule = violation.attrs['rule']
+        rule = violation.rule_name
         rule_ref = "#{ruleset_attr}/#{rule}"
         return true if filter_rule_ref.eql?(ruleset_attr)
         return true if filter_rule_ref.eql?(rule_ref)

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

@@ -47,22 +47,70 @@ class PmdViolation
     # </xs:complexType>
 
     attr_reader :attrs
+    attr_reader :fname
     attr_accessor :text
 
-    def initialize(attrs, branch)
+    # means it was in both branches but changed messages
+    attr_accessor :changed
+
+    def initialize(attrs, branch, fname)
       @attrs = attrs
       @branch = branch
+      @changed = false
+      @fname = fname
       @text = ''
     end
 
+    def line_move?(other)
+      message.eql?(other.message) && (line - other.line).abs <= 5
+    end
+
+    def try_merge?(other)
+      if branch != BASE && branch != other.branch && rule_name == other.rule_name &&
+         !changed && # not already changed
+         (line == other.line || line_move?(other))
+        @changed = true
+        @attrs['oldMessage'] = other.text
+        @attrs['oldLine'] = other.line
+        true
+      else
+        false
+      end
+    end
+
+    def old_message
+      @attrs['oldMessage']
+    end
+
+    def old_line
+      @attrs['oldLine']&.to_i
+    end
+
+    def info_url
+      @attrs['externalInfoUrl']
+    end
+
+    def line
+      @attrs['beginline'].to_i
+    end
+
+    def rule_name
+      @attrs['rule']
+    end
+
+    def message
+      @text
+    end
+
     def eql?(other)
-      @attrs['beginline'].eql?(other.attrs['beginline']) &&
-        @attrs['rule'].eql?(other.attrs['rule']) &&
-        @text.eql?(other.text)
+      rule_name.eql?(other.rule_name) &&
+        line.eql?(other.line) &&
+        fname.eql?(other.fname) &&
+        message.eql?(other.message)
     end
 
     def hash
-      [@attrs['beginline'], @attrs['rule'], @text].hash
+      [line, rule_name, message].hash
     end
   end
 end