Reduce shape variations in Homebrew::Service

Author: dduugg

Library/Homebrew/formula_cellar_checks.rb

@@ -306,7 +306,7 @@ def check_service_command(formula)
     return unless formula.service?
     return unless formula.service.command?
 
-    "Service command does not exist" unless File.exist?(T.must(formula.service.command).first)
+    "Service command does not exist" unless File.exist?(formula.service.command.first)
   end
 
   sig { params(formula: Formula).returns(T.nilable(String)) }

Library/Homebrew/service.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "ipaddr"
@@ -23,13 +23,38 @@ class Service
     PROCESS_TYPE_ADAPTIVE = :adaptive
 
     KEEP_ALIVE_KEYS = [:always, :successful_exit, :crashed, :path].freeze
+    SOCKET_STRING_REGEX = %r{^([a-z]+)://(.+):([0-9]+)$}i
+
+    RunParam = T.type_alias { T.nilable(T.any(T::Array[T.any(String, Pathname)], String, Pathname)) }
+    Sockets = T.type_alias { T::Hash[Symbol, { host: String, port: String, type: String }] }
 
-    # sig { params(formula: Formula).void }
+    sig { returns(String) }
+    attr_reader :plist_name, :service_name
+
+    sig { params(formula: Formula, block: T.nilable(T.proc.void)).void }
     def initialize(formula, &block)
+      @cron = T.let({}, T::Hash[Symbol, T.any(Integer, String)])
+      @environment_variables = T.let({}, T::Hash[Symbol, String])
+      @error_log_path = T.let(nil, T.nilable(String))
       @formula = formula
-      @run_type = RUN_TYPE_IMMEDIATE
-      @run_at_load = true
-      @environment_variables = {}
+      @input_path = T.let(nil, T.nilable(String))
+      @interval = T.let(nil, T.nilable(Integer))
+      @keep_alive = T.let({}, T::Hash[Symbol, T.untyped])
+      @launch_only_once = T.let(false, T::Boolean)
+      @log_path = T.let(nil, T.nilable(String))
+      @macos_legacy_timers = T.let(false, T::Boolean)
+      @plist_name = T.let(default_plist_name, String)
+      @process_type = T.let(nil, T.nilable(Symbol))
+      @require_root = T.let(false, T::Boolean)
+      @restart_delay = T.let(nil, T.nilable(Integer))
+      @root_dir = T.let(nil, T.nilable(String))
+      @run = T.let([], T::Array[String])
+      @run_at_load = T.let(true, T::Boolean)
+      @run_params = T.let(nil, T.any(RunParam, T::Hash[Symbol, RunParam]))
+      @run_type = T.let(RUN_TYPE_IMMEDIATE, Symbol)
+      @service_name = T.let(default_service_name, String)
+      @sockets = T.let({}, Sockets)
+      @working_dir = T.let(nil, T.nilable(String))
       instance_eval(&block) if block
     end
 
@@ -43,21 +68,11 @@ def default_plist_name
       "homebrew.mxcl.#{@formula.name}"
     end
 
-    sig { returns(String) }
-    def plist_name
-      @plist_name ||= default_plist_name
-    end
-
     sig { returns(String) }
     def default_service_name
       "homebrew.#{@formula.name}"
     end
 
-    sig { returns(String) }
-    def service_name
-      @service_name ||= default_service_name
-    end
-
     sig { params(macos: T.nilable(String), linux: T.nilable(String)).void }
     def name(macos: nil, linux: nil)
       raise TypeError, "Service#name expects at least one String" if [macos, linux].none?(String)
@@ -68,27 +83,27 @@ def name(macos: nil, linux: nil)
 
     sig {
       params(
-        command: T.nilable(T.any(T::Array[T.any(String, Pathname)], String, Pathname)),
-        macos:   T.nilable(T.any(T::Array[T.any(String, Pathname)], String, Pathname)),
-        linux:   T.nilable(T.any(T::Array[T.any(String, Pathname)], String, Pathname)),
+        command: T.nilable(RunParam),
+        macos:   T.nilable(RunParam),
+        linux:   T.nilable(RunParam),
       ).returns(T.nilable(T::Array[T.any(String, Pathname)]))
     }
     def run(command = nil, macos: nil, linux: nil)
       # Save parameters for serialization
       if command
         @run_params = command
       elsif macos || linux
-        @run_params = { macos:, linux: }.compact
+        @run_params = { macos:, linux: }.compact, T.any(RunParam, T::Hash[Symbol, RunParam])
       end
 
       command ||= on_system_conditional(macos:, linux:)
       case command
       when nil
         @run
       when String, Pathname
-        @run = [command]
+        @run = [command.to_s]
       when Array
-        @run = command
+        @run = command.map(&:to_s)
       end
     end
 
@@ -161,12 +176,12 @@ def keep_alive(value = nil)
       end
     end
 
-    sig { params(value: T.nilable(T::Boolean)).returns(T.nilable(T::Boolean)) }
+    sig { params(value: T.nilable(T::Boolean)).returns(T::Boolean) }
     def require_root(value = nil)
       case value
       when nil
         @require_root
-      when true, false
+      when TrueClass, FalseClass
         @require_root = value
       end
     end
@@ -183,16 +198,14 @@ def run_at_load(value = nil)
       case value
       when nil
         @run_at_load
-      when true, false
+      when TrueClass, FalseClass
         @run_at_load = value
       end
     end
 
-    SOCKET_STRING_REGEX = %r{^([a-z]+)://(.+):([0-9]+)$}i
-
     sig {
       params(value: T.nilable(T.any(String, T::Hash[Symbol, String])))
-        .returns(T.nilable(T::Hash[Symbol, T::Hash[Symbol, String]]))
+        .returns(T::Hash[Symbol, T::Hash[Symbol, String]])
     }
     def sockets(value = nil)
       return @sockets if value.nil?
@@ -204,9 +217,11 @@ def sockets(value = nil)
         value
       end.transform_values do |socket_string|
         match = socket_string.match(SOCKET_STRING_REGEX)
-        raise TypeError, "Service#sockets a formatted socket definition as <type>://<host>:<port>" if match.blank?
+        raise TypeError, "Service#sockets a formatted socket definition as <type>://<host>:<port>" unless match
 
-        type, host, port = match.captures
+        type = T.must(match[1])
+        host = T.must(match[2])
+        port = T.must(match[3])
 
         begin
           IPAddr.new(host)
@@ -222,15 +237,15 @@ def sockets(value = nil)
     # @return [Boolean]
     sig { returns(T::Boolean) }
     def keep_alive?
-      @keep_alive.present? && @keep_alive[:always] != false
+      !@keep_alive.empty? && @keep_alive[:always] != false
     end
 
-    sig { params(value: T.nilable(T::Boolean)).returns(T.nilable(T::Boolean)) }
+    sig { params(value: T.nilable(T::Boolean)).returns(T::Boolean) }
     def launch_only_once(value = nil)
       case value
       when nil
         @launch_only_once
-      when true, false
+      when TrueClass, FalseClass
         @launch_only_once = value
       end
     end
@@ -281,13 +296,13 @@ def interval(value = nil)
       end
     end
 
-    sig { params(value: T.nilable(String)).returns(T.nilable(Hash)) }
+    sig { params(value: T.nilable(String)).returns(T::Hash[Symbol, T.any(Integer, String)]) }
     def cron(value = nil)
       case value
       when nil
         @cron
       when String
-        @cron = parse_cron(T.must(value))
+        @cron = parse_cron(value)
       end
     end
 
@@ -345,12 +360,12 @@ def environment_variables(variables = {})
       end
     end
 
-    sig { params(value: T.nilable(T::Boolean)).returns(T.nilable(T::Boolean)) }
+    sig { params(value: T.nilable(T::Boolean)).returns(T::Boolean) }
     def macos_legacy_timers(value = nil)
       case value
       when nil
         @macos_legacy_timers
-      when true, false
+      when TrueClass, FalseClass
         @macos_legacy_timers = value
       end
     end
@@ -362,14 +377,14 @@ def std_service_path_env
       "#{HOMEBREW_PREFIX}/bin:#{HOMEBREW_PREFIX}/sbin:/usr/bin:/bin:/usr/sbin:/sbin"
     end
 
-    sig { returns(T.nilable(T::Array[String])) }
+    sig { returns(T::Array[String]) }
     def command
-      @run&.map(&:to_s)&.map { |arg| arg.start_with?("~") ? File.expand_path(arg) : arg }
+      @run.map(&:to_s).map { |arg| arg.start_with?("~") ? File.expand_path(arg) : arg }
     end
 
     sig { returns(T::Boolean) }
     def command?
-      @run.present?
+      !@run.empty?
     end
 
     # Returns the `String` command to run manually instead of the service.
@@ -379,8 +394,8 @@ def manual_command
       vars = @environment_variables.except(:PATH)
                                    .map { |k, v| "#{k}=\"#{v}\"" }
 
-      out = vars + T.must(command).map { |arg| Utils::Shell.sh_quote(arg) } if command?
-      out.join(" ")
+      vars.concat(command.map { |arg| Utils::Shell.sh_quote(arg) })
+      vars.join(" ")
     end
 
     # Returns a `Boolean` describing if a service is timed.
@@ -425,7 +440,7 @@ def to_plist
         end
       end
 
-      if @sockets.present?
+      unless @sockets.empty?
         base[:Sockets] = {}
         @sockets.each do |name, info|
           base[:Sockets][name] = {
@@ -436,7 +451,7 @@ def to_plist
         end
       end
 
-      if @cron.present? && @run_type == RUN_TYPE_CRON
+      if !@cron.empty? && @run_type == RUN_TYPE_CRON
         base[:StartCalendarInterval] = @cron.reject { |_, value| value == "*" }
       end
 
@@ -456,8 +471,8 @@ def to_plist
     sig { returns(String) }
     def to_systemd_unit
       # command needs to be first because it initializes all other values
-      cmd = command&.map { |arg| Utils::Service.systemd_quote(arg) }
-                   &.join(" ")
+      cmd = command.map { |arg| Utils::Service.systemd_quote(arg) }
+                   .join(" ")
 
       options = []
       options << "Type=#{(@launch_only_once == true) ? "oneshot" : "simple"}"
@@ -512,7 +527,7 @@ def to_systemd_timer
     end
 
     # Prepare the service hash for inclusion in the formula API JSON.
-    sig { returns(Hash) }
+    sig { returns(T::Hash[Symbol, T.untyped]) }
     def to_hash
       name_params = {
         macos: (plist_name if plist_name != default_plist_name),
@@ -521,13 +536,13 @@ def to_hash
 
       return { name: name_params }.compact_blank if @run_params.blank?
 
-      cron_string = if @cron.present?
+      cron_string = unless @cron.empty?
         [:Minute, :Hour, :Day, :Month, :Weekday]
           .map { |key| @cron[key].to_s }
           .join(" ")
       end
 
-      sockets_var = if @sockets.present?
+      sockets_var = unless @sockets.empty?
         @sockets.transform_values { |info| "#{info[:type]}://#{info[:host]}:#{info[:port]}" }
                 .then do |sockets_hash|
                   # TODO: Remove this code when all users are running on versions of Homebrew
@@ -565,7 +580,7 @@ def to_hash
     end
 
     # Turn the service API hash values back into what is expected by the formula DSL.
-    sig { params(api_hash: Hash).returns(Hash) }
+    sig { params(api_hash: T::Hash[String, T.untyped]).returns(T::Hash[Symbol, T.untyped]) }
     def self.from_hash(api_hash)
       hash = {}
       hash[:name] = api_hash["name"].transform_keys(&:to_sym) if api_hash.key?("name")