Merge pull request #19314 from Homebrew/typed-dep-helpers

Enable strict typing in DependenciesHelpers

Library/Homebrew/cask/macos.rb

@@ -1,11 +1,9 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 module OS
   module Mac
-    module_function
+    SYSTEM_DIRS = T.let([
-
-    SYSTEM_DIRS = [
       "/",
       "/Applications",
       "/Applications/Utilities",
@@ -234,14 +232,13 @@ module OS
       "/var/spool/mail",
       "/var/tmp",
     ]
-                  .map(&method(:Pathname))
+                  .to_set { Pathname(_1) }
-                  .to_set
+                  .freeze, T::Set[Pathname])
-                  .freeze
     private_constant :SYSTEM_DIRS
 
     # TODO: There should be a way to specify a containing
     #       directory under which nothing can be deleted.
-    UNDELETABLE_PATHS = [
+    UNDELETABLE_PATHS = T.let([
       "~/",
       "~/Applications",
       "~/Applications/.localized",
@@ -378,14 +375,16 @@ module OS
     ]
                         .to_set { |path| Pathname(path.sub(%r{^~(?=(/|$))}, Dir.home)).expand_path }
                         .union(SYSTEM_DIRS)
-                        .freeze
+                        .freeze, T::Set[Pathname])
     private_constant :UNDELETABLE_PATHS
 
-    def system_dir?(dir)
+    sig { params(dir: T.any(Pathname, String)).returns(T::Boolean) }
+    def self.system_dir?(dir)
       SYSTEM_DIRS.include?(Pathname.new(dir).expand_path)
     end
 
-    def undeletable?(path)
+    sig { params(path: T.any(Pathname, String)).returns(T::Boolean) }
+    def self.undeletable?(path)
       UNDELETABLE_PATHS.include?(Pathname.new(path).expand_path)
     end
   end

Library/Homebrew/cmd/deps.rb

@@ -221,8 +221,8 @@ module Homebrew
         deps = dependency.runtime_dependencies if @use_runtime_dependencies
 
         if recursive
-          deps ||= recursive_includes(Dependency, dependency, includes, ignores)
+          deps ||= recursive_dep_includes(dependency, includes, ignores)
-          reqs   = recursive_includes(Requirement, dependency, includes, ignores)
+          reqs   = recursive_req_includes(dependency, includes, ignores)
         else
           deps ||= select_includes(dependency.deps, ignores, includes)
           reqs   = select_includes(dependency.requirements, ignores, includes)

Library/Homebrew/cmd/uses.rb

@@ -94,7 +94,7 @@ module Homebrew
 
       sig {
         params(use_runtime_dependents: T::Boolean, used_formulae: T::Array[T.any(Formula, UnavailableFormula)])
-          .returns(T::Array[Formula])
+          .returns(T::Array[T.any(Formula, CaskDependent)])
       }
       def intersection_of_dependents(use_runtime_dependents, used_formulae)
         recursive = args.recursive?
@@ -150,26 +150,30 @@ module Homebrew
 
       sig {
         params(
-          dependents: T::Array[Formula], used_formulae: T::Array[T.any(Formula, UnavailableFormula)],
+          dependents:    T::Array[T.any(Formula, CaskDependent)],
-          recursive: T::Boolean, includes: T::Array[Symbol], ignores: T::Array[Symbol]
+          used_formulae: T::Array[T.any(Formula, UnavailableFormula)],
-        ).returns(
+          recursive:     T::Boolean,
-          T::Array[Formula],
+          includes:      T::Array[Symbol],
-        )
+          ignores:       T::Array[Symbol],
+        ).returns(T::Array[T.any(Formula, CaskDependent)])
       }
       def select_used_dependents(dependents, used_formulae, recursive, includes, ignores)
         dependents.select do |d|
           deps = if recursive
-            recursive_includes(Dependency, d, includes, ignores)
+            recursive_dep_includes(d, includes, ignores)
           else
             select_includes(d.deps, ignores, includes)
           end
 
           used_formulae.all? do |ff|
             deps.any? do |dep|
-              match = begin
+              match = case dep
+              when Dependency
                 dep.to_formula.full_name == ff.full_name if dep.name.include?("/")
-              rescue
+              when Requirement
                 nil
+              else
+                T.absurd(dep)
               end
               next match unless match.nil?
 

Library/Homebrew/dependencies.rb

@@ -39,11 +39,6 @@ class Dependencies < SimpleDelegator
   def inspect
     "#<#{self.class.name}: #{__getobj__}>"
   end
-
-  sig { returns(T::Array[Dependency]) }
-  def to_a
-    __getobj__.to_a
-  end
 end
 
 # A collection of requirements.

Library/Homebrew/dependencies.rbi

@@ -1,13 +1,32 @@
 # typed: strict
 
 class Dependencies < SimpleDelegator
+  include Enumerable
   include Kernel
+
   # This is a workaround to enable `alias eql? ==`
   # @see https://github.com/sorbet/sorbet/issues/2378#issuecomment-569474238
   sig { params(other: BasicObject).returns(T::Boolean) }
   def ==(other); end
+
+  sig { params(blk: T.proc.params(arg0: Dependency).void).returns(T.self_type) }
+  sig { returns(T::Enumerator[Dependency]) }
+  def each(&blk); end
+
+  sig { params(blk: T.proc.params(arg0: Dependency).returns(T::Boolean)).returns(T::Array[Dependency]) }
+  sig { returns(T::Enumerator[Dependency]) }
+  def select(&blk); end
 end
 
 class Requirements < SimpleDelegator
+  include Enumerable
   include Kernel
+
+  sig { params(blk: T.proc.params(arg0: Requirement).void).returns(T.self_type) }
+  sig { returns(T::Enumerator[Requirement]) }
+  def each(&blk); end
+
+  sig { params(blk: T.proc.params(arg0: Requirement).returns(T::Boolean)).returns(T::Array[Requirement]) }
+  sig { returns(T::Enumerator[Requirement]) }
+  def select(&blk); end
 end

Library/Homebrew/dependencies_helpers.rb

@@ -1,14 +1,10 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "cask_dependent"
 
 # Helper functions for dependencies.
 module DependenciesHelpers
-  extend T::Helpers
-
-  requires_ancestor { Kernel }
-
   def args_includes_ignores(args)
     includes = [:required?, :recommended?] # included by default
     includes << :implicit? if args.include_implicit?
@@ -23,9 +19,35 @@ module DependenciesHelpers
     [includes, ignores]
   end
 
-  def recursive_includes(klass, root_dependent, includes, ignores)
+  sig {
-    raise ArgumentError, "Invalid class argument: #{klass}" if klass != Dependency && klass != Requirement
+    params(root_dependent: T.any(Formula, CaskDependent), includes: T::Array[Symbol], ignores: T::Array[Symbol])
+      .returns(T::Array[Dependency])
+  }
+  def recursive_dep_includes(root_dependent, includes, ignores)
+    # The use of T.unsafe is recommended by the Sorbet docs:
+    #   https://sorbet.org/docs/overloads#multiple-methods-but-sharing-a-common-implementation
+    T.unsafe(recursive_includes(Dependency, root_dependent, includes, ignores))
+  end
 
+  sig {
+    params(root_dependent: T.any(Formula, CaskDependent), includes: T::Array[Symbol], ignores: T::Array[Symbol])
+      .returns(Requirements)
+  }
+  def recursive_req_includes(root_dependent, includes, ignores)
+    # The use of T.unsafe is recommended by the Sorbet docs:
+    #   https://sorbet.org/docs/overloads#multiple-methods-but-sharing-a-common-implementation
+    T.unsafe(recursive_includes(Requirement, root_dependent, includes, ignores))
+  end
+
+  sig {
+    params(
+      klass:          T.any(T.class_of(Dependency), T.class_of(Requirement)),
+      root_dependent: T.any(Formula, CaskDependent),
+      includes:       T::Array[Symbol],
+      ignores:        T::Array[Symbol],
+    ).returns(T.any(T::Array[Dependency], Requirements))
+  }
+  def recursive_includes(klass, root_dependent, includes, ignores)
     cache_key = "recursive_includes_#{includes}_#{ignores}"
 
     klass.expand(root_dependent, cache_key:) do |dependent, dep|
@@ -43,6 +65,11 @@ module DependenciesHelpers
     end
   end
 
+  sig {
+    params(dependables: T.any(Dependencies, Requirements), ignores: T::Array[Symbol],
+           includes: T::Array[Symbol])
+      .returns(T::Array[T.any(Dependency, Requirement)])
+  }
   def select_includes(dependables, ignores, includes)
     dependables.select do |dep|
       next false if ignores.any? { |ignore| dep.public_send(ignore) }
@@ -51,14 +78,18 @@ module DependenciesHelpers
     end
   end
 
+  sig {
+    params(formulae_or_casks: T::Array[T.any(Formula, Keg, Cask::Cask)])
+      .returns(T::Array[T.any(Formula, CaskDependent)])
+  }
   def dependents(formulae_or_casks)
     formulae_or_casks.map do |formula_or_cask|
-      if formula_or_cask.is_a?(Formula)
+      case formula_or_cask
-        formula_or_cask
+      when Formula then formula_or_cask
+      when Cask::Cask then CaskDependent.new(formula_or_cask)
       else
-        CaskDependent.new(formula_or_cask)
+        raise TypeError, "Unsupported type: #{formula_or_cask.class}"
       end
     end
   end
-  module_function :dependents
 end

Library/Homebrew/dependencies_helpers.rbi

@@ -0,0 +1,12 @@
+# typed: strict
+
+module DependenciesHelpers
+  include Kernel
+
+  # This sig is in an RBI to avoid both circular dependencies and unnecessary requires
+  sig {
+    params(args: T.any(Homebrew::Cmd::Deps::Args, Homebrew::Cmd::Uses::Args))
+      .returns([T::Array[Symbol], T::Array[Symbol]])
+  }
+  def args_includes_ignores(args); end
+end

Library/Homebrew/test/dependencies_helpers_spec.rb

@@ -35,7 +35,7 @@ RSpec.describe DependenciesHelpers do
       :any_version_installed?,
     ]
 
-    dependents = described_class.dependents([foo, foo_cask, bar, bar_cask])
+    dependents = Class.new.extend(described_class).dependents([foo, foo_cask, bar, bar_cask])
 
     dependents.each do |dependent|
       methods.each do |method|