Merge pull request #20325 from Homebrew/dug/typed-cask-audit

Enable strict typing in Cask::Audit

Author: dduugg

Library/Homebrew/cask/audit.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "cask/denylist"
@@ -19,6 +19,14 @@ class Audit
     include SystemCommand::Mixin
     include ::Utils::Curl
 
+    Error = T.type_alias do
+      {
+        message:   T.nilable(String),
+        location:  T.nilable(Homebrew::SourceLocation),
+        corrected: T::Boolean,
+      }
+    end
+
     sig { returns(Cask) }
     attr_reader :cask
 
@@ -47,6 +55,7 @@ def initialize(
       download ||= online || signing
 
       @cask = cask
+      @download = T.let(nil, T.nilable(Download))
       @download = Download.new(cask, quarantine:) if download
       @online = online
       @strict = strict
@@ -88,8 +97,9 @@ def run!
       self
     end
 
+    sig { returns(T::Array[Error]) }
     def errors
-      @errors ||= []
+      @errors ||= T.let([], T.nilable(T::Array[Error]))
     end
 
     sig { returns(T::Boolean) }
@@ -113,9 +123,10 @@ def add_error(message, location: nil, strict_only: false)
       # Only raise non-critical audits if the user specified `--strict`.
       return if strict_only && !@strict
 
-      errors << ({ message:, location:, corrected: false })
+      errors << { message:, location:, corrected: false }
     end
 
+    sig { returns(T.nilable(String)) }
     def result
       Formatter.error("failed") if errors?
     end
@@ -346,6 +357,7 @@ def audit_download_url_format
                 location: url.location
     end
 
+    sig { void }
     def audit_download_url_is_osdn
       return if (url = cask.url).nil?
       return if block_url_offline?
@@ -541,8 +553,15 @@ def audit_signing
       end
     end
 
-    sig { void }
-    def extract_artifacts
+    sig {
+      params(
+        _block: T.nilable(T.proc.params(
+          arg0: T::Array[T.any(Artifact::Pkg, Artifact::Relocated)],
+          arg1: Pathname,
+        ).void),
+      ).void
+    }
+    def extract_artifacts(&_block)
       return unless online?
       return if (download = self.download).nil?
 
@@ -557,7 +576,7 @@ def extract_artifacts
 
       return if artifacts.empty?
 
-      @tmpdir ||= Pathname(Dir.mktmpdir("cask-audit", HOMEBREW_TEMP))
+      @tmpdir ||= T.let(Pathname(Dir.mktmpdir("cask-audit", HOMEBREW_TEMP)), T.nilable(Pathname))
 
       # Clean up tmp dir when @tmpdir object is destroyed
       ObjectSpace.define_finalizer(
@@ -606,7 +625,8 @@ def extract_artifacts
                       .extract_nestedly(to: @tmpdir, verbose: false)
       end
 
-      @artifacts_extracted = true # Set the flag to indicate that extraction has occurred.
+      # Set the flag to indicate that extraction has occurred.
+      @artifacts_extracted = T.let(true, T.nilable(TrueClass))
 
       # Yield the artifacts and temp directory to the block if provided.
       yield artifacts, @tmpdir if block_given?
@@ -626,8 +646,8 @@ def audit_rosetta
       extract_artifacts do |artifacts, tmpdir|
         is_container = artifacts.any? { |a| a.is_a?(Artifact::App) || a.is_a?(Artifact::Pkg) }
 
-        artifacts.filter { |a| a.is_a?(Artifact::App) || a.is_a?(Artifact::Binary) }
-                 .each do |artifact|
+        artifacts.each do |artifact|
+          next if !artifact.is_a?(Artifact::App) && !artifact.is_a?(Artifact::Binary)
           next if artifact.is_a?(Artifact::Binary) && is_container
 
           path = tmpdir/artifact.source.relative_path_from(cask.staged_path)
@@ -644,10 +664,10 @@ def audit_rosetta
 
             system_command("lipo", args: ["-archs", main_binary], print_stderr: false)
           when Artifact::Binary
-            binary_path = path.to_s.gsub(cask.appdir, tmpdir)
+            binary_path = path.to_s.gsub(cask.appdir, tmpdir.to_s)
             system_command("lipo", args: ["-archs", binary_path], print_stderr: true)
           else
-            add_error "Unknown artifact type: #{artifact.class}", location: url.location
+            T.absurd(artifact)
           end
 
           # binary stanza can contain shell scripts, so we just continue if lipo fails.
@@ -795,7 +815,7 @@ def cask_bundle_min_os
       return unless online?
 
       min_os = T.let(nil, T.untyped)
-      @staged_path ||= cask.staged_path
+      @staged_path ||= T.let(cask.staged_path, T.nilable(Pathname))
 
       extract_artifacts do |artifacts, tmpdir|
         artifacts.each do |artifact|
@@ -1104,15 +1124,15 @@ def bad_sourceforge_url?
 
     sig { returns(T::Boolean) }
     def bad_osdn_url?
-      domain.match?(%r{^(?:\w+\.)*osdn\.jp(?=/|$)})
+      T.must(domain).match?(%r{^(?:\w+\.)*osdn\.jp(?=/|$)})
     end
 
-    # sig { returns(String) }
+    sig { returns(T.nilable(String)) }
     def homepage
       URI(cask.homepage.to_s).host
     end
 
-    # sig { returns(String) }
+    sig { returns(T.nilable(String)) }
     def domain
       URI(cask.url.to_s).host
     end
@@ -1127,24 +1147,25 @@ def url_match_homepage?
         host_uri.host
       end
 
-      return false if homepage.blank?
+      home = homepage
+      return false if home.blank?
 
-      home = homepage.downcase
+      home.downcase!
       if (split_host = T.must(host).split(".")).length >= 3
         host = T.must(split_host[-2..]).join(".")
       end
-      if (split_home = homepage.split(".")).length >= 3
-        home = split_home[-2..].join(".")
+      if (split_home = home.split(".")).length >= 3
+        home = T.must(split_home[-2..]).join(".")
       end
       host == home
     end
 
-    # sig { params(url: String).returns(String) }
+    sig { params(url: String).returns(String) }
     def strip_url_scheme(url)
       url.sub(%r{^[^:/]+://(www\.)?}, "")
     end
 
-    # sig { returns(String) }
+    sig { returns(String) }
     def url_from_verified
       strip_url_scheme(T.must(cask.url).verified)
     end
@@ -1154,8 +1175,10 @@ def verified_matches_url?
       url_domain, url_path = strip_url_scheme(cask.url.to_s).split("/", 2)
       verified_domain, verified_path = url_from_verified.split("/", 2)
 
-      (url_domain == verified_domain || (verified_domain && url_domain&.end_with?(".#{verified_domain}"))) &&
-        (!verified_path || url_path&.start_with?(verified_path))
+      domains_match = (url_domain == verified_domain) ||
+                      (verified_domain && url_domain&.end_with?(".#{verified_domain}"))
+      paths_match = !verified_path || url_path&.start_with?(verified_path)
+      (domains_match && paths_match) || false
     end
 
     sig { returns(T::Boolean) }
@@ -1177,10 +1200,10 @@ def block_url_offline?
 
     sig { returns(Tap) }
     def core_tap
-      @core_tap ||= CoreTap.instance
+      @core_tap ||= T.let(CoreTap.instance, T.nilable(Tap))
     end
 
-    # sig { returns(T::Array[String]) }
+    sig { returns(T::Array[String]) }
     def core_formula_names
       core_tap.formula_names
     end

Library/Homebrew/cli/parser.rb

@@ -445,7 +445,7 @@ def generate_help_text
                  .gsub(/`(.*?)`/m, "#{Tty.bold}\\1#{Tty.reset}")
                  .gsub(%r{<([^\s]+?://[^\s]+?)>}) { |url| Formatter.url(url) }
                  .gsub(/\*(.*?)\*|<(.*?)>/m) do |underlined|
-                   underlined[1...-1].gsub(/^(\s*)(.*?)$/, "\\1#{Tty.underline}\\2#{Tty.reset}")
+                   T.must(underlined[1...-1]).gsub(/^(\s*)(.*?)$/, "\\1#{Tty.underline}\\2#{Tty.reset}")
                  end
       end
 

Library/Homebrew/utils/formatter.rb

@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 require "utils/tty"
@@ -10,46 +10,54 @@ module Formatter
   COMMAND_DESC_WIDTH = 80
   OPTION_DESC_WIDTH = 45
 
+  sig { params(string: String, color: T.nilable(Symbol)).returns(String) }
   def self.arrow(string, color: nil)
     prefix("==>", string, color)
   end
 
   # Format a string as headline.
   #
   # @api internal
+  sig { params(string: String, color: T.nilable(Symbol)).returns(String) }
   def self.headline(string, color: nil)
     arrow("#{Tty.bold}#{string}#{Tty.reset}", color:)
   end
 
+  sig { params(string: Object).returns(String) }
   def self.identifier(string)
     "#{Tty.green}#{string}#{Tty.default}"
   end
 
+  sig { params(string: String).returns(String) }
   def self.bold(string)
     "#{Tty.bold}#{string}#{Tty.reset}"
   end
 
+  sig { params(string: String).returns(String) }
   def self.option(string)
     bold(string)
   end
 
   # Format a string as success, with an optional label.
   #
   # @api internal
+  sig { params(string: String, label: T.nilable(String)).returns(String) }
   def self.success(string, label: nil)
     label(label, string, :green)
   end
 
   # Format a string as warning, with an optional label.
   #
   # @api internal
+  sig { params(string: T.any(String, Exception), label: T.nilable(String)).returns(String) }
   def self.warning(string, label: nil)
     label(label, string, :yellow)
   end
 
   # Format a string as error, with an optional label.
   #
   # @api internal
+  sig { params(string: T.any(String, Exception), label: T.nilable(String)).returns(String) }
   def self.error(string, label: nil)
     label(label, string, :red)
   end
@@ -80,6 +88,7 @@ def self.truncate(string, max: 30, omission: "...")
   # so we always wrap one word before an option.
   # @see https://github.com/Homebrew/brew/pull/12672
   # @see https://macromates.com/blog/2006/wrapping-text-with-regular-expressions/
+  sig { params(string: String, width: Integer).returns(String) }
   def self.format_help_text(string, width: 172)
     desc = OPTION_DESC_WIDTH
     indent = width - desc
@@ -90,21 +99,26 @@ def self.format_help_text(string, width: 172)
           .gsub(/(.{1,#{width}})( +|$)(?!-)\n?/, "\\1\n")
   end
 
+  sig { params(string: T.any(NilClass, String, URI::Generic)).returns(String) }
   def self.url(string)
     "#{Tty.underline}#{string}#{Tty.no_underline}"
   end
 
+  sig { params(label: T.nilable(String), string: T.any(String, Exception), color: Symbol).returns(String) }
   def self.label(label, string, color)
     label = "#{label}:" unless label.nil?
     prefix(label, string, color)
   end
   private_class_method :label
 
+  sig {
+    params(prefix: T.nilable(String), string: T.any(String, Exception), color: T.nilable(Symbol)).returns(String)
+  }
   def self.prefix(prefix, string, color)
     if prefix.nil? && color.nil?
-      string
+      string.to_s
     elsif prefix.nil?
-      "#{Tty.send(color)}#{string}#{Tty.reset}"
+      "#{Tty.send(T.must(color))}#{string}#{Tty.reset}"
     elsif color.nil?
       "#{prefix} #{string}"
     else
@@ -116,7 +130,8 @@ def self.prefix(prefix, string, color)
   # Layout objects in columns that fit the current terminal width.
   #
   # @api internal
-  def self.columns(*objects, gap_size: 2)
+  sig { params(objects: T::Array[String], gap_size: Integer).returns(String) }
+  def self.columns(objects, gap_size: 2)
     objects = objects.flatten.map(&:to_s)
 
     fallback = proc do
@@ -145,7 +160,7 @@ def self.columns(*objects, gap_size: 2)
       item_indices_for_row = T.cast(row_index.step(objects.size - 1, rows).to_a, T::Array[Integer])
 
       first_n = T.must(item_indices_for_row[0...-1]).map do |index|
-        objects[index] + "".rjust(col_width - object_lengths.fetch(index))
+        objects.fetch(index) + "".rjust(col_width - object_lengths.fetch(index))
       end
 
       # don't add trailing whitespace to last column