Try to cleanup the logic to handle XML attrs

Author: oowekyala

lib/pmdtester/parsers/pmd_report_document.rb

@@ -35,14 +35,27 @@ def start_element(name, attrs = [])
         @current_filename = remove_work_dir!(attrs['name'])
         @current_violations = []
       when 'violation'
-        @current_violation = PmdViolation.new(attrs, @branch_name, @current_filename)
+        @current_violation = PmdViolation.new(
+          branch: @branch_name,
+          fname: @current_filename,
+          info_url: attrs['externalInfoUrl'],
+          bline: attrs['beginline'].to_i,
+          rule_name: attrs['rule'],
+          ruleset_name: attrs['ruleset'].freeze
+        )
       when 'error'
-        remove_work_dir!(attrs['msg'])
         @current_filename = remove_work_dir!(attrs['filename'])
-        @current_error = PmdError.new(attrs, @branch_name, @current_filename)
+
+        @current_error = PmdError.new(
+          branch: @branch_name,
+          filename: @current_filename,
+          short_message: remove_work_dir!(attrs['msg'])
+        )
       end
     end
 
+    # Modifies the string in place and returns it
+    # (this is what sub! does, except it returns nil if no replacement occurred)
     def remove_work_dir!(str)
       str.sub!(/#{@working_dir}/, '')
       str

lib/pmdtester/pmd_error.rb

@@ -21,11 +21,11 @@ class PmdError
     attr_accessor :old_error
     attr_reader :filename, :short_message
 
-    def initialize(attrs, branch, filename)
+    def initialize(branch:, filename:, short_message:)
       @branch = branch
       @stack_trace = ''
       @changed = false
-      @short_message = attrs['msg']
+      @short_message = short_message
       @filename = filename
     end
 

lib/pmdtester/pmd_violation.rb

@@ -30,21 +30,24 @@ class PmdViolation
     attr_reader :fname, :info_url, :line, :old_line, :old_message, :rule_name, :ruleset_name
     attr_accessor :message
 
-    def initialize(attrs, branch, fname)
+    # rubocop:disable Metrics/ParameterLists
+    # Disable it: how is replacing a long parameter list with a single hash helping?
+    def initialize(branch:, fname:, info_url:, bline:, rule_name:, ruleset_name:)
       @branch = branch
       @fname = fname
       @message = ''
 
-      @info_url = attrs['externalInfoUrl']
-      @line = attrs['beginline'].to_i
-      @rule_name = attrs['rule']
+      @info_url = info_url
+      @line = bline
+      @rule_name = rule_name
 
-      @ruleset_name = attrs['ruleset'].freeze
+      @ruleset_name = ruleset_name
 
       @changed = false
       @old_message = nil
       @old_line = nil
     end
+    # rubocop:enable Metrics/ParameterLists
 
     def line_move?(other)
       message.eql?(other.message) && (line - other.line).abs <= 5