Add clarifying comments to rubocop:disables

- Needed for PR 18842 that adds a `DisableComment` RuboCop to ensure that all RuboCop disables have comments.
This commit is contained in:
Issy Long 2025-01-12 16:56:48 +00:00
parent 1e91082d67
commit 6ada9a9665
No known key found for this signature in database
27 changed files with 44 additions and 5 deletions

View File

@ -200,6 +200,7 @@ rescue RuntimeError, SystemCallError => e
end end
exit 1 exit 1
# Catch any other types of exceptions.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
onoe e onoe e

View File

@ -230,6 +230,8 @@ begin
options = Options.create(args.flags_only) options = Options.create(args.flags_only)
build = Build.new(formula, options, args:) build = Build.new(formula, options, args:)
build.install build.install
# Any exception means the build did not complete.
# The `case` for what to do per-exception class is further down.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
error_hash = JSON.parse e.to_json error_hash = JSON.parse e.to_json

View File

@ -63,6 +63,7 @@ module Cask
MacOSRequirement.new([T.must(md[:version]).to_sym], comparator: md[:comparator]) MacOSRequirement.new([T.must(md[:version]).to_sym], comparator: md[:comparator])
elsif (md = /^\s*(?<comparator><|>|[=<>]=)\s*(?<version>\S+)\s*$/.match(first_arg)) elsif (md = /^\s*(?<comparator><|>|[=<>]=)\s*(?<version>\S+)\s*$/.match(first_arg))
MacOSRequirement.new([md[:version]], comparator: md[:comparator]) MacOSRequirement.new([md[:version]], comparator: md[:comparator])
# This is not duplicate of the first case: see `args.first` and a different comparator.
else # rubocop:disable Lint/DuplicateBranch else # rubocop:disable Lint/DuplicateBranch
MacOSRequirement.new([args.first], comparator: "==") MacOSRequirement.new([args.first], comparator: "==")
end end

View File

@ -626,6 +626,7 @@ class Reporter
unless Formulary.factory(new_full_name).keg_only? unless Formulary.factory(new_full_name).keg_only?
system HOMEBREW_BREW_FILE, "link", new_full_name, "--overwrite" system HOMEBREW_BREW_FILE, "link", new_full_name, "--overwrite"
end end
# Rescue any possible exception types.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
if Homebrew::EnvConfig.developer? if Homebrew::EnvConfig.developer?
require "utils/backtrace" require "utils/backtrace"

View File

@ -99,6 +99,7 @@ module Homebrew
exec(*exec_args) exec(*exec_args)
end end
end end
# Rescue any possible exception types.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
retry if retry_test?(f) retry if retry_test?(f)

View File

@ -19,7 +19,7 @@ module Homebrew
sig { params(downloadable: Downloadable, force: T::Boolean).returns(Concurrent::Promises::Future) } sig { params(downloadable: Downloadable, force: T::Boolean).returns(Concurrent::Promises::Future) }
def enqueue(downloadable, force: false) def enqueue(downloadable, force: false)
quiet = pool.max_length > 1 quiet = pool.max_length > 1
# Passing in arguments from outside into the future is a common `concurrent-ruby` pattern. # Passing in arguments from outside into the future is a common `concurrent-ruby` pattern.
# rubocop:disable Lint/ShadowingOuterLocalVariable # rubocop:disable Lint/ShadowingOuterLocalVariable
Concurrent::Promises.future_on(pool, downloadable, force, quiet) do |downloadable, force, quiet| Concurrent::Promises.future_on(pool, downloadable, force, quiet) do |downloadable, force, quiet|
downloadable.clear_cache if force downloadable.clear_cache if force

View File

@ -1,6 +1,6 @@
# typed: strict # typed: strict
# This is a third-party implementation # This is a third-party implementation.
# rubocop:disable Lint/StructNewOverride # rubocop:disable Lint/StructNewOverride
class Mechanize::HTTP class Mechanize::HTTP
ContentDisposition = Struct.new :type, :filename, :creation_date, ContentDisposition = Struct.new :type, :filename, :creation_date,
@ -8,6 +8,7 @@ class Mechanize::HTTP
end end
# rubocop:enable Lint/StructNewOverride # rubocop:enable Lint/StructNewOverride
# This is a third-party implementation.
# rubocop:disable Style/OptionalBooleanParameter # rubocop:disable Style/OptionalBooleanParameter
class Mechanize::HTTP::ContentDispositionParser class Mechanize::HTTP::ContentDispositionParser
sig { sig {

View File

@ -31,6 +31,7 @@ module OS
raise "Missing URL" if cask.url.nil? raise "Missing URL" if cask.url.nil?
rescue Interrupt rescue Interrupt
raise raise
# Handle all possible exceptions reading Casks.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
os_and_arch = "macOS #{current_macos_version} on #{arch}" os_and_arch = "macOS #{current_macos_version} on #{arch}"
onoe "Invalid cask (#{os_and_arch}): #{file}" onoe "Invalid cask (#{os_and_arch}): #{file}"

View File

@ -2903,6 +2903,7 @@ class Formula
test test
end end
end end
# Handle all possible exceptions running formula tests.
rescue Exception # rubocop:disable Lint/RescueException rescue Exception # rubocop:disable Lint/RescueException
staging.retain! if debug? staging.retain! if debug?
raise raise

View File

@ -484,8 +484,8 @@ on_request: installed_on_request?, options:)
if pour_bottle? if pour_bottle?
begin begin
pour pour
# Catch any other types of exceptions as they leave us with nothing installed.
rescue Exception # rubocop:disable Lint/RescueException rescue Exception # rubocop:disable Lint/RescueException
# any exceptions must leave us with nothing installed
ignore_interrupts do ignore_interrupts do
begin begin
FileUtils.rm_r(formula.prefix) if formula.prefix.directory? FileUtils.rm_r(formula.prefix) if formula.prefix.directory?
@ -819,6 +819,7 @@ on_request: installed_on_request?, options:)
oh1 "Installing #{formula.full_name} dependency: #{Formatter.identifier(dep.name)}" oh1 "Installing #{formula.full_name} dependency: #{Formatter.identifier(dep.name)}"
fi.install fi.install
fi.finish fi.finish
# Handle all possible exceptions installing deps.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
ignore_interrupts do ignore_interrupts do
tmp_keg.rename(installed_keg.to_path) if tmp_keg && !installed_keg.directory? tmp_keg.rename(installed_keg.to_path) if tmp_keg && !installed_keg.directory?
@ -1016,6 +1017,7 @@ on_request: installed_on_request?, options:)
formula.update_head_version formula.update_head_version
raise "Empty installation" if !formula.prefix.directory? || Keg.new(formula.prefix).empty_installation? raise "Empty installation" if !formula.prefix.directory? || Keg.new(formula.prefix).empty_installation?
# Handle all possible exceptions when building.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
if e.is_a? BuildError if e.is_a? BuildError
e.formula = formula e.formula = formula
@ -1093,6 +1095,7 @@ on_request: installed_on_request?, options:)
puts "You can try again using:" puts "You can try again using:"
puts " brew link #{formula.name}" puts " brew link #{formula.name}"
@show_summary_heading = true @show_summary_heading = true
# Handle all other possible exceptions when linking.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
ofail "An unexpected error occurred during the `brew link` step" ofail "An unexpected error occurred during the `brew link` step"
puts "The formula built, but is not symlinked into #{HOMEBREW_PREFIX}" puts "The formula built, but is not symlinked into #{HOMEBREW_PREFIX}"
@ -1145,6 +1148,7 @@ on_request: installed_on_request?, options:)
launchd_service_path.chmod 0644 launchd_service_path.chmod 0644
log = formula.var/"log" log = formula.var/"log"
log.mkpath if service.include? log.to_s log.mkpath if service.include? log.to_s
# Handle all possible exceptions when installing service files.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
puts e puts e
ofail "Failed to install service files" ofail "Failed to install service files"
@ -1156,6 +1160,7 @@ on_request: installed_on_request?, options:)
sig { params(keg: Keg).void } sig { params(keg: Keg).void }
def fix_dynamic_linkage(keg) def fix_dynamic_linkage(keg)
keg.fix_dynamic_linkage keg.fix_dynamic_linkage
# Rescue all possible exceptions when fixing linkage.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
ofail "Failed to fix install linkage" ofail "Failed to fix install linkage"
puts "The formula built, but you may encounter issues using it or linking other" puts "The formula built, but you may encounter issues using it or linking other"
@ -1171,6 +1176,7 @@ on_request: installed_on_request?, options:)
def clean def clean
ohai "Cleaning" if verbose? ohai "Cleaning" if verbose?
Cleaner.new(formula).clean Cleaner.new(formula).clean
# Handle all possible exceptions when cleaning does not complete.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
opoo "The cleaning step did not complete successfully" opoo "The cleaning step did not complete successfully"
puts "Still, the installation was successful, so we will link it into your prefix." puts "Still, the installation was successful, so we will link it into your prefix."
@ -1243,6 +1249,7 @@ on_request: installed_on_request?, options:)
exec(*args) exec(*args)
end end
end end
# Handle all possible exceptions when postinstall does not complete.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
opoo "The post-install step did not complete successfully" opoo "The post-install step did not complete successfully"
puts "You can try again using:" puts "You can try again using:"

View File

@ -25,6 +25,7 @@ module Ignorable
def raise(*) def raise(*)
callcc do |continuation| callcc do |continuation|
super super
# Handle all possible exceptions.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
unless e.is_a?(ScriptError) unless e.is_a?(ScriptError)
e.extend(ExceptionMixin) e.extend(ExceptionMixin)

View File

@ -223,6 +223,7 @@ class Migrator
EOS EOS
rescue Interrupt rescue Interrupt
ignore_interrupts { backup_oldname } ignore_interrupts { backup_oldname }
# Any exception means the migration did not complete.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
onoe "The migration did not complete successfully." onoe "The migration did not complete successfully."
puts e puts e
@ -357,6 +358,7 @@ class Migrator
puts puts
puts "You can try again using:" puts "You can try again using:"
puts " brew link #{formula.name}" puts " brew link #{formula.name}"
# Any exception means the `brew link` step did not complete.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
onoe "An unexpected error occurred during linking" onoe "An unexpected error occurred during linking"
puts e puts e

View File

@ -27,6 +27,7 @@ begin
formula.extend(Debrew::Formula) formula.extend(Debrew::Formula)
end end
formula.run_post_install formula.run_post_install
# Handle all possible exceptions.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
error_pipe.puts e.to_json error_pipe.puts e.to_json
error_pipe.close error_pipe.close

View File

@ -74,6 +74,7 @@ module Readall
end end
rescue Interrupt rescue Interrupt
raise raise
# Handle all possible exceptions reading formulae.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
onoe "Invalid formula (#{bottle_tag}): #{file}" onoe "Invalid formula (#{bottle_tag}): #{file}"
$stderr.puts e $stderr.puts e

View File

@ -70,6 +70,7 @@ module Homebrew
fi.finish fi.finish
rescue FormulaInstallationAlreadyAttemptedError rescue FormulaInstallationAlreadyAttemptedError
nil nil
# Any other exceptions we want to restore the previous keg and report the error.
rescue Exception # rubocop:disable Lint/RescueException rescue Exception # rubocop:disable Lint/RescueException
ignore_interrupts { restore_backup(keg, link_keg, verbose:) } ignore_interrupts { restore_backup(keg, link_keg, verbose:) }
raise raise

View File

@ -91,7 +91,8 @@ module Homebrew
def audit_checksum def audit_checksum
return if spec_name == :head return if spec_name == :head
# rubocop:disable Style/InvertibleUnlessCondition (non-invertible) # This condition is non-invertible.
# rubocop:disable Style/InvertibleUnlessCondition
return unless DownloadStrategyDetector.detect(url, using) <= CurlDownloadStrategy return unless DownloadStrategyDetector.detect(url, using) <= CurlDownloadStrategy
# rubocop:enable Style/InvertibleUnlessCondition # rubocop:enable Style/InvertibleUnlessCondition

View File

@ -1061,6 +1061,7 @@ class Tap
cache[:all] ||= begin cache[:all] ||= begin
core_taps = [ core_taps = [
CoreTap.instance, CoreTap.instance,
# The conditional is valid here because we only want the cask tap on macOS.
(CoreCaskTap.instance if OS.mac?), # rubocop:disable Homebrew/MoveToExtendOS (CoreCaskTap.instance if OS.mac?), # rubocop:disable Homebrew/MoveToExtendOS
].compact ].compact

View File

@ -53,6 +53,7 @@ begin
timeout = ENV["HOMEBREW_TEST_TIMEOUT_SECS"]&.to_i || DEFAULT_TEST_TIMEOUT_SECONDS timeout = ENV["HOMEBREW_TEST_TIMEOUT_SECS"]&.to_i || DEFAULT_TEST_TIMEOUT_SECONDS
Timeout.timeout(timeout, &run_test) Timeout.timeout(timeout, &run_test)
end end
# Any exceptions during the test run are reported.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
error_pipe.puts e.to_json error_pipe.puts e.to_json
error_pipe.close error_pipe.close

View File

@ -2,7 +2,8 @@
require "commands" require "commands"
RSpec.shared_context "custom internal commands" do # rubocop:disable RSpec/ContextWording # These shared contexts starting with `when` don't make sense.
RSpec.shared_context "in custom internal commands" do # rubocop:disable RSpec/ContextWording
let(:cmds) do let(:cmds) do
[ [
# internal commands # internal commands

View File

@ -22,6 +22,7 @@ RSpec.describe MacOSVersion do
specify "comparison with Symbol" do specify "comparison with Symbol" do
expect(version).to be > :high_sierra expect(version).to be > :high_sierra
expect(version).to eq :mojave expect(version).to eq :mojave
# We're explicitly testing the `===` operator results here.
expect(version).to be === :mojave # rubocop:disable Style/CaseEquality expect(version).to be === :mojave # rubocop:disable Style/CaseEquality
expect(version).to be < :catalina expect(version).to be < :catalina
end end
@ -34,6 +35,7 @@ RSpec.describe MacOSVersion do
specify "comparison with String" do specify "comparison with String" do
expect(version).to be > "10.3" expect(version).to be > "10.3"
expect(version).to eq "10.14" expect(version).to eq "10.14"
# We're explicitly testing the `===` operator results here.
expect(version).to be === "10.14" # rubocop:disable Style/CaseEquality expect(version).to be === "10.14" # rubocop:disable Style/CaseEquality
expect(version).to be < "10.15" expect(version).to be < "10.15"
end end
@ -41,6 +43,7 @@ RSpec.describe MacOSVersion do
specify "comparison with Version" do specify "comparison with Version" do
expect(version).to be > Version.new("10.3") expect(version).to be > Version.new("10.3")
expect(version).to eq Version.new("10.14") expect(version).to eq Version.new("10.14")
# We're explicitly testing the `===` operator results here.
expect(version).to be === Version.new("10.14") # rubocop:disable Style/CaseEquality expect(version).to be === Version.new("10.14") # rubocop:disable Style/CaseEquality
expect(version).to be < Version.new("10.15") expect(version).to be < Version.new("10.15")
end end

View File

@ -67,6 +67,7 @@ RSpec.describe RuboCop::Cop::FormulaAudit::Patches do
expect_offense_hash(message: <<~EOS.chomp, severity: :convention, line: 5, column: 4, source:) expect_offense_hash(message: <<~EOS.chomp, severity: :convention, line: 5, column: 4, source:)
FormulaAudit/Patches: Patches from Debian should be https://, not http: #{patch_url} FormulaAudit/Patches: Patches from Debian should be https://, not http: #{patch_url}
EOS EOS
# GitHub patch diff regexps can't be any shorter.
# rubocop:disable Layout/LineLength # rubocop:disable Layout/LineLength
elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}) elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)})
# rubocop:enable Layout/LineLength # rubocop:enable Layout/LineLength
@ -232,6 +233,7 @@ RSpec.describe RuboCop::Cop::FormulaAudit::Patches do
expect_offense_hash(message: <<~EOS.chomp, severity: :convention, line: 5, column: 8, source:) expect_offense_hash(message: <<~EOS.chomp, severity: :convention, line: 5, column: 8, source:)
FormulaAudit/Patches: GitLab patches should end with .diff, not .patch: #{patch_url} FormulaAudit/Patches: GitLab patches should end with .diff, not .patch: #{patch_url}
EOS EOS
# GitHub patch diff regexps can't be any shorter.
# rubocop:disable Layout/LineLength # rubocop:disable Layout/LineLength
elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)}) elsif patch_url.match?(%r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)})
# rubocop:enable Layout/LineLength # rubocop:enable Layout/LineLength

View File

@ -31,6 +31,7 @@ module Cask
end end
end end
# These shared contexts starting with `when` don't make sense.
RSpec.shared_context "Homebrew Cask", :needs_macos do # rubocop:disable RSpec/ContextWording RSpec.shared_context "Homebrew Cask", :needs_macos do # rubocop:disable RSpec/ContextWording
around do |example| around do |example|
third_party_tap = Tap.fetch("third-party", "tap") third_party_tap = Tap.fetch("third-party", "tap")

View File

@ -6,6 +6,7 @@ require "formula_installer"
RSpec::Matchers.define_negated_matcher :be_a_failure, :be_a_success RSpec::Matchers.define_negated_matcher :be_a_failure, :be_a_success
# These shared contexts starting with `when` don't make sense.
RSpec.shared_context "integration test" do # rubocop:disable RSpec/ContextWording RSpec.shared_context "integration test" do # rubocop:disable RSpec/ContextWording
extend RSpec::Matchers::DSL extend RSpec::Matchers::DSL

View File

@ -33,6 +33,7 @@ RSpec.describe Utils::Inreplace do
it "raises error if there is nothing to replace in block form" do it "raises error if there is nothing to replace in block form" do
expect do expect do
described_class.inreplace(file.path) do |s| described_class.inreplace(file.path) do |s|
# Using `gsub!` here is what we want, and it's only a test.
s.gsub!("d", "f") # rubocop:disable Performance/StringReplacement s.gsub!("d", "f") # rubocop:disable Performance/StringReplacement
end end
end.to raise_error(Utils::Inreplace::Error) end.to raise_error(Utils::Inreplace::Error)

View File

@ -75,6 +75,7 @@ RSpec.describe SPDX do
license_expression = { any_of: [ license_expression = { any_of: [
"MIT", "MIT",
:public_domain, :public_domain,
# The final array item is legitimately a hash in the case of license expressions.
all_of: ["0BSD", "Zlib"], # rubocop:disable Style/HashAsLastArrayItem all_of: ["0BSD", "Zlib"], # rubocop:disable Style/HashAsLastArrayItem
"curl" => { with: "LLVM-exception" }, "curl" => { with: "LLVM-exception" },
] } ] }
@ -195,6 +196,7 @@ RSpec.describe SPDX do
license_expression = { any_of: [ license_expression = { any_of: [
"MIT", "MIT",
:public_domain, :public_domain,
# The final array item is legitimately a hash in the case of license expressions.
all_of: ["0BSD", "Zlib"], # rubocop:disable Style/HashAsLastArrayItem all_of: ["0BSD", "Zlib"], # rubocop:disable Style/HashAsLastArrayItem
"curl" => { with: "LLVM-exception" }, "curl" => { with: "LLVM-exception" },
] } ] }
@ -235,6 +237,7 @@ RSpec.describe SPDX do
}) })
end end
# The final array item is legitimately a hash in the case of license expressions.
# rubocop:disable Style/HashAsLastArrayItem # rubocop:disable Style/HashAsLastArrayItem
it "handles nested brackets" do it "handles nested brackets" do
expect(described_class.string_to_license_expression("A AND (B OR (C AND D))")).to eq({ expect(described_class.string_to_license_expression("A AND (B OR (C AND D))")).to eq({

View File

@ -269,6 +269,7 @@ RSpec.describe StringInreplaceExtension do
let(:string) { "foo" } let(:string) { "foo" }
it "replaces all occurrences" do it "replaces all occurrences" do
# Using `gsub!` here is what we want, and it's only a test.
string_extension.gsub!("o", "e") # rubocop:disable Performance/StringReplacement string_extension.gsub!("o", "e") # rubocop:disable Performance/StringReplacement
expect(string_extension.inreplace_string).to eq("fee") expect(string_extension.inreplace_string).to eq("fee")
end end

View File

@ -52,6 +52,7 @@ module Utils
Process::UID.change_privilege(Process.euid) if Process.euid != Process.uid Process::UID.change_privilege(Process.euid) if Process.euid != Process.uid
yield(error_pipe) yield(error_pipe)
# This could be any type of exception, so rescue them all.
rescue Exception => e # rubocop:disable Lint/RescueException rescue Exception => e # rubocop:disable Lint/RescueException
error_hash = JSON.parse e.to_json error_hash = JSON.parse e.to_json