Merge pull request #11842 from samford/livecheck/refactor-strategy-block-handling

Refactor livecheck strategy block handling

Library/Homebrew/livecheck/livecheck.rb

@@ -56,9 +56,11 @@ module Homebrew
 
       # Cache demodulized strategy names, to avoid repeating this work
       @livecheck_strategy_names = {}
-      Strategy.constants.sort.each do |strategy_symbol|
+      Strategy.constants.sort.each do |const_symbol|
-        strategy = Strategy.const_get(strategy_symbol)
+        constant = Strategy.const_get(const_symbol)
-        @livecheck_strategy_names[strategy] = strategy.name.demodulize
+        next unless constant.is_a?(Class)
+
+        @livecheck_strategy_names[constant] = T.must(constant.name).demodulize
       end
       @livecheck_strategy_names.freeze
     end

Library/Homebrew/livecheck/strategy.rb

@@ -14,7 +14,7 @@ module Homebrew
 
       module_function
 
-      # Strategy priorities informally range from 1 to 10, where 10 is the
+      # {Strategy} priorities informally range from 1 to 10, where 10 is the
       # highest priority. 5 is the default priority because it's roughly in
       # the middle of this range. Strategies with a priority of 0 (or lower)
       # are ignored.
@@ -32,10 +32,10 @@ module Homebrew
       # The `curl` process will sometimes hang indefinitely (despite setting
       # the `--max-time` argument) and it needs to be quit for livecheck to
       # continue. This value is used to set the `timeout` argument on
-      # `Utils::Curl` method calls in `Strategy`.
+      # `Utils::Curl` method calls in {Strategy}.
       CURL_PROCESS_TIMEOUT = CURL_MAX_TIME + 5
 
-      # Baseline `curl` arguments used in `Strategy` methods.
+      # Baseline `curl` arguments used in {Strategy} methods.
       DEFAULT_CURL_ARGS = [
         # Follow redirections to handle mirrors, relocations, etc.
         "--location",
@@ -60,7 +60,7 @@ module Homebrew
         "--include",
       ] + DEFAULT_CURL_ARGS).freeze
 
-      # Baseline `curl` options used in `Strategy` methods.
+      # Baseline `curl` options used in {Strategy} methods.
       DEFAULT_CURL_OPTIONS = {
         print_stdout: false,
         print_stderr: false,
@@ -75,52 +75,66 @@ module Homebrew
       # In rare cases, this can also be a double newline (`\n\n`).
       HTTP_HEAD_BODY_SEPARATOR = "\r\n\r\n"
 
-      # The `#strategies` method expects `Strategy` constants to be strategies,
+      # An error message to use when a `strategy` block returns a value of
-      # so constants we create need to be private for this to work properly.
+      # an inappropriate type.
-      private_constant :DEFAULT_PRIORITY, :CURL_CONNECT_TIMEOUT, :CURL_MAX_TIME,
+      INVALID_BLOCK_RETURN_VALUE_MSG = "Return value of a strategy block must be a string or array of strings."
-                       :CURL_PROCESS_TIMEOUT, :DEFAULT_CURL_ARGS,
-                       :PAGE_HEADERS_CURL_ARGS, :PAGE_CONTENT_CURL_ARGS,
-                       :DEFAULT_CURL_OPTIONS, :HTTP_HEAD_BODY_SEPARATOR
 
       # Creates and/or returns a `@strategies` `Hash`, which maps a snake
       # case strategy name symbol (e.g. `:page_match`) to the associated
-      # {Strategy}.
+      # strategy.
       #
       # At present, this should only be called after tap strategies have been
       # loaded, otherwise livecheck won't be able to use them.
       # @return [Hash]
+      sig { returns(T::Hash[Symbol, T.untyped]) }
       def strategies
         return @strategies if defined? @strategies
 
         @strategies = {}
-        constants.sort.each do |strategy_symbol|
+        Strategy.constants.sort.each do |const_symbol|
-          key = strategy_symbol.to_s.underscore.to_sym
+          constant = Strategy.const_get(const_symbol)
-          strategy = const_get(strategy_symbol)
+          next unless constant.is_a?(Class)
-          @strategies[key] = strategy
+
+          key = const_symbol.to_s.underscore.to_sym
+          @strategies[key] = constant
         end
         @strategies
       end
       private_class_method :strategies
 
-      # Returns the {Strategy} that corresponds to the provided `Symbol` (or
+      # Returns the strategy that corresponds to the provided `Symbol` (or
-      # `nil` if there is no matching {Strategy}).
+      # `nil` if there is no matching strategy).
       #
-      # @param symbol [Symbol] the strategy name in snake case as a `Symbol`
+      # @param symbol [Symbol, nil] the strategy name in snake case as a
-      #   (e.g. `:page_match`)
+      #   `Symbol` (e.g. `:page_match`)
-      # @return [Strategy, nil]
+      # @return [Class, nil]
+      sig { params(symbol: T.nilable(Symbol)).returns(T.nilable(T.untyped)) }
       def from_symbol(symbol)
-        strategies[symbol]
+        strategies[symbol] if symbol.present?
       end
 
       # Returns an array of strategies that apply to the provided URL.
       #
       # @param url [String] the URL to check for matching strategies
-      # @param livecheck_strategy [Symbol] a {Strategy} symbol from the
+      # @param livecheck_strategy [Symbol] a strategy symbol from the
+      #   `livecheck` block
+      # @param url_provided [Boolean] whether a url is provided in the
       #   `livecheck` block
       # @param regex_provided [Boolean] whether a regex is provided in the
       #   `livecheck` block
+      # @param block_provided [Boolean] whether a `strategy` block is provided
+      #   in the `livecheck` block
       # @return [Array]
-      def from_url(url, livecheck_strategy: nil, url_provided: nil, regex_provided: nil, block_provided: nil)
+      sig {
+        params(
+          url:                String,
+          livecheck_strategy: T.nilable(Symbol),
+          url_provided:       T::Boolean,
+          regex_provided:     T::Boolean,
+          block_provided:     T::Boolean,
+        ).returns(T::Array[T.untyped])
+      }
+      def from_url(url, livecheck_strategy: nil, url_provided: false, regex_provided: false, block_provided: false)
         usable_strategies = strategies.values.select do |strategy|
           if strategy == PageMatch
             # Only treat the `PageMatch` strategy as usable if a regex is
@@ -144,6 +158,13 @@ module Homebrew
         end
       end
 
+      # Collects HTTP response headers, starting with the provided URL.
+      # Redirections will be followed and all the response headers are
+      # collected into an array of hashes.
+      #
+      # @param url [String] the URL to fetch
+      # @return [Array]
+      sig { params(url: String).returns(T::Array[T::Hash[String, String]]) }
       def self.page_headers(url)
         headers = []
 
@@ -223,6 +244,25 @@ module Homebrew
           messages: [error_msg.presence || "cURL failed without an error"],
         }
       end
+
+      # Handles the return value from a `strategy` block in a `livecheck`
+      # block.
+      #
+      # @param value [] the return value from a `strategy` block
+      # @return [Array]
+      sig { params(value: T.untyped).returns(T::Array[String]) }
+      def self.handle_block_return(value)
+        case value
+        when String
+          [value]
+        when Array
+          value.compact.uniq
+        when nil
+          []
+        else
+          raise TypeError, INVALID_BLOCK_RETURN_VALUE_MSG
+        end
+      end
     end
   end
 end

Library/Homebrew/livecheck/strategy/apache.rb

@@ -37,6 +37,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/bitbucket.rb

@@ -44,6 +44,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/cpan.rb

@@ -35,6 +35,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/electron_builder.rb

@@ -7,6 +7,9 @@ module Homebrew
       # The {ElectronBuilder} strategy fetches content at a URL and parses
       # it as an electron-builder appcast in YAML format.
       #
+      # This strategy is not applied automatically and it's necessary to use
+      # `strategy :electron_builder` in a `livecheck` block to apply it.
+      #
       # @api private
       class ElectronBuilder
         extend T::Sig
@@ -14,8 +17,7 @@ module Homebrew
         NICE_NAME = "electron-builder"
 
         # A priority of zero causes livecheck to skip the strategy. We do this
-        # for {ElectronBuilder} so we can selectively apply the strategy using
+        # for {ElectronBuilder} so we can selectively apply it when appropriate.
-        # `strategy :electron_builder` in a `livecheck` block.
         PRIORITY = 0
 
         # The `Regexp` used to determine if the strategy applies to the URL.
@@ -30,40 +32,34 @@ module Homebrew
           URL_MATCH_REGEX.match?(url)
         end
 
-        # Extract version information from page content.
+        # Parses YAML text and identifies versions in it.
         #
-        # @param content [String] the content to check
+        # @param content [String] the YAML text to parse and check
-        # @return [String]
+        # @return [Array]
         sig {
           params(
             content: String,
-            block:   T.nilable(T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.nilable(String))),
+            block:   T.nilable(
-          ).returns(T.nilable(String))
+              T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.any(String, T::Array[String], NilClass)),
+            ),
+          ).returns(T::Array[String])
         }
-        def self.version_from_content(content, &block)
+        def self.versions_from_content(content, &block)
           require "yaml"
 
           yaml = YAML.safe_load(content)
-          return if yaml.blank?
+          return [] if yaml.blank?
 
-          if block
+          return Strategy.handle_block_return(block.call(yaml)) if block
-            case (value = block.call(yaml))
-            when String
-              return value
-            when nil
-              return
-            else
-              raise TypeError, "Return value of `strategy :electron_builder` block must be a string."
-            end
-          end
 
-          yaml["version"]
+          version = yaml["version"]
+          version.present? ? [version] : []
         end
 
-        # Checks the content at the URL for new versions.
+        # Checks the YAML content at the URL for new versions.
         #
         # @param url [String] the URL of the content to check
-        # @param regex [Regexp] a regex used for matching versions in content
+        # @param regex [Regexp, nil] a regex used for matching versions
         # @return [Hash]
         sig {
           params(
@@ -81,8 +77,9 @@ module Homebrew
           match_data.merge!(Strategy.page_content(url))
           content = match_data.delete(:content)
 
-          version = version_from_content(content, &block)
+          versions_from_content(content, &block).each do |version_text|
-          match_data[:matches][version] = Version.new(version) if version
+            match_data[:matches][version_text] = Version.new(version_text)
+          end
 
           match_data
         end

Library/Homebrew/livecheck/strategy/extract_plist.rb

@@ -3,31 +3,35 @@
 
 require "bundle_version"
 require "unversioned_cask_checker"
-require_relative "page_match"
 
 module Homebrew
   module Livecheck
     module Strategy
-      # The {ExtractPlist} strategy downloads the file at a URL and
+      # The {ExtractPlist} strategy downloads the file at a URL and extracts
-      # extracts versions from contained `.plist` files.
+      # versions from contained `.plist` files using {UnversionedCaskChecker}.
+      #
+      # In practice, this strategy operates by downloading very large files,
+      # so it's both slow and data-intensive. As such, the {ExtractPlist}
+      # strategy should only be used as an absolute last resort.
+      #
+      # This strategy is not applied automatically and it's necessary to use
+      # `strategy :extract_plist` in a `livecheck` block to apply it.
       #
       # @api private
       class ExtractPlist
         extend T::Sig
 
-        # A priority of zero causes livecheck to skip the strategy. We only
+        # A priority of zero causes livecheck to skip the strategy. We do this
-        # apply {ExtractPlist} using `strategy :extract_plist` in a `livecheck` block,
+        # for {ExtractPlist} so we can selectively apply it when appropriate.
-        # as we can't automatically determine when this can be successfully
-        # applied to a URL without fetching the content.
         PRIORITY = 0
 
         # The `Regexp` used to determine if the strategy applies to the URL.
         URL_MATCH_REGEX = %r{^https?://}i.freeze
 
         # Whether the strategy can be applied to the provided URL.
-        # The strategy will technically match any HTTP URL but is
+        #
-        # only usable with a `livecheck` block containing a regex
+        # @param url [String] the URL to match against
-        # or block.
+        # @return [Boolean]
         sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
@@ -50,13 +54,37 @@ module Homebrew
           delegate short_version: :bundle_version
         end
 
-        # Checks the content at the URL for new versions.
+        # Identify versions from `Item`s produced using
+        # {UnversionedCaskChecker} version information.
+        #
+        # @param items [Hash] a hash of `Item`s containing version information
+        # @return [Array]
+        sig {
+          params(
+            items: T::Hash[String, Item],
+            block: T.nilable(
+              T.proc.params(arg0: T::Hash[String, Item]).returns(T.any(String, T::Array[String], NilClass)),
+            ),
+          ).returns(T::Array[String])
+        }
+        def self.versions_from_items(items, &block)
+          return Strategy.handle_block_return(block.call(items)) if block
+
+          items.map do |_key, item|
+            item.bundle_version.nice_version
+          end.compact.uniq
+        end
+
+        # Uses {UnversionedCaskChecker} on the provided cask to identify
+        # versions from `plist` files.
         sig {
           params(
             url:   String,
             regex: T.nilable(Regexp),
             cask:  Cask::Cask,
-            block: T.nilable(T.proc.params(arg0: T::Hash[String, Item]).returns(T.nilable(String))),
+            block: T.nilable(
+              T.proc.params(arg0: T::Hash[String, Item]).returns(T.any(String, T::Array[String], NilClass)),
+            ),
           ).returns(T::Hash[Symbol, T.untyped])
         }
         def self.find_versions(url, regex, cask:, &block)
@@ -66,22 +94,10 @@ module Homebrew
           match_data = { matches: {}, regex: regex, url: url }
 
           unversioned_cask_checker = UnversionedCaskChecker.new(cask)
-          versions = unversioned_cask_checker.all_versions.transform_values { |v| Item.new(bundle_version: v) }
+          items = unversioned_cask_checker.all_versions.transform_values { |v| Item.new(bundle_version: v) }
 
-          if block
+          versions_from_items(items, &block).each do |version_text|
-            case (value = block.call(versions))
+            match_data[:matches][version_text] = Version.new(version_text)
-            when String
-              match_data[:matches][value] = Version.new(value)
-            when nil
-              return match_data
-            else
-              raise TypeError, "Return value of `strategy :extract_plist` block must be a string."
-            end
-          elsif versions.any?
-            versions.each_value do |item|
-              version = item.bundle_version.nice_version
-              match_data[:matches][version] = Version.new(version)
-            end
           end
 
           match_data

Library/Homebrew/livecheck/strategy/git.rb

@@ -30,6 +30,19 @@ module Homebrew
         # lowest to highest).
         PRIORITY = 8
 
+        # The default regex used to naively identify numeric versions from tags
+        # when a regex isn't provided.
+        DEFAULT_REGEX = /\D*(.+)/.freeze
+
+        # Whether the strategy can be applied to the provided URL.
+        #
+        # @param url [String] the URL to match against
+        # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
+        def self.match?(url)
+          (DownloadStrategyDetector.detect(url) <= GitDownloadStrategy) == true
+        end
+
         # Fetches a remote Git repository's tags using `git ls-remote --tags`
         # and parses the command's output. If a regex is provided, it will be
         # used to filter out any tags that don't match it.
@@ -37,6 +50,7 @@ module Homebrew
         # @param url [String] the URL of the Git repository to check
         # @param regex [Regexp] the regex to use for filtering tags
         # @return [Hash]
+        sig { params(url: String, regex: T.nilable(Regexp)).returns(T::Hash[Symbol, T.untyped]) }
         def self.tag_info(url, regex = nil)
           # Open3#capture3 is used here because we need to capture stderr
           # output and handle it in an appropriate manner. Alternatives like
@@ -61,12 +75,42 @@ module Homebrew
           tags_data
         end
 
-        # Whether the strategy can be applied to the provided URL.
+        # Identify versions from tag strings using a provided regex or the
+        # `DEFAULT_REGEX`. The regex is expected to use a capture group around
+        # the version text.
         #
-        # @param url [String] the URL to match against
+        # @param tags [Array] the tags to identify versions from
-        # @return [Boolean]
+        # @param regex [Regexp, nil] a regex to identify versions
-        def self.match?(url)
+        # @return [Array]
-          (DownloadStrategyDetector.detect(url) <= GitDownloadStrategy) == true
+        sig {
+          params(
+            tags:  T::Array[String],
+            regex: T.nilable(Regexp),
+            block: T.nilable(
+              T.proc.params(arg0: T::Array[String], arg1: T.nilable(Regexp))
+                .returns(T.any(String, T::Array[String], NilClass)),
+            ),
+          ).returns(T::Array[String])
+        }
+        def self.versions_from_tags(tags, regex = nil, &block)
+          return Strategy.handle_block_return(block.call(tags, regex || DEFAULT_REGEX)) if block
+
+          tags_only_debian = tags.all? { |tag| tag.start_with?("debian/") }
+
+          tags.map do |tag|
+            # Skip tag if it has a 'debian/' prefix and upstream does not do
+            # only 'debian/' prefixed tags
+            next if tag =~ %r{^debian/} && !tags_only_debian
+
+            if regex
+              # Use the first capture group (the version)
+              tag.scan(regex).first&.first
+            else
+              # Remove non-digits from the start of the tag and use that as the
+              # version text
+              tag[DEFAULT_REGEX, 1]
+            end
+          end.compact.uniq
         end
 
         # Checks the Git tags for new versions. When a regex isn't provided,
@@ -74,7 +118,7 @@ module Homebrew
         # strings and parses the remaining text as a {Version}.
         #
         # @param url [String] the URL of the Git repository to check
-        # @param regex [Regexp] the regex to use for matching versions
+        # @param regex [Regexp, nil] a regex used for matching versions
         # @return [Hash]
         sig {
           params(
@@ -82,54 +126,26 @@ module Homebrew
             regex: T.nilable(Regexp),
             cask:  T.nilable(Cask::Cask),
             block: T.nilable(
-              T.proc.params(arg0: T::Array[String]).returns(T.any(String, T::Array[String], NilClass)),
+              T.proc.params(arg0: T::Array[String], arg1: T.nilable(Regexp))
+                .returns(T.any(String, T::Array[String], NilClass)),
             ),
           ).returns(T::Hash[Symbol, T.untyped])
         }
-        def self.find_versions(url, regex, cask: nil, &block)
+        def self.find_versions(url, regex = nil, cask: nil, &block)
           match_data = { matches: {}, regex: regex, url: url }
 
           tags_data = tag_info(url, regex)
+          tags = tags_data[:tags]
 
           if tags_data.key?(:messages)
             match_data[:messages] = tags_data[:messages]
-            return match_data if tags_data[:tags].blank?
+            return match_data if tags.blank?
           end
 
-          tags_only_debian = tags_data[:tags].all? { |tag| tag.start_with?("debian/") }
+          versions_from_tags(tags, regex, &block).each do |version_text|
-
+            match_data[:matches][version_text] = Version.new(version_text)
-          if block
-            case (value = block.call(tags_data[:tags], regex))
-            when String
-              match_data[:matches][value] = Version.new(value)
-            when Array
-              value.compact.uniq.each do |tag|
-                match_data[:matches][tag] = Version.new(tag)
-              end
-            when nil
-              return match_data
-            else
-              raise TypeError, "Return value of `strategy :git` block must be a string or array of strings."
-            end
-
-            return match_data
-          end
-
-          tags_data[:tags].each do |tag|
-            # Skip tag if it has a 'debian/' prefix and upstream does not do
-            # only 'debian/' prefixed tags
-            next if tag =~ %r{^debian/} && !tags_only_debian
-
-            captures = regex.is_a?(Regexp) ? tag.scan(regex) : []
-            tag_cleaned = if captures[0].is_a?(Array)
-              captures[0][0] # Use the first capture group (the version)
-            else
-              tag[/\D*(.*)/, 1] # Remove non-digits from the start of the tag
-            end
-
-            match_data[:matches][tag] = Version.new(tag_cleaned)
           rescue TypeError
-            nil
+            next
           end
 
           match_data

Library/Homebrew/livecheck/strategy/github_latest.rb

@@ -52,6 +52,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/gnome.rb

@@ -40,6 +40,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/gnu.rb

@@ -44,6 +44,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url) && url.exclude?("savannah.")
         end

Library/Homebrew/livecheck/strategy/hackage.rb

@@ -37,6 +37,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/header_match.rb

@@ -1,24 +1,23 @@
-# typed: false
+# typed: true
 # frozen_string_literal: true
 
-require_relative "page_match"
-
 module Homebrew
   module Livecheck
     module Strategy
       # The {HeaderMatch} strategy follows all URL redirections and scans
       # the resulting headers for matching text using the provided regex.
       #
+      # This strategy is not applied automatically and it's necessary to use
+      # `strategy :header_match` in a `livecheck` block to apply it.
+      #
       # @api private
       class HeaderMatch
         extend T::Sig
 
         NICE_NAME = "Header match"
 
-        # A priority of zero causes livecheck to skip the strategy. We only
+        # A priority of zero causes livecheck to skip the strategy. We do this
-        # apply {HeaderMatch} using `strategy :header_match` in a `livecheck`
+        # for {HeaderMatch} so we can selectively apply it when appropriate.
-        # block, as we can't automatically determine when this can be
-        # successfully applied to a URL.
         PRIORITY = 0
 
         # The `Regexp` used to determine if the strategy applies to the URL.
@@ -28,22 +27,61 @@ module Homebrew
         DEFAULT_HEADERS_TO_CHECK = ["content-disposition", "location"].freeze
 
         # Whether the strategy can be applied to the provided URL.
-        # The strategy will technically match any HTTP URL but is
+        #
-        # only usable with a `livecheck` block containing a regex
+        # @param url [String] the URL to match against
-        # or block.
+        # @return [Boolean]
         sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end
 
+        # Identify versions from HTTP headers.
+        #
+        # @param headers [Hash] a hash of HTTP headers to check for versions
+        # @param regex [Regexp, nil] a regex for matching versions
+        # @return [Array]
+        sig {
+          params(
+            headers: T::Hash[String, String],
+            regex:   T.nilable(Regexp),
+            block:   T.nilable(
+              T.proc.params(
+                arg0: T::Hash[String, String],
+                arg1: T.nilable(Regexp),
+              ).returns(T.any(String, T::Array[String], NilClass)),
+            ),
+          ).returns(T::Array[String])
+        }
+        def self.versions_from_headers(headers, regex = nil, &block)
+          return Strategy.handle_block_return(block.call(headers, regex)) if block
+
+          DEFAULT_HEADERS_TO_CHECK.map do |header_name|
+            header_value = headers[header_name]
+            next if header_value.blank?
+
+            if regex
+              header_value[regex, 1]
+            else
+              v = Version.parse(header_value, detected_from_url: true)
+              v.null? ? nil : v.to_s
+            end
+          end.compact.uniq
+        end
+
         # Checks the final URL for new versions after following all redirections,
         # using the provided regex for matching.
+        #
+        # @param url [String] the URL to fetch
+        # @param regex [Regexp, nil] a regex used for matching versions
+        # @return [Hash]
         sig {
           params(
             url:   String,
             regex: T.nilable(Regexp),
             cask:  T.nilable(Cask::Cask),
-            block: T.nilable(T.proc.params(arg0: T::Hash[String, String]).returns(T.nilable(String))),
+            block: T.nilable(
+              T.proc.params(arg0: T::Hash[String, String], arg1: T.nilable(Regexp)).returns(T.nilable(String)),
+            ),
           ).returns(T::Hash[Symbol, T.untyped])
         }
         def self.find_versions(url, regex, cask: nil, &block)
@@ -53,36 +91,12 @@ module Homebrew
 
           # Merge the headers from all responses into one hash
           merged_headers = headers.reduce(&:merge)
+          return match_data if merged_headers.blank?
 
-          version = if block
+          versions_from_headers(merged_headers, regex, &block).each do |version_text|
-            case (value = block.call(merged_headers, regex))
+            match_data[:matches][version_text] = Version.new(version_text)
-            when String
-              value
-            when nil
-              return match_data
-            else
-              raise TypeError, "Return value of `strategy :header_match` block must be a string."
-            end
-          else
-            value = nil
-            DEFAULT_HEADERS_TO_CHECK.each do |header_name|
-              header_value = merged_headers[header_name]
-              next if header_value.blank?
-
-              if regex
-                value = header_value[regex, 1]
-              else
-                v = Version.parse(header_value, detected_from_url: true)
-                value = v.to_s unless v.null?
-              end
-              break if value.present?
-            end
-
-            value
           end
 
-          match_data[:matches][version] = Version.new(version) if version
-
           match_data
         end
       end

Library/Homebrew/livecheck/strategy/launchpad.rb

@@ -35,6 +35,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/npm.rb

@@ -31,6 +31,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/page_match.rb

@@ -11,9 +11,8 @@ module Homebrew
       # strategies apply to a given URL. Though {PageMatch} will technically
       # match any HTTP URL, the strategy also requires a regex to function.
       #
-      # The {find_versions} method is also used within other
+      # The {find_versions} method is also used within other strategies,
-      # strategies, to handle the process of identifying version text in
+      # to handle the process of identifying version text in content.
-      # content.
       #
       # @api public
       class PageMatch
@@ -22,16 +21,19 @@ module Homebrew
         NICE_NAME = "Page match"
 
         # A priority of zero causes livecheck to skip the strategy. We do this
-        # for PageMatch so we can selectively apply the strategy only when a
+        # for {PageMatch} so we can selectively apply it only when a regex is
-        # regex is provided in a `livecheck` block.
+        # provided in a `livecheck` block.
         PRIORITY = 0
 
         # The `Regexp` used to determine if the strategy applies to the URL.
         URL_MATCH_REGEX = %r{^https?://}i.freeze
 
         # Whether the strategy can be applied to the provided URL.
-        # PageMatch will technically match any HTTP URL but is only
+        # {PageMatch} will technically match any HTTP URL but is only
         # usable with a `livecheck` block containing a regex.
+        #
+        # @param url [String] the URL to match against
+        # @return [Boolean]
         sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
@@ -54,19 +56,8 @@ module Homebrew
             ),
           ).returns(T::Array[String])
         }
-        def self.page_matches(content, regex, &block)
+        def self.versions_from_content(content, regex, &block)
-          if block
+          return Strategy.handle_block_return(block.call(content, regex)) if block
-            case (value = block.call(content, regex))
-            when String
-              return [value]
-            when Array
-              return value.compact.uniq
-            when nil
-              return []
-            else
-              raise TypeError, "Return value of `strategy :page_match` block must be a string or array of strings."
-            end
-          end
 
           content.scan(regex).map do |match|
             case match
@@ -82,8 +73,8 @@ module Homebrew
         # regex for matching.
         #
         # @param url [String] the URL of the content to check
-        # @param regex [Regexp] a regex used for matching versions in content
+        # @param regex [Regexp] a regex used for matching versions
-        # @param provided_content [String] page content to use in place of
+        # @param provided_content [String, nil] page content to use in place of
         #   fetching via Strategy#page_content
         # @return [Hash]
         sig {
@@ -109,7 +100,7 @@ module Homebrew
           end
           return match_data if content.blank?
 
-          page_matches(content, regex, &block).each do |match_text|
+          versions_from_content(content, regex, &block).each do |match_text|
             match_data[:matches][match_text] = Version.new(match_text)
           end
 

Library/Homebrew/livecheck/strategy/pypi.rb

@@ -41,6 +41,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/sourceforge.rb

@@ -47,6 +47,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/livecheck/strategy/sparkle.rb

@@ -9,23 +9,24 @@ module Homebrew
       # The {Sparkle} strategy fetches content at a URL and parses
       # it as a Sparkle appcast in XML format.
       #
+      # This strategy is not applied automatically and it's necessary to use
+      # `strategy :sparkle` in a `livecheck` block to apply it.
+      #
       # @api private
       class Sparkle
         extend T::Sig
 
-        # A priority of zero causes livecheck to skip the strategy. We only
+        # A priority of zero causes livecheck to skip the strategy. We do this
-        # apply {Sparkle} using `strategy :sparkle` in a `livecheck` block,
+        # for {Sparkle} so we can selectively apply it when appropriate.
-        # as we can't automatically determine when this can be successfully
-        # applied to a URL without fetching the content.
         PRIORITY = 0
 
         # The `Regexp` used to determine if the strategy applies to the URL.
         URL_MATCH_REGEX = %r{^https?://}i.freeze
 
         # Whether the strategy can be applied to the provided URL.
-        # The strategy will technically match any HTTP URL but is
+        #
-        # only usable with a `livecheck` block containing a regex
+        # @param url [String] the URL to match against
-        # or block.
+        # @return [Boolean]
         sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
@@ -54,6 +55,10 @@ module Homebrew
           delegate short_version: :bundle_version
         end
 
+        # Identify version information from a Sparkle appcast.
+        #
+        # @param content [String] the text of the Sparkle appcast
+        # @return [Item, nil]
         sig { params(content: String).returns(T.nilable(Item)) }
         def self.item_from_content(content)
           require "rexml/document"
@@ -138,6 +143,26 @@ module Homebrew
           items.max_by { |item| [item.pub_date, item.bundle_version] }
         end
 
+        # Identify versions from content
+        #
+        # @param content [String] the content to pull version information from
+        # @return [Array]
+        sig {
+          params(
+            content: String,
+            block:   T.nilable(T.proc.params(arg0: Item).returns(T.any(String, T::Array[String], NilClass))),
+          ).returns(T::Array[String])
+        }
+        def self.versions_from_content(content, &block)
+          item = item_from_content(content)
+          return [] if item.blank?
+
+          return Strategy.handle_block_return(block.call(item)) if block
+
+          version = item.bundle_version&.nice_version
+          version.present? ? [version] : []
+        end
+
         # Checks the content at the URL for new versions.
         sig {
           params(
@@ -155,21 +180,8 @@ module Homebrew
           match_data.merge!(Strategy.page_content(url))
           content = match_data.delete(:content)
 
-          if (item = item_from_content(content))
+          versions_from_content(content, &block).each do |version_text|
-            version = if block
+            match_data[:matches][version_text] = Version.new(version_text)
-              case (value = block.call(item))
-              when String
-                value
-              when nil
-                return match_data
-              else
-                raise TypeError, "Return value of `strategy :sparkle` block must be a string."
-              end
-            else
-              item.bundle_version&.nice_version
-            end
-
-            match_data[:matches][version] = Version.new(version) if version
           end
 
           match_data

Library/Homebrew/livecheck/strategy/xorg.rb

@@ -64,6 +64,7 @@ module Homebrew
         #
         # @param url [String] the URL to match against
         # @return [Boolean]
+        sig { params(url: String).returns(T::Boolean) }
         def self.match?(url)
           URL_MATCH_REGEX.match?(url)
         end

Library/Homebrew/test/livecheck/strategy/apache_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Apache do
   let(:non_apache_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is an Apache URL" do
+    it "returns true for an Apache URL" do
       expect(apache.match?(apache_url)).to be true
     end
 
-    it "returns false if the argument provided is not an Apache URL" do
+    it "returns false for a non-Apache URL" do
       expect(apache.match?(non_apache_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/bitbucket_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Bitbucket do
   let(:non_bitbucket_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a Bitbucket URL" do
+    it "returns true for a Bitbucket URL" do
       expect(bitbucket.match?(bitbucket_url)).to be true
     end
 
-    it "returns false if the argument provided is not a Bitbucket URL" do
+    it "returns false for a non-Bitbucket URL" do
       expect(bitbucket.match?(non_bitbucket_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/cpan_spec.rb

@@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Cpan do
   let(:non_cpan_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a CPAN URL" do
+    it "returns true for a CPAN URL" do
       expect(cpan.match?(cpan_url_no_subdirectory)).to be true
       expect(cpan.match?(cpan_url_with_subdirectory)).to be true
     end
 
-    it "returns false if the argument provided is not a CPAN URL" do
+    it "returns false for a non-CPAN URL" do
       expect(cpan.match?(non_cpan_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb

@@ -1,13 +1,13 @@
 # typed: false
 # frozen_string_literal: true
 
-require "livecheck/strategy/electron_builder"
+require "livecheck/strategy"
 
 describe Homebrew::Livecheck::Strategy::ElectronBuilder do
   subject(:electron_builder) { described_class }
 
-  let(:valid_url) { "https://www.example.com/example/latest-mac.yml" }
+  let(:yaml_url) { "https://www.example.com/example/latest-mac.yml" }
-  let(:invalid_url) { "https://brew.sh/test" }
+  let(:non_yaml_url) { "https://brew.sh/test" }
 
   let(:electron_builder_yaml) {
     <<~EOS
@@ -26,42 +26,46 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do
     EOS
   }
 
+  let(:versions) { ["1.2.3"] }
+
   describe "::match?" do
-    it "returns true for any URL pointing to a YAML file" do
+    it "returns true for a YAML file URL" do
-      expect(electron_builder.match?(valid_url)).to be true
+      expect(electron_builder.match?(yaml_url)).to be true
     end
 
-    it "returns false for a URL not pointing to a YAML file" do
+    it "returns false for non-YAML URL" do
-      expect(electron_builder.match?(invalid_url)).to be false
+      expect(electron_builder.match?(non_yaml_url)).to be false
     end
   end
 
-  describe "::version_from_content" do
+  describe "::versions_from_content" do
-    let(:version_from_electron_builder_yaml) { electron_builder.version_from_content(electron_builder_yaml) }
+    it "returns an empty array if content is blank" do
-
+      expect(electron_builder.versions_from_content("")).to eq([])
-    it "returns nil if content is blank" do
-      expect(electron_builder.version_from_content("")).to be nil
     end
 
-    it "returns a version string when given YAML data" do
+    it "returns an array of version strings when given YAML text" do
-      expect(version_from_electron_builder_yaml).to be_a(String)
+      expect(electron_builder.versions_from_content(electron_builder_yaml)).to eq(versions)
     end
 
-    it "returns a version string when given YAML data and a block" do
+    it "returns an array of version strings when given YAML text and a block" do
-      version = electron_builder.version_from_content(electron_builder_yaml) do |yaml|
+      # Returning a string from block
-        yaml["version"].sub("3", "4")
+      expect(
-      end
+        electron_builder.versions_from_content(electron_builder_yaml) do |yaml|
+          yaml["version"].sub("3", "4")
+        end,
+      ).to eq(["1.2.4"])
 
-      expect(version).to eq "1.2.4"
+      # Returning an array of strings from block
+      expect(electron_builder.versions_from_content(electron_builder_yaml) { versions }).to eq(versions)
     end
 
-    it "allows a nil return from a strategy block" do
+    it "allows a nil return from a block" do
-      expect(electron_builder.version_from_content(electron_builder_yaml) { next }).to eq(nil)
+      expect(electron_builder.versions_from_content(electron_builder_yaml) { next }).to eq([])
     end
 
-    it "errors on an invalid return type from a strategy block" do
+    it "errors on an invalid return type from a block" do
-      expect { electron_builder.version_from_content(electron_builder_yaml) { 123 } }
+      expect { electron_builder.versions_from_content(electron_builder_yaml) { 123 } }
-        .to raise_error(TypeError, "Return value of `strategy :electron_builder` block must be a string.")
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
     end
   end
 end

Library/Homebrew/test/livecheck/strategy/extract_plist_spec.rb

@@ -0,0 +1,72 @@
+# typed: false
+# frozen_string_literal: true
+
+require "livecheck/strategy"
+require "bundle_version"
+
+describe Homebrew::Livecheck::Strategy::ExtractPlist do
+  subject(:extract_plist) { described_class }
+
+  let(:http_url) { "https://brew.sh/blog/" }
+  let(:non_http_url) { "ftp://brew.sh/" }
+
+  let(:items) do
+    {
+      "first"  => extract_plist::Item.new(
+        bundle_version: Homebrew::BundleVersion.new(nil, "1.2"),
+      ),
+      "second" => extract_plist::Item.new(
+        bundle_version: Homebrew::BundleVersion.new(nil, "1.2.3"),
+      ),
+    }
+  end
+
+  let(:versions) { ["1.2", "1.2.3"] }
+
+  describe "::match?" do
+    it "returns true for an HTTP URL" do
+      expect(extract_plist.match?(http_url)).to be true
+    end
+
+    it "returns false for a non-HTTP URL" do
+      expect(extract_plist.match?(non_http_url)).to be false
+    end
+  end
+
+  describe "::versions_from_items" do
+    it "returns an empty array if Items hash is empty" do
+      expect(extract_plist.versions_from_items({})).to eq([])
+    end
+
+    it "returns an array of version strings when given Items" do
+      expect(extract_plist.versions_from_items(items)).to eq(versions)
+    end
+
+    it "returns an array of version strings when given Items and a block" do
+      # Returning a string from block
+      expect(
+        extract_plist.versions_from_items(items) do |items|
+          items["first"].version
+        end,
+      ).to eq(["1.2"])
+
+      # Returning an array of strings from block
+      expect(
+        extract_plist.versions_from_items(items) do |items|
+          items.map do |_key, item|
+            item.bundle_version.nice_version
+          end
+        end,
+      ).to eq(versions)
+    end
+
+    it "allows a nil return from a block" do
+      expect(extract_plist.versions_from_items(items) { next }).to eq([])
+    end
+
+    it "errors on an invalid return type from a block" do
+      expect { extract_plist.versions_from_items(items) { 123 } }
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
+    end
+  end
+end

Library/Homebrew/test/livecheck/strategy/git_spec.rb

@@ -1,7 +1,7 @@
 # typed: false
 # frozen_string_literal: true
 
-require "livecheck/strategy/git"
+require "livecheck/strategy"
 
 describe Homebrew::Livecheck::Strategy::Git do
   subject(:git) { described_class }
@@ -9,20 +9,84 @@ describe Homebrew::Livecheck::Strategy::Git do
   let(:git_url) { "https://github.com/Homebrew/brew.git" }
   let(:non_git_url) { "https://brew.sh/test" }
 
+  let(:tags) {
+    {
+      normal:  ["brew/1.2", "brew/1.2.1", "brew/1.2.2", "brew/1.2.3", "brew/1.2.4", "1.2.5"],
+      hyphens: ["brew/1-2", "brew/1-2-1", "brew/1-2-2", "brew/1-2-3", "brew/1-2-4", "1-2-5"],
+    }
+  }
+
+  let(:regexes) {
+    {
+      standard: /^v?(\d+(?:\.\d+)+)$/i,
+      hyphens:  /^v?(\d+(?:[.-]\d+)+)$/i,
+      brew:     %r{^brew/v?(\d+(?:\.\d+)+)$}i,
+    }
+  }
+
+  let(:versions) {
+    {
+      default:        ["1.2", "1.2.1", "1.2.2", "1.2.3", "1.2.4", "1.2.5"],
+      standard_regex: ["1.2.5"],
+      brew_regex:     ["1.2", "1.2.1", "1.2.2", "1.2.3", "1.2.4"],
+    }
+  }
+
   describe "::tag_info", :needs_network do
     it "returns the Git tags for the provided remote URL that match the regex provided" do
-      expect(git.tag_info(git_url, /^v?(\d+(?:\.\d+))$/))
+      expect(git.tag_info(git_url, regexes[:standard])).not_to be_empty
-        .not_to be_empty
     end
   end
 
   describe "::match?" do
-    it "returns true if the argument provided is a Git repository" do
+    it "returns true for a Git repository URL" do
       expect(git.match?(git_url)).to be true
     end
 
-    it "returns false if the argument provided is not a Git repository" do
+    it "returns false for a non-Git URL" do
       expect(git.match?(non_git_url)).to be false
     end
   end
+
+  describe "::versions_from_tags" do
+    it "returns an empty array if tags array is empty" do
+      expect(git.versions_from_tags([])).to eq([])
+    end
+
+    it "returns an array of version strings when given tags" do
+      expect(git.versions_from_tags(tags[:normal])).to eq(versions[:default])
+      expect(git.versions_from_tags(tags[:normal], regexes[:standard])).to eq(versions[:standard_regex])
+      expect(git.versions_from_tags(tags[:normal], regexes[:brew])).to eq(versions[:brew_regex])
+    end
+
+    it "returns an array of version strings when given tags and a block" do
+      # Returning a string from block, default strategy regex
+      expect(git.versions_from_tags(tags[:normal]) { versions[:default].first }).to eq([versions[:default].first])
+
+      # Returning an array of strings from block, default strategy regex
+      expect(
+        git.versions_from_tags(tags[:hyphens]) do |tags, regex|
+          tags.map { |tag| tag[regex, 1]&.tr("-", ".") }
+        end,
+      ).to eq(versions[:default])
+
+      # Returning an array of strings from block, explicit regex
+      expect(
+        git.versions_from_tags(tags[:hyphens], regexes[:hyphens]) do |tags, regex|
+          tags.map { |tag| tag[regex, 1]&.tr("-", ".") }
+        end,
+      ).to eq(versions[:standard_regex])
+
+      expect(git.versions_from_tags(tags[:hyphens]) { "1.2.3" }).to eq(["1.2.3"])
+    end
+
+    it "allows a nil return from a block" do
+      expect(git.versions_from_tags(tags[:normal]) { next }).to eq([])
+    end
+
+    it "errors on an invalid return type from a block" do
+      expect { git.versions_from_tags(tags[:normal]) { 123 } }
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
+    end
+  end
 end

Library/Homebrew/test/livecheck/strategy/github_latest_spec.rb

@@ -14,19 +14,19 @@ describe Homebrew::Livecheck::Strategy::GithubLatest do
   let(:non_github_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a GitHub release artifact URL" do
+    it "returns true for a GitHub release artifact URL" do
       expect(github_latest.match?(github_release_artifact_url)).to be true
     end
 
-    it "returns true if the argument provided is a GitHub tag archive URL" do
+    it "returns true for a GitHub tag archive URL" do
       expect(github_latest.match?(github_tag_archive_url)).to be true
     end
 
-    it "returns true if the argument provided is a GitHub repository upload URL" do
+    it "returns true for a GitHub repository upload URL" do
       expect(github_latest.match?(github_repository_upload_url)).to be true
     end
 
-    it "returns false if the argument provided is not a GitHub URL" do
+    it "returns false for a non-GitHub URL" do
       expect(github_latest.match?(non_github_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/gnome_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Gnome do
   let(:non_gnome_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a GNOME URL" do
+    it "returns true for a GNOME URL" do
       expect(gnome.match?(gnome_url)).to be true
     end
 
-    it "returns false if the argument provided is not a GNOME URL" do
+    it "returns false for a non-GNOME URL" do
       expect(gnome.match?(non_gnome_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/gnu_spec.rb

@@ -11,15 +11,15 @@ describe Homebrew::Livecheck::Strategy::Gnu do
   let(:non_gnu_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a non-Savannah GNU URL" do
+    it "returns true for a [non-Savannah] GNU URL" do
       expect(gnu.match?(gnu_url)).to be true
     end
 
-    it "returns false if the argument provided is a Savannah GNU URL" do
+    it "returns false for a Savannah GNU URL" do
       expect(gnu.match?(savannah_gnu_url)).to be false
     end
 
-    it "returns false if the argument provided is not a GNU URL" do
+    it "returns false for a non-GNU URL (not nongnu.org)" do
       expect(gnu.match?(non_gnu_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/hackage_spec.rb

@@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Hackage do
   let(:non_hackage_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a Hackage URL" do
+    it "returns true for a Hackage URL" do
       expect(hackage.match?(hackage_url)).to be true
       expect(hackage.match?(hackage_downloads_url)).to be true
     end
 
-    it "returns false if the argument provided is not a Hackage URL" do
+    it "returns false for a non-Hackage URL" do
       expect(hackage.match?(non_hackage_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/header_match_spec.rb

@@ -1,16 +1,116 @@
 # typed: false
 # frozen_string_literal: true
 
-require "livecheck/strategy/header_match"
+require "livecheck/strategy"
 
 describe Homebrew::Livecheck::Strategy::HeaderMatch do
   subject(:header_match) { described_class }
 
-  let(:url) { "https://www.example.com/" }
+  let(:http_url) { "https://brew.sh/blog/" }
+  let(:non_http_url) { "ftp://brew.sh/" }
+
+  let(:versions) {
+    versions = {
+      content_disposition: ["1.2.3"],
+      location:            ["1.2.4"],
+    }
+    versions[:content_disposition_and_location] = versions[:content_disposition] + versions[:location]
+
+    versions
+  }
+
+  let(:headers) {
+    headers = {
+      content_disposition: {
+        "date"                => "Fri, 01 Jan 2021 01:23:45 GMT",
+        "content-type"        => "application/x-gzip",
+        "content-length"      => "120",
+        "content-disposition" => "attachment; filename=brew-#{versions[:content_disposition].first}.tar.gz",
+      },
+      location:            {
+        "date"           => "Fri, 01 Jan 2021 01:23:45 GMT",
+        "content-type"   => "text/html; charset=utf-8",
+        "location"       => "https://github.com/Homebrew/brew/releases/tag/#{versions[:location].first}",
+        "content-length" => "117",
+      },
+    }
+    headers[:content_disposition_and_location] = headers[:content_disposition].merge(headers[:location])
+
+    headers
+  }
+
+  let(:regexes) {
+    {
+      archive: /filename=brew[._-]v?(\d+(?:\.\d+)+)\.t/i,
+      latest:  %r{.*?/tag/v?(\d+(?:\.\d+)+)$}i,
+      loose:   /v?(\d+(?:\.\d+)+)/i,
+    }
+  }
 
   describe "::match?" do
-    it "returns true for any URL" do
+    it "returns true for an HTTP URL" do
-      expect(header_match.match?(url)).to be true
+      expect(header_match.match?(http_url)).to be true
+    end
+
+    it "returns false for a non-HTTP URL" do
+      expect(header_match.match?(non_http_url)).to be false
+    end
+  end
+
+  describe "::versions_from_headers" do
+    it "returns an empty array if headers hash is empty" do
+      expect(header_match.versions_from_headers({})).to eq([])
+    end
+
+    it "returns an array of version strings when given headers" do
+      expect(header_match.versions_from_headers(headers[:content_disposition])).to eq(versions[:content_disposition])
+      expect(header_match.versions_from_headers(headers[:location])).to eq(versions[:location])
+      expect(header_match.versions_from_headers(headers[:content_disposition_and_location]))
+        .to eq(versions[:content_disposition_and_location])
+
+      expect(header_match.versions_from_headers(headers[:content_disposition], regexes[:archive]))
+        .to eq(versions[:content_disposition])
+      expect(header_match.versions_from_headers(headers[:location], regexes[:latest])).to eq(versions[:location])
+      expect(header_match.versions_from_headers(headers[:content_disposition_and_location], regexes[:latest]))
+        .to eq(versions[:location])
+    end
+
+    it "returns an array of version strings when given headers and a block" do
+      # Returning a string from block, no regex
+      expect(
+        header_match.versions_from_headers(headers[:location]) do |headers|
+          v = Version.parse(headers["location"], detected_from_url: true)
+          v.null? ? nil : v.to_s
+        end,
+      ).to eq(versions[:location])
+
+      # Returning a string from block, explicit regex
+      expect(
+        header_match.versions_from_headers(headers[:location], regexes[:latest]) do |headers, regex|
+          headers["location"] ? headers["location"][regex, 1] : nil
+        end,
+      ).to eq(versions[:location])
+
+      # Returning an array of strings from block
+      # NOTE: Strategies runs `#compact` on an array from a block, so nil
+      # values are filtered out without needing to use `#compact` in the block.
+      expect(
+        header_match.versions_from_headers(
+          headers[:content_disposition_and_location],
+          regexes[:loose],
+        ) do |headers, regex|
+          headers.transform_values { |header| header[regex, 1] }.values
+        end,
+      ).to eq(versions[:content_disposition_and_location])
+    end
+
+    it "allows a nil return from a block" do
+      expect(header_match.versions_from_headers(headers[:location]) { next }).to eq([])
+    end
+
+    it "errors on an invalid return type from a block" do
+      expect { header_match.versions_from_headers(headers) { 123 } }
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
     end
   end
 end

Library/Homebrew/test/livecheck/strategy/launchpad_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Launchpad do
   let(:non_launchpad_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a Launchpad URL" do
+    it "returns true for a Launchpad URL" do
       expect(launchpad.match?(launchpad_url)).to be true
     end
 
-    it "returns false if the argument provided is not a Launchpad URL" do
+    it "returns false for a non-Launchpad URL" do
       expect(launchpad.match?(non_launchpad_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/npm_spec.rb

@@ -11,12 +11,12 @@ describe Homebrew::Livecheck::Strategy::Npm do
   let(:non_npm_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is an npm URL" do
+    it "returns true for an npm URL" do
       expect(npm.match?(npm_url)).to be true
       expect(npm.match?(npm_scoped_url)).to be true
     end
 
-    it "returns false if the argument provided is not an npm URL" do
+    it "returns false for a non-npm URL" do
       expect(npm.match?(non_npm_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/page_match_spec.rb

@@ -1,15 +1,17 @@
 # typed: false
 # frozen_string_literal: true
 
-require "livecheck/strategy/page_match"
+require "livecheck/strategy"
 
 describe Homebrew::Livecheck::Strategy::PageMatch do
   subject(:page_match) { described_class }
 
-  let(:url) { "https://brew.sh/blog/" }
+  let(:http_url) { "https://brew.sh/blog/" }
+  let(:non_http_url) { "ftp://brew.sh/" }
+
   let(:regex) { %r{href=.*?/homebrew[._-]v?(\d+(?:\.\d+)+)/?["' >]}i }
 
-  let(:page_content) {
+  let(:content) {
     <<~EOS
       <!DOCTYPE html>
       <html>
@@ -35,7 +37,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do
     EOS
   }
 
-  let(:page_content_matches) { ["2.6.0", "2.5.0", "2.4.0", "2.3.0", "2.2.0", "2.1.0", "2.0.0", "1.9.0"] }
+  let(:content_matches) { ["2.6.0", "2.5.0", "2.4.0", "2.3.0", "2.2.0", "2.1.0", "2.0.0", "1.9.0"] }
 
   let(:find_versions_return_hash) {
     {
@@ -50,7 +52,7 @@ describe Homebrew::Livecheck::Strategy::PageMatch do
         "1.9.0" => Version.new("1.9.0"),
       },
       regex:   regex,
-      url:     url,
+      url:     http_url,
     }
   }
 
@@ -61,29 +63,50 @@ describe Homebrew::Livecheck::Strategy::PageMatch do
   }
 
   describe "::match?" do
-    it "returns true for any URL" do
+    it "returns true for an HTTP URL" do
-      expect(page_match.match?(url)).to be true
+      expect(page_match.match?(http_url)).to be true
+    end
+
+    it "returns false for a non-HTTP URL" do
+      expect(page_match.match?(non_http_url)).to be false
     end
   end
 
-  describe "::page_matches" do
+  describe "::versions_from_content" do
-    it "finds matching text in page content using a regex" do
+    it "returns an empty array if content is blank" do
-      expect(page_match.page_matches(page_content, regex)).to eq(page_content_matches)
+      expect(page_match.versions_from_content("", regex)).to eq([])
     end
 
-    it "finds matching text in page content using a strategy block" do
+    it "returns an array of version strings when given content" do
-      expect(page_match.page_matches(page_content, regex) { |content, regex| content.scan(regex).map(&:first).uniq })
+      expect(page_match.versions_from_content(content, regex)).to eq(content_matches)
-        .to eq(page_content_matches)
+
+      # Regexes should use a capture group around the version but a regex
+      # without one should still be handled
+      expect(page_match.versions_from_content(content, /\d+(?:\.\d+)+/i)).to eq(content_matches)
     end
 
-    it "allows a nil return from a strategy block" do
+    it "returns an array of version strings when given content and a block" do
-      expect(page_match.page_matches(page_content, regex) { next }).to eq([])
+      # Returning a string from block
+      expect(page_match.versions_from_content(content, regex) { "1.2.3" }).to eq(["1.2.3"])
+
+      # Returning an array of strings from block
+      expect(page_match.versions_from_content(content, regex) { |page, regex| page.scan(regex).map(&:first) })
+        .to eq(content_matches)
+    end
+
+    it "allows a nil return from a block" do
+      expect(page_match.versions_from_content(content, regex) { next }).to eq([])
+    end
+
+    it "errors on an invalid return type from a block" do
+      expect { page_match.versions_from_content(content, regex) { 123 } }
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
     end
   end
 
   describe "::find_versions?" do
     it "finds versions in provided_content" do
-      expect(page_match.find_versions(url, regex, provided_content: page_content))
+      expect(page_match.find_versions(http_url, regex, provided_content: content))
         .to eq(find_versions_cached_return_hash)
     end
   end

Library/Homebrew/test/livecheck/strategy/pypi_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Pypi do
   let(:non_pypi_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a PyPI URL" do
+    it "returns true for a PyPI URL" do
       expect(pypi.match?(pypi_url)).to be true
     end
 
-    it "returns false if the argument provided is not a PyPI URL" do
+    it "returns false for a non-PyPI URL" do
       expect(pypi.match?(non_pypi_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/sourceforge_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Sourceforge do
   let(:non_sourceforge_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is a SourceForge URL" do
+    it "returns true for a SourceForge URL" do
       expect(sourceforge.match?(sourceforge_url)).to be true
     end
 
-    it "returns false if the argument provided is not a SourceForge URL" do
+    it "returns false for a non-SourceForge URL" do
       expect(sourceforge.match?(non_sourceforge_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy/sparkle_spec.rb

@@ -1,16 +1,19 @@
 # typed: false
 # frozen_string_literal: true
 
-require "livecheck/strategy/sparkle"
+require "livecheck/strategy"
+require "bundle_version"
 
 describe Homebrew::Livecheck::Strategy::Sparkle do
   subject(:sparkle) { described_class }
 
-  let(:url) { "https://www.example.com/example/appcast.xml" }
+  let(:appcast_url) { "https://www.example.com/example/appcast.xml" }
+  let(:non_http_url) { "ftp://brew.sh/" }
 
   let(:appcast_data) {
     {
       title:         "Version 1.2.3",
+      pub_date:      "Fri, 01 Jan 2021 01:23:45 +0000",
       url:           "https://www.example.com/example/example.tar.gz",
       short_version: "1.2.3",
       version:       "1234",
@@ -23,13 +26,14 @@ describe Homebrew::Livecheck::Strategy::Sparkle do
       <rss version="2.0" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:sparkle="http://www.andymatuschak.org/xml-namespaces/sparkle">
         <channel>
           <title>Example Changelog</title>
-          <link>#{url}</link>
+          <link>#{appcast_url}</link>
           <description>Most recent changes with links to updates.</description>
           <language>en</language>
           <item>
             <title>#{appcast_data[:title]}</title>
             <sparkle:minimumSystemVersion>10.10</sparkle:minimumSystemVersion>
             <sparkle:releaseNotesLink>https://www.example.com/example/1.2.3.html</sparkle:releaseNotesLink>
+            <pubDate>#{appcast_data[:pub_date]}</pubDate>
             <enclosure url="#{appcast_data[:url]}" sparkle:shortVersionString="#{appcast_data[:short_version]}" sparkle:version="#{appcast_data[:version]}" length="12345678" type="application/octet-stream" sparkle:dsaSignature="ABCDEF+GHIJKLMNOPQRSTUVWXYZab/cdefghijklmnopqrst/uvwxyz1234567==" />
           </item>
         </channel>
@@ -37,9 +41,24 @@ describe Homebrew::Livecheck::Strategy::Sparkle do
     EOS
   }
 
+  let(:item) {
+    Homebrew::Livecheck::Strategy::Sparkle::Item.new(
+      title:          appcast_data[:title],
+      pub_date:       Time.parse(appcast_data[:pub_date]),
+      url:            appcast_data[:url],
+      bundle_version: Homebrew::BundleVersion.new(appcast_data[:short_version], appcast_data[:version]),
+    )
+  }
+
+  let(:versions) { [item.bundle_version.nice_version] }
+
   describe "::match?" do
-    it "returns true for any URL" do
+    it "returns true for an HTTP URL" do
-      expect(sparkle.match?(url)).to be true
+      expect(sparkle.match?(appcast_url)).to be true
+    end
+
+    it "returns false for a non-HTTP URL" do
+      expect(sparkle.match?(non_http_url)).to be false
     end
   end
 
@@ -52,10 +71,39 @@ describe Homebrew::Livecheck::Strategy::Sparkle do
 
     it "returns an Item when given XML data" do
       expect(item_from_appcast_xml).to be_a(Homebrew::Livecheck::Strategy::Sparkle::Item)
+      expect(item_from_appcast_xml).to eq(item)
       expect(item_from_appcast_xml.title).to eq(appcast_data[:title])
+      expect(item_from_appcast_xml.pub_date).to eq(Time.parse(appcast_data[:pub_date]))
       expect(item_from_appcast_xml.url).to eq(appcast_data[:url])
       expect(item_from_appcast_xml.short_version).to eq(appcast_data[:short_version])
       expect(item_from_appcast_xml.version).to eq(appcast_data[:version])
     end
   end
+
+  describe "::versions_from_content" do
+    it "returns an array of version strings when given content" do
+      expect(sparkle.versions_from_content(appcast_xml)).to eq(versions)
+    end
+
+    it "returns an array of version strings when given content and a block" do
+      # Returning a string from block
+      expect(
+        sparkle.versions_from_content(appcast_xml) do |item|
+          item.bundle_version&.nice_version&.sub("3", "4")
+        end,
+      ).to eq([item.bundle_version.nice_version.sub("3", "4")])
+
+      # Returning an array of strings from block
+      expect(sparkle.versions_from_content(appcast_xml) { versions }).to eq(versions)
+    end
+
+    it "allows a nil return from a block" do
+      expect(sparkle.versions_from_content(appcast_xml) { next }).to eq([])
+    end
+
+    it "errors on an invalid return type from a block" do
+      expect { sparkle.versions_from_content(appcast_xml) { 123 } }
+        .to raise_error(TypeError, Homebrew::Livecheck::Strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
+    end
+  end
 end

Library/Homebrew/test/livecheck/strategy/xorg_spec.rb

@@ -10,11 +10,11 @@ describe Homebrew::Livecheck::Strategy::Xorg do
   let(:non_xorg_url) { "https://brew.sh/test" }
 
   describe "::match?" do
-    it "returns true if the argument provided is an X.Org URL" do
+    it "returns true for an X.Org URL" do
       expect(xorg.match?(xorg_url)).to be true
     end
 
-    it "returns false if the argument provided is not an X.Org URL" do
+    it "returns false for a non-X.Org URL" do
       expect(xorg.match?(non_xorg_url)).to be false
     end
   end

Library/Homebrew/test/livecheck/strategy_spec.rb

@@ -30,4 +30,20 @@ describe Homebrew::Livecheck::Strategy do
       end
     end
   end
+
+  describe "::handle_block_return" do
+    it "returns an array of version strings when given a valid value" do
+      expect(strategy.handle_block_return("1.2.3")).to eq(["1.2.3"])
+      expect(strategy.handle_block_return(["1.2.3", "1.2.4"])).to eq(["1.2.3", "1.2.4"])
+    end
+
+    it "returns an empty array when given a nil value" do
+      expect(strategy.handle_block_return(nil)).to eq([])
+    end
+
+    it "errors when given an invalid value" do
+      expect { strategy.handle_block_return(123) }
+        .to raise_error(TypeError, strategy::INVALID_BLOCK_RETURN_VALUE_MSG)
+    end
+  end
 end