diff --git a/Library/Homebrew/cask/cmd/style.rb b/Library/Homebrew/cask/cmd/style.rb index 8355ba9eca2b175cb06629ed9df081edcf4c98c1..6f7a368d57ffb471191dbec9e452800043e4d0a9 100644 --- a/Library/Homebrew/cask/cmd/style.rb +++ b/Library/Homebrew/cask/cmd/style.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "json" + module Cask class Cmd class Style < AbstractCommand @@ -7,26 +9,50 @@ module Cask "checks Cask style using RuboCop" end - option "--fix", :fix, false + def self.rubocop(*paths, auto_correct: false, debug: false, json: false) + Homebrew.install_bundler_gems! - def run - install_rubocop cache_env = { "XDG_CACHE_HOME" => "#{HOMEBREW_CACHE}/style" } - hide_warnings = debug? ? [] : [ENV["HOMEBREW_RUBY_PATH"], "-W0", "-S"] - Dir.mktmpdir do |tmpdir| - system(cache_env, *hide_warnings, "rubocop", *rubocop_args, "--", *cask_paths, chdir: tmpdir) - end - raise CaskError, "style check failed" unless $CHILD_STATUS.success? - end + hide_warnings = debug ? [] : [ENV["HOMEBREW_RUBY_PATH"], "-W0", "-S"] + + args = [ + "--force-exclusion", + "--config", "#{HOMEBREW_LIBRARY}/.rubocop_cask.yml" + ] - def install_rubocop - capture_stderr do - begin - Homebrew.install_bundler_gems! - rescue SystemExit - raise CaskError, Tty.strip_ansi($stderr.string).chomp.sub(/\AError: /, "") + if json + args << "--format" << "json" + else + if auto_correct + args << "--auto-correct" + else + args << "--debug" if debug + args << "--parallel" end + + args << "--format" << "simple" + args << "--color" if Tty.color? + end + + executable, *args = [*hide_warnings, "rubocop", *args, "--", *paths] + + result = Dir.mktmpdir do |tmpdir| + system_command executable, args: args, chdir: tmpdir, env: cache_env, + print_stdout: !json, print_stderr: !json end + + result.assert_success! unless (0..1).cover?(result.exit_status) + + return JSON.parse(result.stdout) if json + + result + end + + option "--fix", :fix, false + + def run + result = self.class.rubocop(*cask_paths, auto_correct: fix?, debug: debug?) + raise CaskError, "Style check failed." unless result.status.success? end def cask_paths @@ -39,32 +65,12 @@ module Cask end end - def rubocop_args - fix? ? autocorrect_args : normal_args - end - - def default_args - [ - "--force-exclusion", - "--config", "#{HOMEBREW_LIBRARY}/.rubocop_cask.yml", - "--format", "simple" - ] - end - def test_cask_paths [ Pathname.new("#{HOMEBREW_LIBRARY}/Homebrew/test/support/fixtures/cask/Casks"), Pathname.new("#{HOMEBREW_LIBRARY}/Homebrew/test/support/fixtures/third-party/Casks"), ] end - - def normal_args - default_args + ["--parallel"] - end - - def autocorrect_args - default_args + ["--auto-correct"] - end end end end diff --git a/Library/Homebrew/cmd/style.rb b/Library/Homebrew/cmd/style.rb index 632ca9c1813277f75fdaf8f7dd30184b56643e3f..77c9a0d67f11d920dc702f12916f71d3fe4d4fdb 100644 --- a/Library/Homebrew/cmd/style.rb +++ b/Library/Homebrew/cmd/style.rb @@ -62,6 +62,6 @@ module Homebrew NewFormulaAudit] end - Homebrew.failed = Style.check_style_and_print(target, options) + Homebrew.failed = !Style.check_style_and_print(target, options) end end diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index 9af74e5389ff53ca203b0389a4478f6f0b4d14fc..9c2ced48c814d229616dd1c6100909a2b1b145de 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -95,7 +95,7 @@ module Homebrew odie "--only-cops/--except-cops and --strict/--only cannot be used simultaneously!" end - options = { fix: args.fix?, realpath: true } + options = { fix: args.fix? } if only_cops options[:only_cops] = only_cops diff --git a/Library/Homebrew/style.rb b/Library/Homebrew/style.rb index a0b97c00b7fb703e78403e6c722d2bdb338b9cae..0cb0d59806760538f5d0991f7ef8257961f9238a 100644 --- a/Library/Homebrew/style.rb +++ b/Library/Homebrew/style.rb @@ -6,19 +6,17 @@ module Homebrew # Checks style for a list of files, printing simple RuboCop output. # Returns true if violations were found, false otherwise. - def check_style_and_print(files, options = {}) - check_style_impl(files, :print, options) + def check_style_and_print(files, **options) + check_style_impl(files, :print, **options) end # Checks style for a list of files, returning results as a RubocopResults # object parsed from its JSON output. - def check_style_json(files, options = {}) - check_style_impl(files, :json, options) + def check_style_json(files, **options) + check_style_impl(files, :json, **options) end - def check_style_impl(files, output_type, options = {}) - fix = options[:fix] - + def check_style_impl(files, output_type, fix: false, except_cops: nil, only_cops: nil) Homebrew.install_bundler_gems! require "rubocop" require "rubocops" @@ -34,22 +32,22 @@ module Homebrew args += ["--extra-details", "--display-cop-names"] if ARGV.verbose? - if options[:except_cops] - options[:except_cops].map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop.to_s, "") } - cops_to_exclude = options[:except_cops].select do |cop| + if except_cops + except_cops.map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop.to_s, "") } + cops_to_exclude = except_cops.select do |cop| RuboCop::Cop::Cop.registry.names.include?(cop) || RuboCop::Cop::Cop.registry.departments.include?(cop.to_sym) end args << "--except" << cops_to_exclude.join(",") unless cops_to_exclude.empty? - elsif options[:only_cops] - options[:only_cops].map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop.to_s, "") } - cops_to_include = options[:only_cops].select do |cop| + elsif only_cops + only_cops.map! { |cop| RuboCop::Cop::Cop.registry.qualified_cop_name(cop.to_s, "") } + cops_to_include = only_cops.select do |cop| RuboCop::Cop::Cop.registry.names.include?(cop) || RuboCop::Cop::Cop.registry.departments.include?(cop.to_sym) end - odie "RuboCops #{options[:only_cops].join(",")} were not found" if cops_to_include.empty? + odie "RuboCops #{only_cops.join(",")} were not found" if cops_to_include.empty? args << "--only" << cops_to_include.join(",") end @@ -102,7 +100,7 @@ module Homebrew raise "Invalid output_type for check_style_impl: #{output_type}" end - return !rubocop_success if files.present? || !has_non_formula + return rubocop_success if files.present? || !has_non_formula shellcheck = which("shellcheck") shellcheck ||= which("shellcheck", ENV["HOMEBREW_PATH"]) @@ -113,7 +111,7 @@ module Homebrew end unless shellcheck opoo "Could not find or install `shellcheck`! Not checking shell style." - return !rubocop_success + return rubocop_success end shell_files = [ @@ -125,7 +123,7 @@ module Homebrew # TODO: check, fix completions here too. # TODO: consider using ShellCheck JSON output shellcheck_success = system shellcheck, "--shell=bash", *shell_files - !rubocop_success || !shellcheck_success + rubocop_success && shellcheck_success end class RubocopResults @@ -168,8 +166,8 @@ module Homebrew "[Corrected] " if corrected? end - def to_s(options = {}) - if options[:display_cop_name] + def to_s(display_cop_name: false) + if display_cop_name "#{severity_code}: #{location.to_short_s}: #{cop_name}: " \ "#{Tty.green}#{correction_status}#{Tty.reset}#{message}" else diff --git a/Library/Homebrew/system_command.rb b/Library/Homebrew/system_command.rb index 5129720c5e70836e6461483f829dc4bd48c2940d..ec99b832bd51bcabed1b6cba57174002971c9fa5 100644 --- a/Library/Homebrew/system_command.rb +++ b/Library/Homebrew/system_command.rb @@ -49,7 +49,8 @@ class SystemCommand end end - assert_success if must_succeed? + result = Result.new(command, @output, @status, secrets: @secrets) + result.assert_success! if must_succeed? result end @@ -105,12 +106,6 @@ class SystemCommand ["/usr/bin/sudo", *askpass_flags, "-E", "--"] end - def assert_success - return if @status.success? - - raise ErrorDuringExecution.new(command, status: @status, output: @output, secrets: @secrets) - end - def expanded_args @expanded_args ||= args.map do |arg| if arg.respond_to?(:to_path) @@ -166,18 +161,21 @@ class SystemCommand sources.each(&:close_read) end - def result - Result.new(command, @output, @status) - end - class Result attr_accessor :command, :status, :exit_status - def initialize(command, output, status) + def initialize(command, output, status, secrets:) @command = command @output = output @status = status @exit_status = status.exitstatus + @secrets = secrets + end + + def assert_success! + return if @status.success? + + raise ErrorDuringExecution.new(command, status: @status, output: @output, secrets: @secrets) end def stdout diff --git a/Library/Homebrew/test/cask/cmd/style_spec.rb b/Library/Homebrew/test/cask/cmd/style_spec.rb index a55b89fde9ba6604f815dc8653f74f4645471c0e..e3e5d50407f8e174a1fcc2b2e87cb694c62cb4a5 100644 --- a/Library/Homebrew/test/cask/cmd/style_spec.rb +++ b/Library/Homebrew/test/cask/cmd/style_spec.rb @@ -11,58 +11,6 @@ describe Cask::Cmd::Style, :cask do it_behaves_like "a command that handles invalid options" - describe "#run" do - subject { cli.run } - - before do - allow(cli).to receive_messages(install_rubocop: nil, - system: nil, - rubocop_args: nil, - cask_paths: nil) - allow($CHILD_STATUS).to receive(:success?).and_return(success) - end - - context "when rubocop succeeds" do - let(:success) { true } - - it "does not raise an error" do - expect { subject }.not_to raise_error - end - end - - context "when rubocop fails" do - let(:success) { false } - - it "raises an error" do - expect { subject }.to raise_error(Cask::CaskError) - end - end - end - - describe "#install_rubocop" do - subject { cli.install_rubocop } - - context "when installation succeeds" do - before do - allow(Homebrew).to receive(:install_bundler_gems!) - end - - it "exits successfully" do - expect { subject }.not_to raise_error - end - end - - context "when installation fails" do - before do - allow(Homebrew).to receive(:install_bundler_gems!).and_raise(SystemExit) - end - - it "raises an error" do - expect { subject }.to raise_error(Cask::CaskError) - end - end - end - describe "#cask_paths" do subject { cli.cask_paths } @@ -117,41 +65,4 @@ describe Cask::Cmd::Style, :cask do end end end - - describe "#rubocop_args" do - subject { cli.rubocop_args } - - before do - allow(cli).to receive(:fix?).and_return(fix) - end - - context "when fix? is true" do - let(:fix) { true } - - it { is_expected.to include("--auto-correct") } - end - - context "when fix? is false" do - let(:fix) { false } - - it { is_expected.not_to include("--auto-correct") } - end - end - - describe "#default_args" do - subject { cli.default_args } - - it { is_expected.to include("--format", "simple") } - end - - describe "#autocorrect_args" do - subject { cli.autocorrect_args } - - let(:default_args) { ["--format", "simple"] } - - it "adds --auto-correct to default args" do - allow(cli).to receive(:default_args).and_return(default_args) - expect(subject).to include("--auto-correct", *default_args) - end - end end diff --git a/Library/Homebrew/test/cask/pkg_spec.rb b/Library/Homebrew/test/cask/pkg_spec.rb index 3772ea094043355a7c35cd4b6bb566fe2b0c2e31..486ab8f822715239650709851485897d1809bf0e 100644 --- a/Library/Homebrew/test/cask/pkg_spec.rb +++ b/Library/Homebrew/test/cask/pkg_spec.rb @@ -152,7 +152,8 @@ describe Cask::Pkg, :cask do "/usr/sbin/pkgutil", args: ["--pkg-info-plist", pkg_id], ).and_return( - SystemCommand::Result.new(nil, [[:stdout, pkg_info_plist]], instance_double(Process::Status, exitstatus: 0)), + SystemCommand::Result.new(nil, [[:stdout, pkg_info_plist]], instance_double(Process::Status, exitstatus: 0), + secrets: []), ) info = pkg.info diff --git a/Library/Homebrew/test/cmd/style_spec.rb b/Library/Homebrew/test/cmd/style_spec.rb index 7e428913454a2bdea831965e4ffc6e017f040388..0dcb681702fe17169544b4cf3712d7bc17b0dce2 100644 --- a/Library/Homebrew/test/cmd/style_spec.rb +++ b/Library/Homebrew/test/cmd/style_spec.rb @@ -62,8 +62,10 @@ describe "brew style" do end end EOS - options = { fix: true, only_cops: ["NewFormulaAudit/DependencyOrder"], realpath: true } - rubocop_result = Homebrew::Style.check_style_json([formula], options) + rubocop_result = Homebrew::Style.check_style_json( + [formula], + fix: true, only_cops: ["NewFormulaAudit/DependencyOrder"], + ) offense_string = rubocop_result.file_offenses(formula.realpath).first.to_s expect(offense_string).to match(/\[Corrected\]/) end @@ -79,7 +81,7 @@ describe "brew style" do rubocop_result = Homebrew::Style.check_style_and_print([target_file]) - expect(rubocop_result).to eq false + expect(rubocop_result).to eq true end end end diff --git a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb index 2f767ae9c9d539de580ea022c460abfee558565f..253e9c8e8696180c84dc6ac63b32485881a60628 100644 --- a/Library/Homebrew/test/support/helper/cask/fake_system_command.rb +++ b/Library/Homebrew/test/support/helper/cask/fake_system_command.rb @@ -55,7 +55,7 @@ class FakeSystemCommand if response.respond_to?(:call) response.call(command_string, options) else - SystemCommand::Result.new(command, [[:stdout, response]], OpenStruct.new(exitstatus: 0)) + SystemCommand::Result.new(command, [[:stdout, response]], OpenStruct.new(exitstatus: 0), secrets: []) end end diff --git a/Library/Homebrew/test/system_command_result_spec.rb b/Library/Homebrew/test/system_command_result_spec.rb index c76cb5d6fa4c871c1bb2e9145e835f0ac6e60a1d..1d53f5e7b6f9be7f16e0c2035cdbe3e5a22dc1e4 100644 --- a/Library/Homebrew/test/system_command_result_spec.rb +++ b/Library/Homebrew/test/system_command_result_spec.rb @@ -4,7 +4,8 @@ require "system_command" describe SystemCommand::Result do subject(:result) { - described_class.new([], output_array, instance_double(Process::Status, exitstatus: 0, success?: true)) + described_class.new([], output_array, instance_double(Process::Status, exitstatus: 0, success?: true), + secrets: []) } let(:output_array) { diff --git a/Library/Homebrew/utils/gems.rb b/Library/Homebrew/utils/gems.rb index 2e86e4f51f4bcd43ede1d528092abcf2d6f6e1cb..61b3f7f1f31a57b62f76d50d967a844773d7cd98 100644 --- a/Library/Homebrew/utils/gems.rb +++ b/Library/Homebrew/utils/gems.rb @@ -92,7 +92,7 @@ module Homebrew @bundle_installed ||= begin bundle = "#{gem_user_bindir}/bundle" bundle_check_output = `#{bundle} check 2>&1` - bundle_check_failed = !$CHILD_STATUS.exitstatus.zero? + bundle_check_failed = !$CHILD_STATUS.success? # for some reason sometimes the exit code lies so check the output too. if bundle_check_failed || bundle_check_output.include?("Install missing gems") diff --git a/Library/Homebrew/utils/tty.rb b/Library/Homebrew/utils/tty.rb index 76e2867dd9c3446635c014b994ebb79c998a21f5..3f69e508f50e17a8765e5ee98135c2a3b12c460b 100644 --- a/Library/Homebrew/utils/tty.rb +++ b/Library/Homebrew/utils/tty.rb @@ -64,10 +64,17 @@ module Tty end def to_s - return "" if !ENV["HOMEBREW_COLOR"] && (ENV["HOMEBREW_NO_COLOR"] || !$stdout.tty?) + return "" unless color? current_escape_sequence ensure reset_escape_sequence! end + + def color? + return false if ENV["HOMEBREW_NO_COLOR"] + return true if ENV["HOMEBREW_COLOR"] + + $stdout.tty? + end end