Merge pull request #619 from troessner/xml_report_fixes

I accidentally the whole XMLReport

lib/reek/report/report.rb

@@ -144,53 +144,46 @@ class XMLReport < Base
       require 'rexml/document'
 
       def show
-        checkstyle = REXML::Element.new('checkstyle', document)
-
-        smells.group_by(&:source).each do |file, file_smells|
-          file_to_xml(file, file_smells, checkstyle)
-        end
-
-        print_xml(checkstyle.parent)
+        document.write output: $stdout, indent: 2
+        $stdout.puts
       end
 
       private
 
       def document
-        REXML::Document.new.tap do |doc|
-          doc << REXML::XMLDecl.new
+        REXML::Document.new.tap do |document|
+          document << REXML::XMLDecl.new << checkstyle
         end
       end
 
-      def file_to_xml(file, file_smells, parent)
-        REXML::Element.new('file', parent).tap do |element|
-          element.attributes['name'] = File.realpath(file)
-          smells_to_xml(file_smells, element)
+      def checkstyle
+        REXML::Element.new('checkstyle').tap do |checkstyle|
+          smells.group_by(&:source).each do |source, source_smells|
+            checkstyle << file(source, source_smells)
+          end
         end
       end
 
-      def smells_to_xml(smells, parent)
-        smells.each do |smell|
-          smell_to_xml(smell, parent)
+      def file(name, smells)
+        REXML::Element.new('file').tap do |file|
+          file.add_attribute 'name', File.realpath(name)
+          smells.each do |smell|
+            smell.lines.each do |line|
+              file << error(smell, line)
+            end
+          end
         end
       end
 
-      def smell_to_xml(smell, parent)
-        REXML::Element.new('error', parent).tap do |element|
-          attributes = [
-            ['line', smell.lines.first],
-            ['column', 0],
-            ['severity', 'warning'],
-            ['message', smell.message],
-            ['source', smell.smell_type]
-          ]
-          element.add_attributes(attributes)
+      def error(smell, line)
+        REXML::Element.new('error').tap do |error|
+          error.add_attributes 'column'   => 0,
+                               'line'     => line,
+                               'message'  => smell.message,
+                               'severity' => 'warning',
+                               'source'   => smell.smell_type
         end
       end
-
-      def print_xml(document)
-        formatter = REXML::Formatters::Default.new
-        puts formatter.write(document, '')
-      end
     end
   end
 end

spec/reek/report/xml_report_spec.rb

@@ -1,35 +1,25 @@
-require 'pathname'
 require_relative '../../spec_helper'
 require_relative '../../../lib/reek/examiner'
 require_relative '../../../lib/reek/report/report'
-require_relative '../../../lib/reek/report/formatter'
 
 RSpec.describe Reek::Report::XMLReport do
-  let(:instance) { Reek::Report::XMLReport.new }
+  let(:xml_report) { Reek::Report::XMLReport.new }
 
   context 'empty source' do
-    let(:examiner) { Reek::Examiner.new('') }
-
-    before do
-      instance.add_examiner examiner
-    end
-
-    it 'prints empty checkstyle xml' do
-      expect { instance.show }.to output("<?xml version='1.0'?>\n<checkstyle/>\n").to_stdout
+    it 'prints empty checkstyle XML' do
+      xml_report.add_examiner Reek::Examiner.new('')
+      xml = "<?xml version='1.0'?>\n<checkstyle/>\n"
+      expect { xml_report.show }.to output(xml).to_stdout
     end
   end
 
   context 'source with voliations' do
-    let(:examiner) { Reek::Examiner.new('def simple(a) a[0] end') }
-
-    before do
-      allow(File).to receive(:realpath).and_return('/some/path')
-      instance.add_examiner examiner
-    end
-
-    it 'prints non-empty checkstyle xml' do
-      sample_path = SAMPLES_PATH.join('checkstyle.xml')
-      expect { instance.show }.to output(sample_path.read).to_stdout
+    it 'prints non-empty checkstyle XML' do
+      path = SAMPLES_PATH.join('two_smelly_files/dirty_one.rb')
+      xml_report.add_examiner Reek::Examiner.new(path)
+      xml = SAMPLES_PATH.join('checkstyle.xml').read
+      xml = xml.gsub(path.to_s, path.expand_path.to_s)
+      expect { xml_report.show }.to output(xml).to_stdout
     end
   end
 end

spec/samples/checkstyle.xml

@@ -1,2 +1,13 @@
 <?xml version='1.0'?>
-<checkstyle><file name='/some/path'><error column='0' line='1' message='doesn&apos;t depend on instance state' severity='warning' source='UtilityFunction'/><error column='0' line='1' message='has the parameter name &apos;a&apos;' severity='warning' source='UncommunicativeParameterName'/></file></checkstyle>
+<checkstyle>
+  <file name='spec/samples/two_smelly_files/dirty_one.rb'>
+    <error column='0' line='5' message='has the variable name &apos;@s&apos;' severity='warning' source='UncommunicativeVariableName'/>
+    <error column='0' line='4' message='calls @s.title 2 times' severity='warning' source='DuplicateMethodCall'/>
+    <error column='0' line='6' message='calls @s.title 2 times' severity='warning' source='DuplicateMethodCall'/>
+    <error column='0' line='4' message='calls puts(@s.title) 2 times' severity='warning' source='DuplicateMethodCall'/>
+    <error column='0' line='6' message='calls puts(@s.title) 2 times' severity='warning' source='DuplicateMethodCall'/>
+    <error column='0' line='5' message='contains iterators nested 2 deep' severity='warning' source='NestedIterators'/>
+    <error column='0' line='3' message='has the name &apos;a&apos;' severity='warning' source='UncommunicativeMethodName'/>
+    <error column='0' line='5' message='has the variable name &apos;x&apos;' severity='warning' source='UncommunicativeVariableName'/>
+  </file>
+</checkstyle>