Merge pull request #44 from codeclimate/will/issue-location

improve derivation of lines from Sexp

Author: wfleming

lib/cc/engine/analyzers/sexp_lines.rb

@@ -0,0 +1,28 @@
+module CC
+  module Engine
+    module Analyzers
+      class SexpLines
+        attr_reader :begin_line, :end_line
+
+        def initialize(root_sexp)
+          @root_sexp = root_sexp
+          calculate
+        end
+
+        private
+
+        attr_reader :root_sexp
+
+        def calculate
+          @begin_line = root_sexp.line
+          @end_line = root_sexp.end_line || root_sexp.line
+
+          root_sexp.deep_each do |sexp|
+            @begin_line = [@begin_line, sexp.line].min
+            @end_line = [@end_line, sexp.end_line || sexp.line].max
+          end
+        end
+      end
+    end
+  end
+end

lib/cc/engine/analyzers/violation.rb

@@ -69,25 +69,16 @@ def format_other_locations
         end
 
         def format_sexp(sexp)
+          lines = SexpLines.new(sexp)
           {
             "path": sexp.file.gsub(%r(^./), ""),
             "lines": {
-              "begin": sexp.line,
-              "end": sexp.end_line || sexp_max_line(sexp, sexp.line)
-            }
+              "begin": lines.begin_line,
+              "end": lines.end_line,
+            },
           }
         end
 
-        def sexp_max_line(sexp_tree, default)
-          max = default
-
-          sexp_tree.deep_each do |sexp|
-            max = sexp.line if sexp.line > max
-          end
-
-          max
-        end
-
         def content_body
           @_content_body ||= { "body": File.read(read_up_path) }
         end

lib/cc/engine/duplication.rb

@@ -6,6 +6,7 @@
 require 'cc/engine/analyzers/reporter'
 require 'cc/engine/analyzers/engine_config'
 require 'cc/engine/analyzers/sexp'
+require 'cc/engine/analyzers/sexp_lines'
 require 'flay'
 require 'json'
 

spec/cc/engine/analyzers/sexp_lines_spec.rb

@@ -0,0 +1,43 @@
+require "spec_helper"
+require "cc/engine/duplication"
+
+module CC::Engine::Analyzers
+  RSpec.describe SexpLines do
+    describe "violation location" do
+      it "gets appropriate locations for rescue blocks" do
+        source = <<-SOURCE
+          begin
+            foo
+          rescue SyntaxError => e
+            Jekyll.logger.warn "YAML Exception reading \#{File.join(base, name)}: \#{e.message}"
+          rescue Exception => e
+            Jekyll.logger.warn "Error reading file \#{File.join(base, name)}: \#{e.message}"
+          end
+        SOURCE
+        flay = Flay.new({
+          diff: false,
+          mass: CC::Engine::Analyzers::Ruby::Main::DEFAULT_MASS_THRESHOLD,
+          summary: false,
+          verbose: false,
+          number: true,
+          timeout: 10,
+          liberal: false,
+          fuzzy: false,
+          only: nil,
+        })
+
+        sexp = RubyParser.new.process(source, "file.rb")
+        flay.process_sexp(sexp)
+        report = flay.analyze[0]
+        sexps = flay.hashes[report.structural_hash]
+        locations = sexps.map { |sexp| SexpLines.new(sexp) }
+
+        expect(locations.count).to eq 2
+        expect(locations[0].begin_line).to eq(3)
+        expect(locations[0].end_line).to eq(7)
+        expect(locations[1].begin_line).to eq(5)
+        expect(locations[1].end_line).to eq(7)
+      end
+    end
+  end
+end