From 734a651f930294f2e303fb2a566b9e1605cb4803 Mon Sep 17 00:00:00 2001 From: Issy Long Date: Fri, 3 Mar 2023 22:13:41 +0000 Subject: [PATCH] rubocop: Enable `Layout/MultilineMethodCallIndentation` & fix offenses - Part of trying to reduce the number of `Excludes:` we have in our RuboCop configs. - The fixes here all seemed reasonable, with some minimal tweaks for line length and less floatiness. Apart from `test/dev-cmd/bottle_spec.rb` where RuboCop wanted to do some ridiculously floaty indentation and there wasn't an obvious alternative place to break the lines, so I opted for in-line disables instead. --- Library/Homebrew/.rubocop.yml | 5 - Library/Homebrew/test/cask/cask_spec.rb | 7 +- .../Homebrew/test/cask/cmd/install_spec.rb | 9 +- .../Homebrew/test/cask/cmd/uninstall_spec.rb | 4 +- .../Homebrew/test/compiler_selector_spec.rb | 12 +- Library/Homebrew/test/dev-cmd/bottle_spec.rb | 9 ++ .../curl_github_packages_spec.rb | 26 ++-- .../download_strategies/curl_post_spec.rb | 26 ++-- .../test/download_strategies/curl_spec.rb | 112 ++++++++++-------- Library/Homebrew/test/resource_spec.rb | 3 +- .../Homebrew/test/utils/autoremove_spec.rb | 8 +- Library/Homebrew/test/utils/user_spec.rb | 3 +- 12 files changed, 125 insertions(+), 99 deletions(-) diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index e8a25dfd6a..046352fee4 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -10,11 +10,6 @@ Homebrew/MoveToExtendOS: - "{extend,test,requirements}/**/*" - "os.rb" -# make rspec formatting more flexible -Layout/MultilineMethodCallIndentation: - Exclude: - - "**/*_spec.rb" - Naming/PredicateName: AllowedMethods: - is_32_bit? diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index eb90f5090c..9427ee8eaf 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -232,10 +232,9 @@ describe Cask::Cask, :cask do context "when loaded from json file" do it "returns expected hash" do expect(Homebrew::API::Cask).not_to receive(:fetch_source) - hash = Cask::CaskLoader::FromAPILoader - .new("everything", from_json: JSON.parse(expected_json)) - .load(config: nil) - .to_h + hash = Cask::CaskLoader::FromAPILoader.new( + "everything", from_json: JSON.parse(expected_json) + ).load(config: nil).to_h expect(hash).to be_a(Hash) expect(JSON.pretty_generate(hash)).to eq(expected_json) diff --git a/Library/Homebrew/test/cask/cmd/install_spec.rb b/Library/Homebrew/test/cask/cmd/install_spec.rb index 2d9a866dbe..4efab6e2cb 100644 --- a/Library/Homebrew/test/cask/cmd/install_spec.rb +++ b/Library/Homebrew/test/cask/cmd/install_spec.rb @@ -28,7 +28,8 @@ describe Cask::Cmd::Install, :cask do it "recognizes the --appdir flag" do appdir = mktmpdir - expect(Cask::CaskLoader).to receive(:load).with("local-caffeine", any_args) + expect(Cask::CaskLoader).to receive(:load) + .with("local-caffeine", any_args) .and_wrap_original { |f, *args| caffeine = f.call(*args) expect(caffeine.config.appdir).to eq appdir @@ -41,7 +42,8 @@ describe Cask::Cmd::Install, :cask do it "recognizes the --appdir flag from HOMEBREW_CASK_OPTS" do appdir = mktmpdir - expect(Cask::CaskLoader).to receive(:load).with("local-caffeine", any_args) + expect(Cask::CaskLoader).to receive(:load) + .with("local-caffeine", any_args) .and_wrap_original { |f, *args| caffeine = f.call(*args) expect(caffeine.config.appdir).to eq appdir @@ -57,7 +59,8 @@ describe Cask::Cmd::Install, :cask do global_appdir = mktmpdir appdir = mktmpdir - expect(Cask::CaskLoader).to receive(:load).with("local-caffeine", any_args) + expect(Cask::CaskLoader).to receive(:load) + .with("local-caffeine", any_args) .and_wrap_original { |f, *args| caffeine = f.call(*args) expect(caffeine.config.appdir).to eq appdir diff --git a/Library/Homebrew/test/cask/cmd/uninstall_spec.rb b/Library/Homebrew/test/cask/cmd/uninstall_spec.rb index 527675b6fa..b8618cb217 100644 --- a/Library/Homebrew/test/cask/cmd/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/cmd/uninstall_spec.rb @@ -26,7 +26,7 @@ describe Cask::Cmd::Uninstall, :cask do it "shows an error when a Cask is provided that's not installed" do expect { described_class.run("local-caffeine") } - .to raise_error(Cask::CaskNotInstalledError, /is not installed/) + .to raise_error(Cask::CaskNotInstalledError, /is not installed/) end it "tries anyway on a non-present Cask when --force is given" do @@ -63,7 +63,7 @@ describe Cask::Cmd::Uninstall, :cask do cask.config.appdir.join("MyFancyApp.app").rmtree expect { described_class.run("with-uninstall-script-app") } - .to raise_error(Cask::CaskError, /uninstall script .* does not exist/) + .to raise_error(Cask::CaskError, /uninstall script .* does not exist/) expect(cask).to be_installed diff --git a/Library/Homebrew/test/compiler_selector_spec.rb b/Library/Homebrew/test/compiler_selector_spec.rb index f1b4120fb3..8862d921db 100644 --- a/Library/Homebrew/test/compiler_selector_spec.rb +++ b/Library/Homebrew/test/compiler_selector_spec.rb @@ -41,14 +41,16 @@ describe CompilerSelector do it "returns gcc-6 if gcc formula offers gcc-6 on mac", :needs_macos do software_spec.fails_with(:clang) - allow(Formulary).to receive(:factory).with("gcc") + allow(Formulary).to receive(:factory) + .with("gcc") .and_return(instance_double(Formula, version: Version.new("6.0"))) expect(selector.compiler).to eq("gcc-6") end it "returns gcc-5 if gcc formula offers gcc-5 on linux", :needs_linux do software_spec.fails_with(:clang) - allow(Formulary).to receive(:factory).with("gcc@11") + allow(Formulary).to receive(:factory) + .with("gcc@11") .and_return(instance_double(Formula, version: Version.new("5.0"))) expect(selector.compiler).to eq("gcc-5") end @@ -57,7 +59,8 @@ describe CompilerSelector do software_spec.fails_with(:clang) software_spec.fails_with(gcc: "5") software_spec.fails_with(gcc: "7") - allow(Formulary).to receive(:factory).with("gcc@11") + allow(Formulary).to receive(:factory) + .with("gcc@11") .and_return(instance_double(Formula, version: Version.new("5.0"))) expect(selector.compiler).to eq("gcc-6") end @@ -65,7 +68,8 @@ describe CompilerSelector do it "returns gcc-7 if gcc formula offers gcc-5 and fails with gcc <= 6 on linux", :needs_linux do software_spec.fails_with(:clang) software_spec.fails_with(:gcc) { version "6" } - allow(Formulary).to receive(:factory).with("gcc@11") + allow(Formulary).to receive(:factory) + .with("gcc@11") .and_return(instance_double(Formula, version: Version.new("5.0"))) expect(selector.compiler).to eq("gcc-7") end diff --git a/Library/Homebrew/test/dev-cmd/bottle_spec.rb b/Library/Homebrew/test/dev-cmd/bottle_spec.rb index 6c4f5510fc..b137086ea0 100644 --- a/Library/Homebrew/test/dev-cmd/bottle_spec.rb +++ b/Library/Homebrew/test/dev-cmd/bottle_spec.rb @@ -86,6 +86,8 @@ describe "brew bottle" do system "git", "commit", "-m", "testball 0.1" end + # RuboCop would align the `.and` with `.to_stdout` which is too floaty. + # rubocop:disable Layout/MultilineMethodCallIndentation expect { brew "bottle", "--merge", @@ -105,6 +107,7 @@ describe "brew bottle" do EOS .and not_to_output.to_stderr .and be_a_success + # rubocop:enable Layout/MultilineMethodCallIndentation expect((core_tap.path/"Formula/testball.rb").read).to eq <<~EOS class Testball < Formula @@ -153,6 +156,8 @@ describe "brew bottle" do system "git", "commit", "-m", "testball 0.1" end + # RuboCop would align the `.and` with `.to_stdout` which is too floaty. + # rubocop:disable Layout/MultilineMethodCallIndentation expect { brew "bottle", "--merge", @@ -172,6 +177,7 @@ describe "brew bottle" do EOS .and not_to_output.to_stderr .and be_a_success + # rubocop:enable Layout/MultilineMethodCallIndentation expect((core_tap.path/"Formula/testball.rb").read).to eq <<~EOS class Testball < Formula @@ -218,6 +224,8 @@ describe "brew bottle" do system "git", "commit", "-m", "testball 0.1" end + # RuboCop would align the `.and` with `.to_stdout` which is too floaty. + # rubocop:disable Layout/MultilineMethodCallIndentation expect { brew "bottle", "--merge", @@ -239,6 +247,7 @@ describe "brew bottle" do EOS .and not_to_output.to_stderr .and be_a_success + # rubocop:enable Layout/MultilineMethodCallIndentation expect((core_tap.path/"Formula/testball.rb").read).to eq <<~EOS class Testball < Formula diff --git a/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb b/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb index 7a729bd1bb..60e19b9db4 100644 --- a/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_github_packages_spec.rb @@ -20,12 +20,13 @@ describe CurlGitHubPackagesDownloadStrategy do end it "calls curl with anonymous authentication headers" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("--header", "Authorization: Bearer QQ==")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("--header", "Authorization: Bearer QQ==")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -34,12 +35,13 @@ describe CurlGitHubPackagesDownloadStrategy do let(:authorization) { "Bearer dead-beef-cafe" } it "calls curl with the provided header value" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("--header", "Authorization: #{authorization}")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("--header", "Authorization: #{authorization}")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end diff --git a/Library/Homebrew/test/download_strategies/curl_post_spec.rb b/Library/Homebrew/test/download_strategies/curl_post_spec.rb index 0d47b8c6f0..fc5ae8570b 100644 --- a/Library/Homebrew/test/download_strategies/curl_post_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_post_spec.rb @@ -29,12 +29,13 @@ describe CurlPostDownloadStrategy do } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("-d", "form=data").and(array_including_cons("-d", "is=good"))), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("-d", "form=data").and(array_including_cons("-d", "is=good"))), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -44,12 +45,13 @@ describe CurlPostDownloadStrategy do let(:specs) { { using: :post } } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("-X", "POST")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("-X", "POST")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end diff --git a/Library/Homebrew/test/download_strategies/curl_spec.rb b/Library/Homebrew/test/download_strategies/curl_spec.rb index 8cd3480d7a..5b1957325f 100644 --- a/Library/Homebrew/test/download_strategies/curl_spec.rb +++ b/Library/Homebrew/test/download_strategies/curl_spec.rb @@ -42,12 +42,13 @@ describe CurlDownloadStrategy do let(:specs) { { user_agent: "Mozilla/25.0.1" } } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("--user-agent", "Mozilla/25.0.1")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("--user-agent", "Mozilla/25.0.1")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -59,15 +60,16 @@ describe CurlDownloadStrategy do let(:specs) { { user_agent: :fake } } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons( - "--user-agent", - a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/), - )), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons( + "--user-agent", + a_string_matching(/Mozilla.*Mac OS X 10.*AppleWebKit/), + )), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -84,12 +86,13 @@ describe CurlDownloadStrategy do } it "adds the appropriate curl args and does not URL-encode the cookies" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("-b", "coo=k/e;mon=ster")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("-b", "coo=k/e;mon=ster")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -99,12 +102,13 @@ describe CurlDownloadStrategy do let(:specs) { { referer: "https://somehost/also" } } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("-e", "https://somehost/also")), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("-e", "https://somehost/also")), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -116,12 +120,15 @@ describe CurlDownloadStrategy do let(:specs) { { headers: ["foo", "bar"] } } it "adds the appropriate curl args" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("--header", "foo").and(array_including_cons("--header", "bar"))), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including( + args: array_including_cons("--header", "foo").and(array_including_cons("--header", "bar")), + ), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -132,12 +139,13 @@ describe CurlDownloadStrategy do context "with an asset hosted under example.com" do it "leaves the URL unchanged" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons(url)), - ) - .at_least(:once) - .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons(url)), + ) + .at_least(:once) + .and_return(instance_double(SystemCommand::Result, success?: true, stdout: "", assert_success!: nil)) strategy.fetch end @@ -149,12 +157,13 @@ describe CurlDownloadStrategy do let(:status) { instance_double(Process::Status, success?: true, exitstatus: 0) } it "rewrites the URL correctly" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("#{artifact_domain}/#{resource_path}")), - ) - .at_least(:once) - .and_return(SystemCommand::Result.new(["curl"], [""], status, secrets: [])) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("#{artifact_domain}/#{resource_path}")), + ) + .at_least(:once) + .and_return(SystemCommand::Result.new(["curl"], [""], status, secrets: [])) strategy.fetch end @@ -166,12 +175,13 @@ describe CurlDownloadStrategy do let(:status) { instance_double(Process::Status, success?: true, exitstatus: 0) } it "rewrites the URL correctly" do - expect(strategy).to receive(:system_command).with( - /curl/, - hash_including(args: array_including_cons("#{artifact_domain}/#{resource_path}")), - ) - .at_least(:once) - .and_return(SystemCommand::Result.new(["curl"], [""], status, secrets: [])) + expect(strategy).to receive(:system_command) + .with( + /curl/, + hash_including(args: array_including_cons("#{artifact_domain}/#{resource_path}")), + ) + .at_least(:once) + .and_return(SystemCommand::Result.new(["curl"], [""], status, secrets: [])) strategy.fetch end diff --git a/Library/Homebrew/test/resource_spec.rb b/Library/Homebrew/test/resource_spec.rb index 7d4f77d0ac..a1182c9ce1 100644 --- a/Library/Homebrew/test/resource_spec.rb +++ b/Library/Homebrew/test/resource_spec.rb @@ -191,7 +191,8 @@ describe Resource do fn = instance_double(Pathname, file?: true, basename: "foo") checksum = resource.sha256(TEST_SHA256) - expect(fn).to receive(:verify_checksum).with(checksum) + expect(fn).to receive(:verify_checksum) + .with(checksum) .and_raise(ChecksumMismatchError.new(fn, checksum, Object.new)) expect { diff --git a/Library/Homebrew/test/utils/autoremove_spec.rb b/Library/Homebrew/test/utils/autoremove_spec.rb index d7fe8fc9d6..2d04134135 100644 --- a/Library/Homebrew/test/utils/autoremove_spec.rb +++ b/Library/Homebrew/test/utils/autoremove_spec.rb @@ -62,7 +62,7 @@ describe Utils::Autoremove do it "filters out runtime dependencies" do allow(tab_from_keg).to receive(:poured_from_bottle).and_return(true) expect(described_class.send(:formulae_with_no_formula_dependents, formulae)) - .to eq([formula_with_deps, formula_is_build_dep]) + .to eq([formula_with_deps, formula_is_build_dep]) end end @@ -70,7 +70,7 @@ describe Utils::Autoremove do it "filters out runtime and build dependencies" do allow(tab_from_keg).to receive(:poured_from_bottle).and_return(false) expect(described_class.send(:formulae_with_no_formula_dependents, formulae)) - .to eq([formula_with_deps]) + .to eq([formula_with_deps]) end end end @@ -85,13 +85,13 @@ describe Utils::Autoremove do specify "installed on request" do allow(tab_from_keg).to receive(:installed_on_request).and_return(true) expect(described_class.send(:unused_formulae_with_no_formula_dependents, formulae)) - .to eq([]) + .to eq([]) end specify "not installed on request" do allow(tab_from_keg).to receive(:installed_on_request).and_return(false) expect(described_class.send(:unused_formulae_with_no_formula_dependents, formulae)) - .to match_array(formulae) + .to match_array(formulae) end end diff --git a/Library/Homebrew/test/utils/user_spec.rb b/Library/Homebrew/test/utils/user_spec.rb index 006998a652..817852292b 100644 --- a/Library/Homebrew/test/utils/user_spec.rb +++ b/Library/Homebrew/test/utils/user_spec.rb @@ -10,7 +10,8 @@ describe User do describe "#gui?" do before do - allow(SystemCommand).to receive(:run).with("who", any_args) + allow(SystemCommand).to receive(:run) + .with("who", any_args) .and_return([who_output, "", instance_double(Process::Status, success?: true)]) end