From c0f64882f186daf78228a115c21bea440023df26 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 30 Aug 2020 19:08:24 +0200 Subject: [PATCH] Split `check_style_impl` into `run_rubocop` and `run_shellcheck`. --- Library/Homebrew/style.rb | 151 ++++++++++++++++++++++++++++---------- 1 file changed, 111 insertions(+), 40 deletions(-) diff --git a/Library/Homebrew/style.rb b/Library/Homebrew/style.rb index 36f538b158..b624f1cf63 100644 --- a/Library/Homebrew/style.rb +++ b/Library/Homebrew/style.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "shellwords" + module Homebrew # Helper module for running RuboCop. # @@ -20,8 +22,42 @@ module Homebrew end def check_style_impl(files, output_type, - fix: false, except_cops: nil, only_cops: nil, display_cop_names: false, + fix: false, + except_cops: nil, only_cops: nil, + display_cop_names: false, debug: false, verbose: false) + raise ArgumentError, "Invalid output type: #{output_type.inspect}" unless [:print, :json].include?(output_type) + + shell_files, ruby_files = + Array(files).map(&method(:Pathname)) + .partition { |f| f.realpath == HOMEBREW_BREW_FILE.realpath || f.extname == ".sh" } + + rubocop_result = if shell_files.any? && ruby_files.none? + output_type == :json ? [] : true + else + run_rubocop(ruby_files, output_type, + fix: fix, + except_cops: except_cops, only_cops: only_cops, + display_cop_names: display_cop_names, + debug: debug, verbose: verbose) + end + + shellcheck_result = if ruby_files.any? && shell_files.none? + output_type == :json ? [] : true + else + run_shellcheck(shell_files, output_type) + end + + if output_type == :json + RubocopResults.new(rubocop_result + shellcheck_result) + else + rubocop_result && shellcheck_result + end + end + + def run_rubocop(files, output_type, + fix: false, except_cops: nil, only_cops: nil, display_cop_names: false, + debug: false, verbose: false) Homebrew.install_bundler_gems! require "rubocop" require "rubocops" @@ -58,11 +94,11 @@ module Homebrew args << "--only" << cops_to_include.join(",") end - has_non_formula = Array(files).any? do |file| + has_non_formula = files.any? do |file| File.expand_path(file).start_with? HOMEBREW_LIBRARY_PATH end - if files.present? && !has_non_formula + if files.any? && !has_non_formula config = if files.first && File.exist?("#{files.first}/spec") HOMEBREW_LIBRARY/".rubocop_rspec.yml" else @@ -79,63 +115,98 @@ module Homebrew cache_env = { "XDG_CACHE_HOME" => "#{HOMEBREW_CACHE}/style" } - rubocop_success = false - case output_type when :print args << "--debug" if debug args << "--format" << "simple" if files.present? - system(cache_env, "rubocop", *args) - rubocop_success = $CHILD_STATUS.success? + + system cache_env, "rubocop", *args + $CHILD_STATUS.success? when :json - json, err, status = - Open3.capture3(cache_env, "rubocop", "--format", "json", *args) - # exit status of 1 just means violations were found; other numbers mean - # execution errors. - # exitstatus can also be nil if RuboCop process crashes, e.g. due to - # native extension problems. - # JSON needs to be at least 2 characters. - if !(0..1).cover?(status.exitstatus) || json.to_s.length < 2 - raise "Error running `rubocop --format json #{args.join " "}`\n#{err}" - end - - return RubocopResults.new(JSON.parse(json)) - else - raise "Invalid output_type for check_style_impl: #{output_type}" + result = system_command "rubocop", args: ["--format", "json", *args], env: cache_env + json = json_result!(result) + json["files"] end + end - return rubocop_success if files.present? - + def run_shellcheck(files, output_type) shellcheck = which("shellcheck") shellcheck ||= which("shellcheck", ENV["HOMEBREW_PATH"]) shellcheck ||= begin ohai "Installing `shellcheck` for shell style checks..." - system HOMEBREW_BREW_FILE, "install", "shellcheck" + safe_system HOMEBREW_BREW_FILE, "install", "shellcheck" which("shellcheck") || which("shellcheck", ENV["HOMEBREW_PATH"]) end - unless shellcheck - opoo "Could not find or install `shellcheck`! Not checking shell style." - return rubocop_success + + if files.empty? + files = [ + HOMEBREW_BREW_FILE, + # TODO: HOMEBREW_REPOSITORY/"completions/bash/brew", + *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/*.sh"), + *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/cmd/*.sh"), + *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/utils/*.sh"), + ] end - shell_files = [ - HOMEBREW_BREW_FILE, - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/*.sh"), - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/cmd/*.sh"), - *Pathname.glob("#{HOMEBREW_LIBRARY}/Homebrew/utils/*.sh"), - ].select(&:exist?) - # TODO: check, fix completions here too. - # TODO: consider using ShellCheck JSON output - shellcheck_success = system shellcheck, "--shell=bash", *shell_files - rubocop_success && shellcheck_success + args = ["--shell=bash", "--", *files] # TODO: Add `--enable=all` to check for more problems. + + case output_type + when :print + system shellcheck, "--format=tty", *args + $CHILD_STATUS.success? + when :json + result = system_command shellcheck, args: ["--format=json1", *args] + json = json_result!(result) + + # Convert to same format as RuboCop offenses. + json["comments"].group_by { |v| v["file"] } + .map do |k, v| + { + "path" => k, + "offenses" => v.map do |o| + o.delete("file") + + o["cop_name"] = "SC#{o.delete("code")}" + + level = o.delete("level") + o["severity"] = { "style" => "refactor", "info" => "convention" }.fetch(level, level) + + line = o.delete("line") + column = o.delete("column") + + o["corrected"] = false + o["correctable"] = o.delete("fix").present? + + o["location"] = { + "start_line" => line, + "start_column" => column, + "last_line" => o.delete("endLine"), + "last_column" => o.delete("endColumn"), + "line" => line, + "column" => column, + } + + o + end, + } + end + end + end + + def json_result!(result) + # An exit status of 1 just means violations were found; other numbers mean + # execution errors. + # JSON needs to be at least 2 characters. + result.assert_success! if !(0..1).cover?(result.status.exitstatus) || result.stdout.length < 2 + + JSON.parse(result.stdout) end # Result of a RuboCop run. class RubocopResults - def initialize(json) - @metadata = json["metadata"] + def initialize(files) @file_offenses = {} - json["files"].each do |f| + files.each do |f| next if f["offenses"].empty? file = File.realpath(f["path"])