QUA-840: fix issue reporting (#4)

Author: dantevvp

Diff for: lib/cc/engine/golangci.rb

@@ -2,54 +2,73 @@
 require 'pathname'
 require 'json'
 require 'digest/sha1'
+require 'open3'
 
 module CC
   module Engine
     class Golangci
       ALLOWED_EXTENSIONS = %w[.go]
+      ISSUE_IDENTIFIER_REGEXP = /^([^\s]+): (.*)/.freeze
 
       def initialize(engine_config)
         @engine_config = engine_config || {}
         @files_to_analyze = []
         @dirs_to_analyze = []
       end
 
-      attr_reader :engine_config, :dirs_to_analyze, :files_to_analyze
+      attr_reader :engine_config
 
       def run
-        build_paths_to_analyze
+        issues = analyze_paths.compact
 
-        run_for_paths(dirs_to_analyze)
-        run_for_paths(files_to_analyze)
+        puts output_issues(issues)
       end
 
-      def run_for_paths(paths)
-        data = IO.popen(command_env, command(paths)).read
-        begin
-          data = JSON.parse(data)
-        rescue JSON::ParserError
-          warn "Error parsing golangci-lint's output:"
-          warn data
-          exit!
+      def analyze_paths
+        # run the linter for each include_paths path
+        # we do this because the linter is very strict and loud about the paths to analyze we provide as arguments
+        # this method is noticeably slower than running the linter just once
+
+        include_paths.flat_map do |path|
+          real_path = Pathname.new(path).realpath
+
+          if real_path.directory?
+            path += "..."
+          else
+            next unless ALLOWED_EXTENSIONS.include?(real_path.extname)
+          end
+
+          issues = run_command(path)["Issues"]
+          next unless issues.is_a?(Array) && issues.length > 0
+
+          issues.map { |issue| process_issue(issue) }
         end
+      end
+
+      def run_command(path)
+        data = IO.popen(command_env, command(path)).read
+        return {} if data.nil? || data.empty?
 
-        issues = data["Issues"]
-        return unless issues.is_a?(Array) && issues.length > 0
-  
-        puts data['Issues'].map { |issue| "#{convert_issue(issue)}\0" }.join
+        JSON.parse(data)
+      rescue JSON::ParserError
+        warn "Error parsing golangci-lint's output:"
+        warn "#{data}"
+        exit!
       end
 
-      def command(paths)
-        ["/usr/local/bin/golangci-lint", "run", "--out-format", "json", *paths]
+      def command(path)
+        ["/usr/local/bin/golangci-lint", "run", "--out-format", "json", path]
       end
 
       def command_env
         { "CGO_ENABLED" => "0" }
       end
 
-      ISSUE_IDENTIFIER_REGEXP = /^([^\s]+): (.*)/.freeze
+      def output_issues(issues)
+        issues.uniq { |issue| issue[:fingerprint] }.map { |issue| issue.to_json + "\0" }.join
+      end
 
-      def convert_issue(issue)
+      def process_issue(issue)
         text = issue['Text']
         # Data coming from linters is not standardised, so it may be quite
         # complicated to extract a check_name and description from it. Here we
@@ -69,7 +88,7 @@ def convert_issue(issue)
           categories: categories_for_linter(linter_name),
           fingerprint: fingerprint_issue(issue),
           location: locate_issue(issue)
-        }.to_json
+        }
       end
 
       def categories_for_linter(linter)
@@ -104,28 +123,8 @@ def fingerprint_issue(issue)
 
       private
 
-      def build_paths_to_analyze
-        # golangci-lint surfaces errors when analyzing directories and files in the same run,
-        # so we need to split the analysis into two different runs: one for directories and one for files
-
-        include_paths = engine_config["include_paths"] || ["./"]
-
-        include_paths.each do |path|
-          begin
-            pathname = Pathname.new(path).realpath
-
-            if pathname.directory?
-              # golangci-lint allows adding ... to a directory path to analyze it recursively
-              # we want to do this for all directories
-
-              @dirs_to_analyze << (pathname + "...").to_s
-            else
-              @files_to_analyze << pathname.to_s if ALLOWED_EXTENSIONS.include?(pathname.extname)
-            end
-          rescue Errno::ENOENT
-            nil
-          end
-        end.compact
+      def include_paths
+        @include_paths ||= engine_config["include_paths"]
       end
     end
   end