From 80d75bc0d8c3fdba360e7c9163cfe55f10994325 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Sun, 2 Sep 2018 20:14:54 +0100 Subject: [PATCH] Homebrew/.rubocop: tweak rules. Adjust the rules based on the current codebase. Remove various enable, disables and default values that are unnecessary. Add more comments explaining why. Make minor changes needed to enable a few more rules. --- Library/.rubocop.yml | 200 ++++++++---------- Library/Homebrew/.rubocop.yml | 86 +++++--- Library/Homebrew/build.rb | 2 +- .../lib/hbc/artifact/abstract_flight_block.rb | 2 +- Library/Homebrew/cask/lib/hbc/audit.rb | 2 +- Library/Homebrew/cask/lib/hbc/cli.rb | 4 +- Library/Homebrew/cask/lib/hbc/cli/doctor.rb | 4 +- .../cask/lib/hbc/cli/internal_stanza.rb | 2 +- Library/Homebrew/cask/lib/hbc/download.rb | 2 +- Library/Homebrew/cask/lib/hbc/dsl.rb | 4 +- .../Homebrew/cask/lib/hbc/dsl/depends_on.rb | 2 +- Library/Homebrew/cask/lib/hbc/installer.rb | 10 +- Library/Homebrew/cask/lib/hbc/metadata.rb | 2 +- Library/Homebrew/cask/lib/hbc/staged.rb | 4 +- Library/Homebrew/cask/lib/hbc/utils.rb | 2 +- Library/Homebrew/cmd/deps.rb | 2 +- Library/Homebrew/cmd/info.rb | 4 +- Library/Homebrew/cmd/outdated.rb | 2 +- Library/Homebrew/cmd/search.rb | 2 +- Library/Homebrew/debrew.rb | 2 +- Library/Homebrew/dev-cmd/audit.rb | 8 +- Library/Homebrew/dev-cmd/linkage.rb | 2 +- Library/Homebrew/dev-cmd/pull.rb | 6 +- Library/Homebrew/diagnostic.rb | 4 +- Library/Homebrew/extend/ARGV.rb | 2 +- .../os/mac/requirements/java_requirement.rb | 4 +- Library/Homebrew/formula.rb | 10 +- Library/Homebrew/formulary.rb | 4 +- Library/Homebrew/keg.rb | 2 +- Library/Homebrew/locale.rb | 4 +- Library/Homebrew/migrator.rb | 2 +- Library/Homebrew/os/mac/architecture_list.rb | 2 +- .../Homebrew/requirements/java_requirement.rb | 2 +- Library/Homebrew/tap.rb | 2 +- .../test/rubocops/patches_cop_spec.rb | 92 ++++---- Library/Homebrew/unpack_strategy.rb | 6 +- Library/Homebrew/unpack_strategy/dmg.rb | 2 + Library/Homebrew/utils.rb | 6 +- Library/Homebrew/utils/inreplace.rb | 2 +- 39 files changed, 266 insertions(+), 236 deletions(-) diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 3560552395..8606d8974f 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -7,12 +7,15 @@ AllCops: require: ./Homebrew/rubocops.rb +# enable all formulae audits FormulaAudit: Enabled: true +# enable all formulae strict audits FormulaAuditStrict: Enabled: true +# disable all formulae strict audits by default NewFormulaAudit: Enabled: false @@ -20,18 +23,19 @@ NewFormulaAudit: Layout/AlignParameters: Enabled: false +# favour parens-less DSL-style arguments +Lint/AmbiguousOperator: + Enabled: false + +# this is a bit less "floaty" Layout/CaseIndentation: EnforcedStyle: end -Layout/EmptyLineBetweenDefs: - AllowAdjacentOneLineDefs: true - +# this is a bit less "floaty" Layout/EndAlignment: - EnforcedStyleAlignWith: variable - -Layout/IndentArray: - EnforcedStyle: special_inside_parentheses + EnforcedStyleAlignWith: start_of_line +# enforce use of <<~EOS Layout/IndentHeredoc: EnforcedStyle: squiggly @@ -39,22 +43,29 @@ Layout/IndentHeredoc: Layout/SpaceAroundOperators: Enabled: false -# favor parens-less DSL-style arguments -Lint/AmbiguousOperator: - Enabled: false +# use spaces for indentation; detect tabs +Layout/Tab: + Enabled: true -# so many of these in formulae and can't be autocorrected -Lint/AmbiguousRegexpLiteral: - Enabled: false - -# favor parens-less DSL-style arguments +# favour parens-less DSL-style arguments Lint/AmbiguousBlockAssociation: Enabled: false + +# so many of these in formulae and can't be autocorrected +# TODO: fix these as `ruby -w` complains about them. +Lint/AmbiguousRegexpLiteral: + Enabled: false + # assignment in conditions are useful sometimes +# TODO: add parentheses for these and remove Lint/AssignmentInCondition: Enabled: false +# we output how to use interpolated strings too often +Lint/InterpolationCheck: + Enabled: false + # so many of these in formulae and can't be autocorrected Lint/ParenthesesAsGroupedExpression: Enabled: false @@ -62,58 +73,64 @@ Lint/ParenthesesAsGroupedExpression: # most metrics don't make sense to apply for formulae/taps Metrics/AbcSize: Enabled: false - Metrics/BlockLength: Enabled: false - Metrics/ClassLength: Enabled: false - Metrics/CyclomaticComplexity: Enabled: false +Metrics/MethodLength: + Enabled: false +# Metrics/ModuleLength: +# Enabled: false +Metrics/PerceivedComplexity: + Enabled: false +# keyword arguments don't have the same readability problems +Metrics/ParameterLists: + CountKeywordArgs: false + +# GitHub diff UI wraps beyond 118 characters (so that's the goal) Metrics/LineLength: - # 118 is the goal as GitHub diff UI wraps beyond that Max: 189 # ignore manpage comments and long single-line strings IgnoredPatterns: ['#: ', ' url "', ' mirror "', ' plist_options :'] -Metrics/MethodLength: - Enabled: false +# dashes in filenames are typical +Naming/FileName: + Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/ -Metrics/ModuleLength: - Enabled: false +# implicitly allow EOS as we use it everywhere +Naming/HeredocDelimiterNaming: + Blacklist: + - END, EOD, EOF -Metrics/ParameterLists: - CountKeywordArgs: false - -Metrics/PerceivedComplexity: +# we have too many variables like sha256 where this harms readability +Naming/VariableNumber: Enabled: false # makes code less readable for minor performance increases Performance/Caller: Enabled: false -Style/Alias: - EnforcedStyle: prefer_alias - -Style/AsciiComments: - Enabled: false +# we're doing this already so why not +Performance/CaseWhenSplat: + Enabled: true +# enable to avoid leaking resources Style/AutoResourceCleanup: Enabled: true +# this is a little more obvious what's going on Style/BarePercentLiterals: EnforcedStyle: percent_q -Style/BlockDelimiters: - EnforcedStyle: line_count_based +# consistency helps readability and helps people who don't know Ruby +Style/CollectionMethods: + Enabled: true -Style/ClassAndModuleChildren: - EnforcedStyle: nested - -# our current conditional style is established, clear and -# requiring users to change that now would be confusing. +# our current conditional style is established +# TODO: enable this when possible Style/ConditionalAssignment: Enabled: false @@ -121,32 +138,17 @@ Style/ConditionalAssignment: Style/Documentation: Enabled: false +# we don't need UTF-8 encoding comments Style/Encoding: Enabled: true -# disabled until we get the Metrics/LineLength down to 80. -Style/IfUnlessModifier: - Enabled: false - -# messes with existing plist/caveats style -Style/TrailingBodyOnMethodDefinition: - Enabled: false - -# use spaces for indentation; detect tabs -Layout/Tab: - Enabled: true - -# We have no use for using `warn` because we are -# calling Ruby with warnings disabled ourselves. -Style/StderrPuts: - Enabled: false - -# dashes in filenames are typical -Naming/FileName: - Regex: !ruby/regexp /^[\w\@\-\+\.]+(\.rb)?$/ - # falsely flags e.g. curl formatting arguments as format strings Style/FormatStringToken: + EnforcedStyle: template + +# we want to add this slowly and manually +# TODO: add to more files +Style/FrozenStringLiteralComment: Enabled: false # so many of these in formulae and can't be autocorrected @@ -162,33 +164,31 @@ Style/HashSyntax: - '**/lib/**/*' - '**/spec/**/*' +# this doesn't make sense for wide lines below maximum line length +# https://github.com/rubocop-hq/rubocop/issues/6149 +Style/IfUnlessModifier: + Enabled: false + # only for numbers >= 1_000_000 Style/NumericLiterals: MinDigits: 7 -# zero-prefixed octal literals are just too widely used (and mostly understood) +# zero-prefixed octal literals are just too widely used (and understood) Style/NumericLiteralPrefix: EnforcedOctalStyle: zero_only -# consistency and readability when faced with string interpolation -Style/PercentLiteralDelimiters: - PreferredDelimiters: - '%': '()' - '%i': '()' - '%q': '()' - '%Q': '()' - '%r': '{}' - '%s': '()' - '%w': '[]' - '%W': '[]' - '%x': '()' +# rescuing StandardError is an understood default +Style/RescueStandardError: + EnforcedStyle: implicit -Style/RaiseArgs: - EnforcedStyle: exploded +# return nil is unnecessary and a common mistake believing it's required +Style/ReturnNil: + Enabled: true -# paths abound, easy escape -Style/RegexpLiteral: - EnforcedStyle: slashes +# We have no use for using `warn` because we are +# calling Ruby with warnings disabled ourselves (for now). +Style/StderrPuts: + Enabled: false # ruby style guide favorite Style/StringLiterals: @@ -198,46 +198,30 @@ Style/StringLiterals: Style/StringLiteralsInInterpolation: EnforcedStyle: double_quotes +# consistency helps readability and helps people who don't know Ruby +Style/StringMethods: + Enabled: true + +# less confusing to non-Rubyists Style/SymbolArray: EnforcedStyle: brackets +# make things a bit easier to read Style/TernaryParentheses: EnforcedStyle: require_parentheses_when_complex -# makes diffs nicer +# messes with existing plist/caveats style +Style/TrailingBodyOnMethodDefinition: + Enabled: false + +# all trailing commas make diffs nicer +Style/TrailingCommaInArguments: + EnforcedStyleForMultiline: comma Style/TrailingCommaInArrayLiteral: EnforcedStyleForMultiline: comma - Style/TrailingCommaInHashLiteral: EnforcedStyleForMultiline: comma -Style/TrailingCommaInArguments: - EnforcedStyleForMultiline: comma - -# we have too many variables like sha256 where this harms readability -Naming/VariableNumber: - Enabled: false - -# doesn't make sense for Homebrew/brew but does for taps -Naming/UncommunicativeMethodParamName: - Enabled: true - +# a bit confusing to non-Rubyists but useful for longer arrays Style/WordArray: MinSize: 4 - -# we want to add this slowly and manually -Style/FrozenStringLiteralComment: - Enabled: false - -# generally rescuing StandardError is fine -Style/RescueStandardError: - Enabled: false - -# implicitly allow EOS as we use it everywhere -Naming/HeredocDelimiterNaming: - Blacklist: - - END, EOD, EOF - -# we output how to use interpolated strings too often -Lint/InterpolationCheck: - Enabled: false diff --git a/Library/Homebrew/.rubocop.yml b/Library/Homebrew/.rubocop.yml index 05f4df0a8f..da003e380b 100644 --- a/Library/Homebrew/.rubocop.yml +++ b/Library/Homebrew/.rubocop.yml @@ -10,14 +10,11 @@ AllCops: - '**/Casks/**/*' - '**/vendor/**/*' +# make rspec formatting more flexible Layout/MultilineMethodCallIndentation: Exclude: - '**/*_spec.rb' -# Gets false positives with our heredocs nested inside arrays -Layout/ClosingHeredocIndentation: - Enabled: false - # so many of these in formulae but none in here Lint/AmbiguousRegexpLiteral: Enabled: true @@ -31,70 +28,101 @@ Lint/NestedMethodDefinition: Lint/ParenthesesAsGroupedExpression: Enabled: true +# unused keyword arguments improve APIs Lint/UnusedMethodArgument: AllowUnusedKeywordArguments: true # TODO: try to bring down all metrics maximums Metrics/AbcSize: - Max: 250 - + Enabled: true + Max: 275 Metrics/BlockLength: - Max: 1250 - + Enabled: true + Max: 1100 Metrics/BlockNesting: + Enabled: true Max: 5 - Metrics/ClassLength: - Max: 1226 - + Enabled: true + Max: 1400 Metrics/CyclomaticComplexity: + Enabled: true Max: 75 +Metrics/MethodLength: + Enabled: true + Max: 300 +Metrics/ModuleLength: + Enabled: true + Max: 500 +Metrics/PerceivedComplexity: + Enabled: true + Max: 100 +# GitHub diff UI wraps beyond 118 characters Metrics/LineLength: - # 118 is the goal as GitHub diff UI wraps beyond that Max: 118 # ignore manpage comments IgnoredPatterns: ['#: '] -Metrics/MethodLength: - Max: 195 - -Metrics/ModuleLength: - Max: 222 - -Metrics/PerceivedComplexity: - Max: 100 - -# we won't change backward compatible method names -Naming/MethodName: - Exclude: - - 'compat/**/*' - # we won't change backward compatible predicate names +# TODO: deprecate whitelisted names and move to compat Naming/PredicateName: Exclude: - 'compat/**/*' NameWhitelist: is_32_bit?, is_64_bit? -# f meaning formulae is pretty standard +# whitelist those that are standard +# TODO: try to remove some of these Naming/UncommunicativeMethodParamName: - Enabled: false + AllowedNames: + - '_' + - 'a' + - 'b' + - 'cc' + - 'c1' + - 'c2' + - 'd' + - 'e' + - 'f' + - 'ff' + - 'fn' + - 'id' + - 'io' + - 'o' + - 'p' + - 'pr' + - 'r' + - 'rb' + - 's' + - 'to' + - 'v' # Avoid false positives on modifiers used on symbols of methods # See https://github.com/rubocop-hq/rubocop/issues/5953 Style/AccessModifierDeclarations: Enabled: false +# make rspec formatting more flexible Style/BlockDelimiters: Exclude: - '**/*_spec.rb' - '**/shared_examples/**/*.rb' +# document our public APIs +Style/Documentation: + Enabled: true + Include: + - 'Library/Homebrew/formula.rb' +Style/DocumentationMethod: + Enabled: true + Include: + - 'Library/Homebrew/formula.rb' + # so many of these in formulae but none in here Style/GuardClause: Enabled: true -# hash-rockets preferred for formulae, a: 1 preferred elsewhere +# hash-rockets preferred for formulae, a: 1 preferred here Style/HashSyntax: EnforcedStyle: ruby19_no_mixed_keys diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index 4dfaf9e7e0..fe4225384e 100644 --- a/Library/Homebrew/build.rb +++ b/Library/Homebrew/build.rb @@ -174,7 +174,7 @@ class Build raise end Keg.new(path).optlink - rescue StandardError + rescue raise "#{f.opt_prefix} not present or broken\nPlease reinstall #{f.full_name}. Sorry :(" end end diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb index a3075ff409..aa5b959401 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb @@ -30,7 +30,7 @@ module Hbc def class_for_dsl_key(dsl_key) namespace = self.class.name.to_s.sub(/::.*::.*$/, "") - self.class.const_get("#{namespace}::DSL::#{dsl_key.to_s.split("_").collect(&:capitalize).join}") + self.class.const_get("#{namespace}::DSL::#{dsl_key.to_s.split("_").map(&:capitalize).join}") end def abstract_phase(dsl_key) diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb index 99192ee1df..2f596f0d10 100644 --- a/Library/Homebrew/cask/lib/hbc/audit.rb +++ b/Library/Homebrew/cask/lib/hbc/audit.rb @@ -39,7 +39,7 @@ module Hbc check_latest_with_auto_updates check_stanza_requires_uninstall self - rescue StandardError => e + rescue => e odebug "#{e.message}\n#{e.backtrace.join("\n")}" add_error "exception while auditing #{cask}: #{e.message}" self diff --git a/Library/Homebrew/cask/lib/hbc/cli.rb b/Library/Homebrew/cask/lib/hbc/cli.rb index 21a632bca7..34e9d6904a 100644 --- a/Library/Homebrew/cask/lib/hbc/cli.rb +++ b/Library/Homebrew/cask/lib/hbc/cli.rb @@ -94,7 +94,7 @@ module Hbc path = PATH.new(tap_cmd_directories, ENV["HOMEBREW_PATH"]) external_ruby_cmd = tap_cmd_directories.map { |d| d/"brewcask-#{command}.rb" } - .detect(&:file?) + .find(&:file?) external_ruby_cmd ||= which("brewcask-#{command}.rb", path) if external_ruby_cmd @@ -126,7 +126,7 @@ module Hbc end def detect_command_and_arguments(*args) - command = args.detect do |arg| + command = args.find do |arg| if self.class.commands.include?(arg) true else diff --git a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb index 184d3377d0..8d2ec08acd 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/doctor.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/doctor.rb @@ -49,7 +49,7 @@ module Hbc if locations.empty? puts self.class.none_string else - locations.collect do |l| + locations.map do |l| add_error "Legacy install at #{l}. Run \"brew uninstall --force brew-cask\"." puts l end @@ -163,7 +163,7 @@ module Hbc def self.cask_count_for_tap(tap) Formatter.pluralize(tap.cask_files.count, "cask") - rescue StandardError + rescue add_error "Unable to read from Tap: #{tap.path}" "0" end diff --git a/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb b/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb index 29a2863ca8..779f29b068 100644 --- a/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb +++ b/Library/Homebrew/cask/lib/hbc/cli/internal_stanza.rb @@ -63,7 +63,7 @@ module Hbc begin value = cask.send(stanza) - rescue StandardError + rescue opoo "failure calling '#{stanza}' on Cask '#{cask}'" unless quiet? puts "" next diff --git a/Library/Homebrew/cask/lib/hbc/download.rb b/Library/Homebrew/cask/lib/hbc/download.rb index 1429611783..65a44e0c7f 100644 --- a/Library/Homebrew/cask/lib/hbc/download.rb +++ b/Library/Homebrew/cask/lib/hbc/download.rb @@ -39,7 +39,7 @@ module Hbc def fetch downloader.fetch @downloaded_path = downloader.cached_location - rescue StandardError => e + rescue => e error = CaskError.new("Download failed on Cask '#{cask}' with message: #{e}") error.set_backtrace e.backtrace raise error diff --git a/Library/Homebrew/cask/lib/hbc/dsl.rb b/Library/Homebrew/cask/lib/hbc/dsl.rb index 10255f497f..e3a2cfdedd 100644 --- a/Library/Homebrew/cask/lib/hbc/dsl.rb +++ b/Library/Homebrew/cask/lib/hbc/dsl.rb @@ -102,7 +102,7 @@ module Hbc instance_variable_set("@#{stanza}", yield) rescue CaskInvalidError raise - rescue StandardError => e + rescue => e raise CaskInvalidError.new(cask, "'#{stanza}' stanza failed with: #{e}") end @@ -271,7 +271,7 @@ module Hbc artifacts.add(klass.from_args(cask, *args)) rescue CaskInvalidError raise - rescue StandardError => e + rescue => e raise CaskInvalidError.new(cask, "invalid '#{klass.dsl_key}' stanza: #{e}") end end diff --git a/Library/Homebrew/cask/lib/hbc/dsl/depends_on.rb b/Library/Homebrew/cask/lib/hbc/dsl/depends_on.rb index 5faac299fa..2a377a1bd8 100644 --- a/Library/Homebrew/cask/lib/hbc/dsl/depends_on.rb +++ b/Library/Homebrew/cask/lib/hbc/dsl/depends_on.rb @@ -48,7 +48,7 @@ module Hbc else raise end - rescue StandardError + rescue raise "invalid 'depends_on macos' value: #{arg.inspect}" end end diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb index 2ecd0328a6..e8df8ac104 100644 --- a/Library/Homebrew/cask/lib/hbc/installer.rb +++ b/Library/Homebrew/cask/lib/hbc/installer.rb @@ -74,7 +74,7 @@ module Hbc extract_primary_container save_caskfile - rescue StandardError => e + rescue => e purge_versioned_files raise e end @@ -209,7 +209,7 @@ module Hbc artifact.install_phase(command: @command, verbose: verbose?, force: force?) already_installed_artifacts.unshift(artifact) end - rescue StandardError => e + rescue => e begin already_installed_artifacts.each do |artifact| next unless artifact.respond_to?(:uninstall_phase) @@ -397,7 +397,7 @@ module Hbc See System Preferences to enable it manually. EOS end - rescue StandardError => e + rescue => e purge_versioned_files raise e end @@ -503,12 +503,12 @@ module Hbc end def backup_path - return nil if @cask.staged_path.nil? + return if @cask.staged_path.nil? Pathname.new "#{@cask.staged_path}.upgrading" end def backup_metadata_path - return nil if @cask.metadata_versioned_path.nil? + return if @cask.metadata_versioned_path.nil? Pathname.new "#{@cask.metadata_versioned_path}.upgrading" end diff --git a/Library/Homebrew/cask/lib/hbc/metadata.rb b/Library/Homebrew/cask/lib/hbc/metadata.rb index 030b18dd9f..a1f5e10bb5 100644 --- a/Library/Homebrew/cask/lib/hbc/metadata.rb +++ b/Library/Homebrew/cask/lib/hbc/metadata.rb @@ -47,7 +47,7 @@ module Hbc parent = metadata_timestamped_path(version: version, timestamp: timestamp, create: create) - return nil if parent.nil? + return if parent.nil? subdir = parent.join(leaf) diff --git a/Library/Homebrew/cask/lib/hbc/staged.rb b/Library/Homebrew/cask/lib/hbc/staged.rb index fa2439a2e5..c87b742e0b 100644 --- a/Library/Homebrew/cask/lib/hbc/staged.rb +++ b/Library/Homebrew/cask/lib/hbc/staged.rb @@ -13,13 +13,13 @@ module Hbc def plist_set(key, value) plist_exec("Set #{key} #{value}") - rescue StandardError => e + rescue => e raise CaskError, "#{@cask.token}: 'plist_set' failed with: #{e}" end def bundle_identifier plist_exec("Print CFBundleIdentifier").stdout.chomp - rescue StandardError => e + rescue => e raise CaskError, "#{@cask.token}: 'bundle_identifier' failed with: #{e}" end diff --git a/Library/Homebrew/cask/lib/hbc/utils.rb b/Library/Homebrew/cask/lib/hbc/utils.rb index 275165b646..c866854a10 100644 --- a/Library/Homebrew/cask/lib/hbc/utils.rb +++ b/Library/Homebrew/cask/lib/hbc/utils.rb @@ -27,7 +27,7 @@ module Hbc tried_ownership = false begin yield path - rescue StandardError + rescue # in case of permissions problems unless tried_permissions # TODO: Better handling for the case where path is a symlink. diff --git a/Library/Homebrew/cmd/deps.rb b/Library/Homebrew/cmd/deps.rb index a3bd2b8b58..c1ccb7ca84 100644 --- a/Library/Homebrew/cmd/deps.rb +++ b/Library/Homebrew/cmd/deps.rb @@ -151,7 +151,7 @@ module Homebrew end def deps_for_formulae(formulae, recursive = false, &block) - formulae.map { |f| deps_for_formula(f, recursive) }.inject(&block) + formulae.map { |f| deps_for_formula(f, recursive) }.reduce(&block) end def puts_deps(formulae) diff --git a/Library/Homebrew/cmd/info.rb b/Library/Homebrew/cmd/info.rb index cfe8b13d87..f1e23e31f3 100644 --- a/Library/Homebrew/cmd/info.rb +++ b/Library/Homebrew/cmd/info.rb @@ -182,7 +182,7 @@ module Homebrew end def decorate_dependencies(dependencies) - deps_status = dependencies.collect do |dep| + deps_status = dependencies.map do |dep| if dep.satisfied?([]) pretty_installed(dep_display_s(dep)) else @@ -193,7 +193,7 @@ module Homebrew end def decorate_requirements(requirements) - req_status = requirements.collect do |req| + req_status = requirements.map do |req| req_s = req.display_s req.satisfied? ? pretty_installed(req_s) : pretty_uninstalled(req_s) end diff --git a/Library/Homebrew/cmd/outdated.rb b/Library/Homebrew/cmd/outdated.rb index 4d7dba0dbc..3b4f1f0e68 100644 --- a/Library/Homebrew/cmd/outdated.rb +++ b/Library/Homebrew/cmd/outdated.rb @@ -88,7 +88,7 @@ module Homebrew end json << { name: f.full_name, - installed_versions: outdated_versions.collect(&:to_s), + installed_versions: outdated_versions.map(&:to_s), current_version: current_version, pinned: f.pinned?, pinned_version: f.pinned_version } diff --git a/Library/Homebrew/cmd/search.rb b/Library/Homebrew/cmd/search.rb index 26a58a88e8..a8d7ccca5f 100644 --- a/Library/Homebrew/cmd/search.rb +++ b/Library/Homebrew/cmd/search.rb @@ -56,7 +56,7 @@ module Homebrew conflicts(*package_manager_switches) end - if package_manager = PACKAGE_MANAGERS.detect { |name,| args[:"#{name}?"] } + if package_manager = PACKAGE_MANAGERS.find { |name,| args[:"#{name}?"] } _, url = package_manager exec_browser url.call(URI.encode_www_form_component(args.remaining.join(" "))) return diff --git a/Library/Homebrew/debrew.rb b/Library/Homebrew/debrew.rb index a04c2c54ae..d41b1918b2 100644 --- a/Library/Homebrew/debrew.rb +++ b/Library/Homebrew/debrew.rb @@ -60,7 +60,7 @@ module Debrew if i.positive? choice = menu.entries[i - 1] else - possible = menu.entries.find_all { |e| e.name.start_with?(input) } + possible = menu.entries.select { |e| e.name.start_with?(input) } case possible.size when 0 then puts "No such option" diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index ce80021feb..8b13751fae 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -257,8 +257,10 @@ module Homebrew wanted_mode = 0100644 & ~File.umask actual_mode = formula.path.stat.mode unless actual_mode == wanted_mode - problem format("Incorrect file permissions (%03o): chmod %03o %s", - actual_mode & 0777, wanted_mode & 0777, formula.path) + problem format("Incorrect file permissions (%03o): chmod %03o %{path}", + actual: actual_mode & 0777, + wanted: wanted_mode & 0777, + path: formula.path) end problem "'DATA' was found, but no '__END__'" if text.data? && !text.end? @@ -407,7 +409,7 @@ module Homebrew dep.options.each do |opt| next if dep_f.option_defined?(opt) - next if dep_f.requirements.detect do |r| + next if dep_f.requirements.find do |r| if r.recommended? opt.name == "with-#{r.name}" elsif r.optional? diff --git a/Library/Homebrew/dev-cmd/linkage.rb b/Library/Homebrew/dev-cmd/linkage.rb index aa07ec0263..2fa90a6a68 100644 --- a/Library/Homebrew/dev-cmd/linkage.rb +++ b/Library/Homebrew/dev-cmd/linkage.rb @@ -29,7 +29,7 @@ module Homebrew CacheStoreDatabase.use(:linkage) do |db| kegs = if ARGV.kegs.empty? - Formula.installed.collect(&:opt_or_installed_prefix_keg).reject(&:nil?) + Formula.installed.map(&:opt_or_installed_prefix_keg).reject(&:nil?) else ARGV.kegs end diff --git a/Library/Homebrew/dev-cmd/pull.rb b/Library/Homebrew/dev-cmd/pull.rb index c457e199d6..1cc3231627 100644 --- a/Library/Homebrew/dev-cmd/pull.rb +++ b/Library/Homebrew/dev-cmd/pull.rb @@ -513,7 +513,7 @@ module Homebrew def self.lookup(name) json = Utils.popen_read(HOMEBREW_BREW_FILE, "info", "--json=v1", name) - return nil unless $CHILD_STATUS.success? + return unless $CHILD_STATUS.success? Homebrew.force_utf8!(json) FormulaInfoFromJson.new(JSON.parse(json)[0]) @@ -526,9 +526,9 @@ module Homebrew def bottle_info(my_bottle_tag = Utils::Bottles.tag) tag_s = my_bottle_tag.to_s - return nil unless info["bottle"]["stable"] + return unless info["bottle"]["stable"] btl_info = info["bottle"]["stable"]["files"][tag_s] - return nil unless btl_info + return unless btl_info BottleInfo.new(btl_info["url"], btl_info["sha256"]) end diff --git a/Library/Homebrew/diagnostic.rb b/Library/Homebrew/diagnostic.rb index 807c9024b6..19223a1096 100644 --- a/Library/Homebrew/diagnostic.rb +++ b/Library/Homebrew/diagnostic.rb @@ -61,13 +61,13 @@ module Homebrew # Specify paths relative to a prefix eg. "include/foo.h". # Sets @found for your convenience. def find_relative_paths(*relative_paths) - @found = [HOMEBREW_PREFIX, "/usr/local"].uniq.inject([]) do |found, prefix| + @found = [HOMEBREW_PREFIX, "/usr/local"].uniq.reduce([]) do |found, prefix| found + relative_paths.map { |f| File.join(prefix, f) }.select { |f| File.exist? f } end end def inject_file_list(list, string) - list.inject(string) { |acc, elem| acc << " #{elem}\n" } + list.reduce(string) { |acc, elem| acc << " #{elem}\n" } end ############# END HELPERS diff --git a/Library/Homebrew/extend/ARGV.rb b/Library/Homebrew/extend/ARGV.rb index 3385ff5a00..f637dd9326 100644 --- a/Library/Homebrew/extend/ARGV.rb +++ b/Library/Homebrew/extend/ARGV.rb @@ -93,7 +93,7 @@ module HomebrewArgvExtension def kegs require "keg" require "formula" - @kegs ||= downcased_unique_named.collect do |name| + @kegs ||= downcased_unique_named.map do |name| raise UsageError if name.empty? rack = Formulary.to_rack(name.downcase) diff --git a/Library/Homebrew/extend/os/mac/requirements/java_requirement.rb b/Library/Homebrew/extend/os/mac/requirements/java_requirement.rb index b561d3c7af..bd3ff0d541 100644 --- a/Library/Homebrew/extend/os/mac/requirements/java_requirement.rb +++ b/Library/Homebrew/extend/os/mac/requirements/java_requirement.rb @@ -23,11 +23,11 @@ class JavaRequirement < Requirement end def java_home_cmd - return nil unless File.executable?("/usr/libexec/java_home") + return unless File.executable?("/usr/libexec/java_home") args = %w[--failfast] args << "--version" << @version.to_s if @version java_home = Utils.popen_read("/usr/libexec/java_home", *args).chomp - return nil unless $CHILD_STATUS.success? + return unless $CHILD_STATUS.success? Pathname.new(java_home)/"bin/java" end diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index c8aef3491f..af2c449d7e 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -1120,7 +1120,7 @@ class Formula begin yield self, staging - rescue StandardError + rescue staging.retain! if ARGV.interactive? || ARGV.debug? raise ensure @@ -1373,7 +1373,7 @@ class Formula files.each do |file| begin yield Formulary.factory(file) - rescue StandardError => e + rescue => e # Don't let one broken formula break commands. But do complain. onoe "Failed to import: #{file}" puts e @@ -1812,7 +1812,9 @@ class Formula @exec_count ||= 0 @exec_count += 1 - logfn = format("#{logs}/#{active_log_prefix}%02d.%s", @exec_count, File.basename(cmd).split(" ").first) + logfn = format("#{logs}/#{active_log_prefix}%02d.%{cmd_base}", + exec_count: @exec_count, + cmd_base: File.basename(cmd).split(" ").first) logs.mkpath File.open(logfn, "w") do |log| @@ -2006,7 +2008,7 @@ class Formula $stdout.reopen(out) $stderr.reopen(out) out.close - args.collect!(&:to_s) + args.map!(&:to_s) begin exec(cmd, *args) rescue diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 974ed85cea..abd035e5b0 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -283,7 +283,7 @@ module Formulary # to install the formula will be set instead. def self.from_rack(rack, spec = nil, alias_path: nil) kegs = rack.directory? ? rack.subdirs.map { |d| Keg.new(d) } : [] - keg = kegs.detect(&:linked?) || kegs.detect(&:optlinked?) || kegs.max_by(&:version) + keg = kegs.find(&:linked?) || kegs.find(&:optlinked?) || kegs.max_by(&:version) if keg from_keg(keg, spec, alias_path: alias_path) @@ -432,7 +432,7 @@ module Formulary "#{tap}HomebrewFormula/#{name}.rb", "#{tap}#{name}.rb", "#{tap}Aliases/#{name}", - ]).detect(&:file?) + ]).find(&:file?) end.compact end diff --git a/Library/Homebrew/keg.rb b/Library/Homebrew/keg.rb index 02164bed77..1ef7c41b0b 100644 --- a/Library/Homebrew/keg.rb +++ b/Library/Homebrew/keg.rb @@ -389,7 +389,7 @@ class Keg def oldname_opt_record @oldname_opt_record ||= if (opt_dir = HOMEBREW_PREFIX/"opt").directory? - opt_dir.subdirs.detect do |dir| + opt_dir.subdirs.find do |dir| dir.symlink? && dir != opt_record && path.parent == dir.resolved_path.parent end end diff --git a/Library/Homebrew/locale.rb b/Library/Homebrew/locale.rb index 38860403fe..44179ada0f 100644 --- a/Library/Homebrew/locale.rb +++ b/Library/Homebrew/locale.rb @@ -69,8 +69,8 @@ class Locale alias == eql? def detect(locale_groups) - locale_groups.detect { |locales| locales.any? { |locale| eql?(locale) } } || - locale_groups.detect { |locales| locales.any? { |locale| include?(locale) } } + locale_groups.find { |locales| locales.any? { |locale| eql?(locale) } } || + locale_groups.find { |locales| locales.any? { |locale| include?(locale) } } end def to_s diff --git a/Library/Homebrew/migrator.rb b/Library/Homebrew/migrator.rb index bc94321f70..52f15bc339 100644 --- a/Library/Homebrew/migrator.rb +++ b/Library/Homebrew/migrator.rb @@ -174,7 +174,7 @@ class Migrator keg_dirs += new_cellar.subdirs if new_cellar.exist? keg_dirs += old_cellar.subdirs kegs = keg_dirs.map { |d| Keg.new(d) } - kegs.detect(&:linked?) || kegs.detect(&:optlinked?) + kegs.find(&:linked?) || kegs.find(&:optlinked?) end def pinned? diff --git a/Library/Homebrew/os/mac/architecture_list.rb b/Library/Homebrew/os/mac/architecture_list.rb index 6373aed44d..6b357652e7 100644 --- a/Library/Homebrew/os/mac/architecture_list.rb +++ b/Library/Homebrew/os/mac/architecture_list.rb @@ -37,7 +37,7 @@ module ArchitectureListExtension end def as_arch_flags - collect { |a| "-arch #{a}" }.join(" ") + map { |a| "-arch #{a}" }.join(" ") end def as_cmake_arch_flags diff --git a/Library/Homebrew/requirements/java_requirement.rb b/Library/Homebrew/requirements/java_requirement.rb index c9f5b8e182..06f5f32b36 100644 --- a/Library/Homebrew/requirements/java_requirement.rb +++ b/Library/Homebrew/requirements/java_requirement.rb @@ -94,7 +94,7 @@ class JavaRequirement < Requirement end def preferred_java - possible_javas.detect do |java| + possible_javas.find do |java| next false unless java&.executable? next true unless @version next true if satisfies_version(java) diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index 2a6a25288b..bfe2812911 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -335,7 +335,7 @@ class Tap # path to the directory of all {Formula} files for this {Tap}. def formula_dir - @formula_dir ||= potential_formula_dirs.detect(&:directory?) || path/"Formula" + @formula_dir ||= potential_formula_dirs.find(&:directory?) || path/"Formula" end def potential_formula_dirs diff --git a/Library/Homebrew/test/rubocops/patches_cop_spec.rb b/Library/Homebrew/test/rubocops/patches_cop_spec.rb index 0c04a98c05..7173092bac 100644 --- a/Library/Homebrew/test/rubocops/patches_cop_spec.rb +++ b/Library/Homebrew/test/rubocops/patches_cop_spec.rb @@ -47,56 +47,62 @@ describe RuboCop::Cop::FormulaAudit::Patches do inspect_source(source) expected_offense = if patch_url =~ %r{/raw\.github\.com/} - [{ message: <<~EOS.chomp, - GitHub/Gist patches should specify a revision: - #{patch_url} + [{ message: + <<~EOS.chomp, + GitHub/Gist patches should specify a revision: + #{patch_url} EOS severity: :convention, line: 5, column: 12, source: source }] elsif patch_url =~ %r{macports/trunk} - [{ message: <<~EOS.chomp, - MacPorts patches should specify a revision instead of trunk: - #{patch_url} + [{ message: + <<~EOS.chomp, + MacPorts patches should specify a revision instead of trunk: + #{patch_url} EOS severity: :convention, line: 5, column: 33, source: source }] elsif patch_url =~ %r{^http://trac\.macports\.org} - [{ message: <<~EOS.chomp, - Patches from MacPorts Trac should be https://, not http: - #{patch_url} + [{ message: + <<~EOS.chomp, + Patches from MacPorts Trac should be https://, not http: + #{patch_url} EOS severity: :convention, line: 5, column: 5, source: source }] elsif patch_url =~ %r{^http://bugs\.debian\.org} - [{ message: <<~EOS.chomp, - Patches from Debian should be https://, not http: - #{patch_url} + [{ message: + <<~EOS.chomp, + Patches from Debian should be https://, not http: + #{patch_url} EOS severity: :convention, line: 5, column: 5, source: source }] elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - [{ message: <<~EOS, - use GitHub pull request URLs: - https://github.com/foo/foo-bar/pull/100.patch - Rather than patch-diff: - https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch + [{ message: + <<~EOS, + use GitHub pull request URLs: + https://github.com/foo/foo-bar/pull/100.patch + Rather than patch-diff: + https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch EOS severity: :convention, line: 5, column: 5, source: source }] elsif patch_url =~ %r{https?://github\.com/.+/.+/(?:commit|pull)/[a-fA-F0-9]*.(?:patch|diff)} - [{ message: <<~EOS, - GitHub patches should use the full_index parameter: - #{patch_url}?full_index=1 + [{ message: + <<~EOS, + GitHub patches should use the full_index parameter: + #{patch_url}?full_index=1 EOS severity: :convention, line: 5, @@ -132,9 +138,10 @@ describe RuboCop::Cop::FormulaAudit::Patches do line: 4, column: 2, source: source }, - { message: <<~EOS.chomp, - Patches from MacPorts Trac should be https://, not http: - http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/ + { message: + <<~EOS.chomp, + Patches from MacPorts Trac should be https://, not http: + http://trac.macports.org/export/68507/trunk/dports/net/trafshow/files/ EOS severity: :convention, line: 8, @@ -175,47 +182,52 @@ describe RuboCop::Cop::FormulaAudit::Patches do inspect_source(source) expected_offense = if patch_url =~ %r{/raw\.github\.com/} - [{ message: <<~EOS.chomp, - GitHub/Gist patches should specify a revision: - #{patch_url} + [{ message: + <<~EOS.chomp, + GitHub/Gist patches should specify a revision: + #{patch_url} EOS severity: :convention, line: 5, column: 16, source: source }] elsif patch_url =~ %r{macports/trunk} - [{ message: <<~EOS.chomp, - MacPorts patches should specify a revision instead of trunk: - #{patch_url} + [{ message: + <<~EOS.chomp, + MacPorts patches should specify a revision instead of trunk: + #{patch_url} EOS severity: :convention, line: 5, column: 37, source: source }] elsif patch_url =~ %r{^http://trac\.macports\.org} - [{ message: <<~EOS.chomp, - Patches from MacPorts Trac should be https://, not http: - #{patch_url} + [{ message: + <<~EOS.chomp, + Patches from MacPorts Trac should be https://, not http: + #{patch_url} EOS severity: :convention, line: 5, column: 9, source: source }] elsif patch_url =~ %r{^http://bugs\.debian\.org} - [{ message: <<~EOS.chomp, - Patches from Debian should be https://, not http: - #{patch_url} + [{ message: + <<~EOS.chomp, + Patches from Debian should be https://, not http: + #{patch_url} EOS severity: :convention, line: 5, column: 9, source: source }] elsif patch_url =~ %r{https?://patch-diff\.githubusercontent\.com/raw/(.+)/(.+)/pull/(.+)\.(?:diff|patch)} - [{ message: <<~EOS, - use GitHub pull request URLs: - https://github.com/foo/foo-bar/pull/100.patch - Rather than patch-diff: - https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch + [{ message: + <<~EOS, + use GitHub pull request URLs: + https://github.com/foo/foo-bar/pull/100.patch + Rather than patch-diff: + https://patch-diff.githubusercontent.com/raw/foo/foo-bar/pull/100.patch EOS severity: :convention, line: 5, diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb index d4068963f3..54c7dc1d80 100644 --- a/Library/Homebrew/unpack_strategy.rb +++ b/Library/Homebrew/unpack_strategy.rb @@ -80,11 +80,11 @@ module UnpackStrategy def self.from_extension(extension) strategies.sort_by { |s| s.extensions.map(&:length).max(0) } .reverse - .detect { |s| s.extensions.any? { |ext| extension.end_with?(ext) } } + .find { |s| s.extensions.any? { |ext| extension.end_with?(ext) } } end def self.from_magic(path) - strategies.detect { |s| s.can_extract?(path) } + strategies.find { |s| s.can_extract?(path) } end def self.detect(path, extension_only: false, type: nil, ref_type: nil, ref: nil) @@ -93,7 +93,7 @@ module UnpackStrategy if extension_only strategy ||= from_extension(path.extname) strategy ||= strategies.select { |s| s < Directory || s == Fossil } - .detect { |s| s.can_extract?(path) } + .find { |s| s.can_extract?(path) } else strategy ||= from_magic(path) strategy ||= from_extension(path.extname) diff --git a/Library/Homebrew/unpack_strategy/dmg.rb b/Library/Homebrew/unpack_strategy/dmg.rb index b47d7453da..5068a57fc5 100644 --- a/Library/Homebrew/unpack_strategy/dmg.rb +++ b/Library/Homebrew/unpack_strategy/dmg.rb @@ -31,8 +31,10 @@ module UnpackStrategy end def bom + # rubocop:disable Style/AsciiComments # We need to use `find` here instead of Ruby in order to properly handle # file names containing special characters, such as “e” + “´” vs. “é”. + # rubocop:enable Style/AsciiComments system_command("find", args: [".", "-print0"], chdir: self, print_stderr: false) .stdout .split("\0") diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb index b3ecb0d091..a29d494ff9 100644 --- a/Library/Homebrew/utils.rb +++ b/Library/Homebrew/utils.rb @@ -182,7 +182,7 @@ module Homebrew def _system(cmd, *args, **options) pid = fork do yield if block_given? - args.collect!(&:to_s) + args.map!(&:to_s) begin exec(cmd, *args, **options) rescue @@ -380,7 +380,7 @@ end # GZips the given paths, and returns the gzipped paths def gzip(*paths) - paths.collect do |path| + paths.map do |path| safe_system "gzip", path Pathname.new("#{path}.gz") end @@ -426,7 +426,7 @@ def nostdout end def paths - @paths ||= PATH.new(ENV["HOMEBREW_PATH"]).collect do |p| + @paths ||= PATH.new(ENV["HOMEBREW_PATH"]).map do |p| begin File.expand_path(p).chomp("/") rescue ArgumentError diff --git a/Library/Homebrew/utils/inreplace.rb b/Library/Homebrew/utils/inreplace.rb index b4c219f064..f7d06db5aa 100644 --- a/Library/Homebrew/utils/inreplace.rb +++ b/Library/Homebrew/utils/inreplace.rb @@ -1,7 +1,7 @@ module Utils class InreplaceError < RuntimeError def initialize(errors) - formatted_errors = errors.inject("inreplace failed\n") do |s, (path, errs)| + formatted_errors = errors.reduce("inreplace failed\n") do |s, (path, errs)| s << "#{path}:\n" << errs.map { |e| " #{e}\n" }.join end super formatted_errors