Apply suggestions from code review

Homebrew · Jul 6, 2024 · bab0dda · bab0dda
1 parent cd18694
commit bab0dda
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 9 deletions.
diff --git a/Library/Homebrew/development_tools.rb b/Library/Homebrew/development_tools.rb
@@ -16,7 +16,7 @@ def locate(tool)
  # Don't call tools (cc, make, strip, etc.) directly!
  # Give the name of the binary you look for as a string to this method
  # in order to get the full path back as a Pathname.
- (@locate ||= T.let({}, T.nilable(T::Hash[T.untyped, T.untyped]))).fetch(tool) do |key|
+ (@locate ||= T.let({}, T.nilable(T::Hash[T.any(String, Symbol), T.untyped]))).fetch(tool) do |key|
  @locate[key] = if File.executable?((path = "/usr/bin/#{tool}"))
  Pathname.new path
  # Homebrew GCCs most frequently; much faster to check this before xcrun
@@ -63,8 +63,9 @@ def default_compiler
  sig { returns(Version) }
  def clang_version
  @clang_version ||= T.let(
- if (path = locate("clang")) && (bv = `#{path} --version`[/(?:clang|LLVM) version (\d+\.\d(?:\.\d)?)/, 1])
- Version.new(bv)
+ if (path = locate("clang")) &&
+ (build_version = `#{path} --version`[/(?:clang|LLVM) version (\d+\.\d(?:\.\d)?)/, 1])
+ Version.new(build_version)
  else
  Version::NULL
  end, T.nilable(Version)
@@ -93,8 +94,8 @@ def clang_build_version
  def llvm_clang_build_version
  @llvm_clang_build_version ||= T.let(begin
  path = Formulary.factory("llvm").opt_prefix/"bin/clang"
- if path.executable? && (bv = `#{path} --version`[/clang version (\d+\.\d\.\d)/, 1])
- Version.new(bv)
+ if path.executable? && (build_version = `#{path} --version`[/clang version (\d+\.\d\.\d)/, 1])
+ Version.new(build_version)
  else
  Version::NULL
  end
@@ -109,8 +110,9 @@ def gcc_version(cc)
  (@gcc_version ||= T.let({}, T.nilable(T::Hash[String, Version]))).fetch(cc) do
  path = HOMEBREW_PREFIX/"opt/#{CompilerSelector.preferred_gcc}/bin"/cc
  path = locate(cc) unless path.exist?
- version = if path && (bv = `#{path} --version`[/gcc(?:(?:-\d+(?:\.\d)?)? \(.+\))? (\d+\.\d\.\d)/, 1])
- Version.new(bv)
+ version = if path &&
+ (build_version = `#{path} --version`[/gcc(?:(?:-\d+(?:\.\d)?)? \(.+\))? (\d+\.\d\.\d)/, 1])
+ Version.new(build_version)
  else
  Version::NULL
  end

diff --git a/Library/Homebrew/unpack_strategy.rb b/Library/Homebrew/unpack_strategy.rb
@@ -7,6 +7,7 @@
 module UnpackStrategy
  extend T::Helpers
 
+ # FIXME: Enable cop again when https://github.com/sorbet/sorbet/issues/3532 is fixed.
  AnyStrategy = T.type_alias do # rubocop:disable Style/MutableConstant
  T.any(
  T.class_of(UnpackStrategy::Tar),
@@ -101,6 +102,8 @@ def self.from_type(type)
 
  sig { params(extension: String).returns(T.nilable(AnyStrategy)) }
  def self.from_extension(extension)
+ return unless strategies
+
  strategies&.sort_by { |s| s.extensions.map(&:length).max || 0 }
  &.reverse
  &.find { |s| s.extensions.any? { |ext| extension.end_with?(ext) } }
@@ -120,8 +123,8 @@ def self.detect(path, prioritize_extension: false, type: nil, ref_type: nil, ref
 
  if prioritize_extension && path.extname.present?
  strategy ||= from_extension(path.extname)
- strategy ||= strategies&.select { |s| s < Directory || s == Fossil }
-  &.find { |s| s.can_extract?(path) }
+
+ strategy ||= strategies&.find { |s| (s < Directory || s == Fossil) && s.can_extract?(path) }
  else
  strategy ||= from_magic(path)
  strategy ||= from_extension(path.extname)