fix cli and csv output

Author: markburns

lib/problematic_variable_finder/formatters/cli_formatter.rb

@@ -3,28 +3,17 @@
 module ProblematicVariableFinder
   module Formatters
     class CliFormatter
-      attr_reader :gem_name, :problems
+      attr_reader :problems
 
-      def initialize(gem_name, problems)
-        @gem_name = gem_name
+      def initialize(problems)
         @problems = problems
       end 
 
       def call
-        problems.each do |path, (full_path, file_problems)|
-          puts
-
-          file_problems.each do |problem|
-            display_problem(full_path, path, problem)
-          end
+        problems.each do |problem|
+          DisplayCliProblem.new(problem).call
         end
       end
-
-      private
-
-      def display_problem(full_path, path, problem)
-        DisplayCliProblem.new(gem_name, full_path, path, problem).call
-      end
     end
   end
 end

lib/problematic_variable_finder/formatters/csv.rb

@@ -6,33 +6,39 @@ module ProblematicVariableFinder
   module Formatters
     class Csv < CliFormatter
       def call
-        write_mode = File.exist?(csv_filename) ? "a" : "w"
-
         CSV.open(csv_filename, "#{write_mode}b") do |csv|
-          csv << [
-            'github_link',
-            'gem_name',
-            'gem_version',
-            'location',
-            'type',
-            'code'
-          ]
-
-          problems.each do |gem_with_version, gem_problems|
-            puts
-
-            gem_problems.each do |problem|
-              csv << display_problem(problem)
-            end
+          unless exists?
+            csv << [
+              'github_link',
+              'gem_name',
+              'gem_version',
+              'out_of_date',
+              'location',
+              'type',
+              'code'
+            ]
+          end
+
+          problems.compact.each do |problem|
+            csv << display_problem(problem)
           end
 
           csv
         end
       end
 
-
       private
 
+      def write_mode
+        exists? ? "a" : "w"
+      end
+
+      def exists?
+        return @exists if defined?(@exists)
+
+        @exists = File.exist?(csv_filename)
+      end
+
       def csv_filename
         "problematic_variables.csv"
       end

lib/problematic_variable_finder/formatters/display_cli_problem.rb

@@ -1,12 +1,9 @@
 module ProblematicVariableFinder
   module Formatters
     class DisplayCliProblem
-      attr_reader :gem_name, :full_path, :path, :problem
+      attr_reader :problem
 
-      def initialize(gem_name, full_path, path, problem)
-        @gem_name = gem_name
-        @full_path = full_path
-        @path = path
+      def initialize(problem)
         @problem = problem
       end
 
@@ -56,7 +53,7 @@ def snippet
           name.to_s
         when Parser::AST::Node, ProblematicVariableFinder::Parsing::SexpWrapper
           if name.respond_to?(:loc)
-          file_contents[name.loc.expression.begin_pos..name.loc.expression.end_pos]
+            file_contents[name.loc.expression.begin_pos..name.loc.expression.end_pos]
           else
             name.to_s
           end
@@ -65,9 +62,9 @@ def snippet
 
       def print_problem_header
         puts '-----------------'
-        puts "#{gem_name}"
-        puts "#{path}:#{line_number}"
-        puts "#{problem[:type]}: #{problem[:name]}"
+        puts "#{problem.gem_name}"
+        puts "#{problem.filename}:#{problem.line_number}"
+        puts "#{problem.type}: #{problem.code}"
         puts
       end
     end

lib/problematic_variable_finder/formatters/display_csv_problem.rb

@@ -11,7 +11,7 @@ def call
           problem.gem_name,
           problem.gem_version,
           problem.out_of_date,
-          "#{problem.path}:#{problem.line_number}",
+          "#{problem.filename}:#{problem.line_number}",
           problem.type,
           problem.code
         ]

lib/problematic_variable_finder/gem_problems.rb

@@ -15,41 +15,26 @@ def problems
     end
 
     def determine_problems
-      problems = {}
-
-      gems.each do |name, version|
+      gems.flat_map do |name, version|
         next if ignore_gem?(name)
         next if exclude_because_of_only_list?(name)
 
-        key = "#{name}-#{version}"
+        key = "#{name}-#{version}-cache-bust-1"
 
         gem_problems = cache(key) do
           find_gem_problems(name, version)
         end
 
-        gem_problems = objectify(gem_problems)
-
-        problems[key] = gem_problems if gem_problems.any?
+        objectify(name, version, gem_problems)
       end
-
-      puts problems
-
-      problems
     end
 
-    def objectify(gem_problems)
-      gem_problems.flat_map do |filename, file_problems|
-        file_problems.map do |problem|
-          Problem.new(
-            gem_name: name,
-            gem_version: version,
-            type: problem[:type],
-            filename: filename,
-            line_number: problem[:line_number],
-            code: problem[:name].to_s,
-            out_of_date: outdated_gems.include?(name)
-          )
-        end
+    def objectify(name, version, gem_problems)
+      gem_problems.map do |problem|
+        problem.gem_name = name
+        problem.gem_version = version
+        problem.out_of_date = outdated_gems.include?(name)
+        problem
       end
     end
 
@@ -74,19 +59,23 @@ def ignore_list
       @ignore_list ||= ProblematicVariableFinder.read_file(File.expand_path('DEFAULT_IGNORED_GEMS', __dir__)).split("\n").map(&:strip)
     end
 
-    def outdated_gems 
+    def outdated_gems
+      byebug
       @outdated_gems ||= outdated.map{|o| o.gsub(/\s+\*\s+/, '').split(" ").first }
     end
 
     def outdated
-      @outdated ||= cache('BUNDLE_OUT_OF_DATE_INFO') do
-        `bundle outdated`.split("\n").grep(/ \*/).reject do |s|
-          s['development'] ||
-            s['test']
+      @outdated ||= cache("BUNDLE_OUT_OF_DATE_INFO_#{gemfile_lock_sha}") do
+        `bundle outdated --group default --group production`.split("\n").map do |s|
+          s.split(" ").map(&:strip).first
         end
       end
     end
 
+    def gemfile_lock_sha
+      Digest::SHA1.hexdigest(ProblematicVariableFinder.read_file('Gemfile.lock'))
+    end
+
     def find_gem_problems(name, version)
       directory = "#{gem_path}/#{name}-#{version}/"
       folder = gem_path + '/' + [name, version].join('-') + '/'

lib/problematic_variable_finder/problem.rb

@@ -3,7 +3,7 @@ class Problem
     attr_accessor \
       :gem_name,
       :gem_version,
-      :file_name,
+      :filename,
       :line_number,
       :out_of_date,
       :type,
@@ -19,16 +19,15 @@ def github_link
       @github_link ||=
         begin
           if source_code_uri
-            "#{source_code_uri}/#{file_name}:#{line_number}"
+            "#{source_code_uri}/#{filename}##{line_number}"
           else
-            "#{gem_name} #{file_name}:#{line_number}"
+            "#{gem_name} #{filename}##{line_number}"
           end
         end
     end
 
     def source_code_uri
-      gem_spec&.metadata['source_code_uri'] 
-
+      gem_spec&.metadata&.fetch('source_code_uri', nil)
     end
 
     def gem_spec

lib/problematic_variable_finder/problem_finder.rb

@@ -33,33 +33,24 @@ def find_problems_in_directory(path, remove_paths=[])
         end
       end
 
-      directory_problems = {}
-
-      files.each do |f|
-        puts f
-        full_path, path, problems = find_file_problems(f, remove_paths)
-        problems.map! do |problem|
+      files.flat_map do |f|
+        _, path, problems = find_file_problems(f, remove_paths)
+        problems.map do |problem|
           Problem.new(
-            gem_name: name,
-            gem_version: version,
             type: problem[:type],
-            filename: filename,
+            filename: path,
             line_number: problem[:line_number],
-            code: problem[:name].to_s,
-            out_of_date: outdated_gems.include?(name)
+            code: problem[:name].to_s
           )
         end
-        directory_problems[path]  = [full_path, problems] if problems.any?
       end
-
-      directory_problems
     end
 
     def find_file_problems(f, remove_paths)
       full_path = File.expand_path f
       friendly_path = full_path
       remove_paths.each do |p|
-        friendly_path = f.gsub(p, '')
+        friendly_path = friendly_path.gsub(p, '')
       end
 
       problems = begin

lib/problematic_variable_finder/runner.rb

@@ -9,15 +9,17 @@
 require 'problematic_variable_finder/problem_finder'
 require 'problematic_variable_finder/formatters/cli_formatter'
 require 'problematic_variable_finder/formatters/csv'
+require 'problematic_variable_finder/fs_caching'
 
 module ProblematicVariableFinder
   class Runner
+    include FsCaching
     def self.call
       new.call
     end
 
     def call
-      display_problems('main app problems', main_problems)
+      display_problems(main_problems)
       display_gem_problems
 
       if gem_problems.outdated_gems.any?
@@ -29,35 +31,36 @@ def call
     end
 
     def display_gem_problems
-      each_gem_problem do |gem_name, problems, out_of_date|
-        if options[:verbose]
-          display_problems(gem_name, problems)
-        else
-          puts '-----------------'
-          puts "#{gem_name} #{out_of_date ? '(out of date)' : ''} #{problems.flatten.length} possible issues"
-        end
+      if options[:verbose]
+        display_problems(gem_problems.problems)
+      else
+        puts '-----------------'
+        puts "#{gem_name} #{out_of_date ? '(out of date)' : ''} #{problems.flatten.length} possible issues"
       end
     end
 
-    def display_problems(gem_name, problems)
+    def display_problems(problems)
       klass = case options[:format]
               when :csv
                 Formatters::Csv
               else
                 Formatters::CliFormatter
               end
 
-      klass.new(gem_name, problems).call
+      klass.new(problems).call
     end
 
-    def each_gem_problem
-      gem_problems.problems.each do |gem_name, (problems, out_of_date)|
-        yield gem_name, problems, out_of_date
+    def main_problems
+      cache("main_problems_#{sha_of_all_app_files}") do
+        app_problems + lib_problems
       end
     end
 
-    def main_problems
-      app_problems.merge(lib_problems)
+    def sha_of_all_app_files
+      app = Dir["app/**/*.rb"]
+      lib = Dir["lib/**/*.rb"]
+      contents = (app + lib).map { |f| (ProblematicVariableFinder.read_file(f)) }.join
+      Digest::SHA1.hexdigest(contents)
     end
 
     def app_problems