From a970780851dc048be35323b4798406b82528609e Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 26 Jul 2021 20:32:10 -0400 Subject: [PATCH 1/3] livecheck: allow nil return from strategy blocks --- Library/Homebrew/livecheck/strategy/apache.rb | 4 ++- .../Homebrew/livecheck/strategy/bitbucket.rb | 4 ++- Library/Homebrew/livecheck/strategy/cpan.rb | 4 ++- .../livecheck/strategy/electron_builder.rb | 19 ++++++++----- .../livecheck/strategy/extract_plist.rb | 13 +++++---- Library/Homebrew/livecheck/strategy/git.rb | 7 +++-- .../livecheck/strategy/github_latest.rb | 4 ++- Library/Homebrew/livecheck/strategy/gnome.rb | 4 ++- Library/Homebrew/livecheck/strategy/gnu.rb | 4 ++- .../Homebrew/livecheck/strategy/hackage.rb | 4 ++- .../livecheck/strategy/header_match.rb | 28 ++++++++++++------- .../Homebrew/livecheck/strategy/launchpad.rb | 4 ++- Library/Homebrew/livecheck/strategy/npm.rb | 4 ++- .../Homebrew/livecheck/strategy/page_match.rb | 23 +++++++++++---- Library/Homebrew/livecheck/strategy/pypi.rb | 4 ++- .../livecheck/strategy/sourceforge.rb | 4 ++- .../Homebrew/livecheck/strategy/sparkle.rb | 17 +++++------ Library/Homebrew/livecheck/strategy/xorg.rb | 4 ++- .../strategy/electron_builder_spec.rb | 9 ++++++ .../livecheck/strategy/page_match_spec.rb | 6 +++- 20 files changed, 119 insertions(+), 51 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy/apache.rb b/Library/Homebrew/livecheck/strategy/apache.rb index 2fdbbcb1fe..36b2395094 100644 --- a/Library/Homebrew/livecheck/strategy/apache.rb +++ b/Library/Homebrew/livecheck/strategy/apache.rb @@ -52,7 +52,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/bitbucket.rb b/Library/Homebrew/livecheck/strategy/bitbucket.rb index 754d2c5e37..4072748aed 100644 --- a/Library/Homebrew/livecheck/strategy/bitbucket.rb +++ b/Library/Homebrew/livecheck/strategy/bitbucket.rb @@ -59,7 +59,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/cpan.rb b/Library/Homebrew/livecheck/strategy/cpan.rb index f642f200a0..88965bd8d6 100644 --- a/Library/Homebrew/livecheck/strategy/cpan.rb +++ b/Library/Homebrew/livecheck/strategy/cpan.rb @@ -50,7 +50,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/electron_builder.rb b/Library/Homebrew/livecheck/strategy/electron_builder.rb index 7dda2f8aa8..45626f315d 100644 --- a/Library/Homebrew/livecheck/strategy/electron_builder.rb +++ b/Library/Homebrew/livecheck/strategy/electron_builder.rb @@ -37,19 +37,24 @@ module Homebrew sig { params( content: String, - block: T.nilable(T.proc.params(arg0: Hash).returns(String)), + block: T.nilable(T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.nilable(String))), ).returns(T.nilable(String)) } def self.version_from_content(content, &block) require "yaml" - return unless (yaml = YAML.safe_load(content)) + yaml = YAML.safe_load(content) + return if yaml.blank? if block - value = block.call(yaml) - return value if value.is_a?(String) - - raise TypeError, "Return value of `strategy :electron_builder` block must be a string." + 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"] @@ -65,7 +70,7 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: Hash).returns(String)), + block: T.nilable(T.proc.params(arg0: T::Hash[String, T.untyped]).returns(T.nilable(String))), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/extract_plist.rb b/Library/Homebrew/livecheck/strategy/extract_plist.rb index 7b24e3bafb..46b7d12e08 100644 --- a/Library/Homebrew/livecheck/strategy/extract_plist.rb +++ b/Library/Homebrew/livecheck/strategy/extract_plist.rb @@ -56,7 +56,7 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: Cask::Cask, - block: T.nilable(T.proc.params(arg0: T::Hash[String, Item]).returns(String)), + block: T.nilable(T.proc.params(arg0: T::Hash[String, Item]).returns(T.nilable(String))), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask:, &block) @@ -69,13 +69,14 @@ module Homebrew versions = unversioned_cask_checker.all_versions.transform_values { |v| Item.new(bundle_version: v) } if block - match = block.call(versions) - - unless T.unsafe(match).is_a?(String) + case (value = block.call(versions)) + 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 - - match_data[:matches][match] = Version.new(match) if match elsif versions.any? versions.each_value do |item| version = item.bundle_version.nice_version diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb index c681d5282a..f8c1aee36f 100644 --- a/Library/Homebrew/livecheck/strategy/git.rb +++ b/Library/Homebrew/livecheck/strategy/git.rb @@ -81,8 +81,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: T::Array[String]) - .returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: T::Array[String]).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) @@ -105,6 +106,8 @@ module Homebrew value.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 diff --git a/Library/Homebrew/livecheck/strategy/github_latest.rb b/Library/Homebrew/livecheck/strategy/github_latest.rb index d29888f329..68a9657448 100644 --- a/Library/Homebrew/livecheck/strategy/github_latest.rb +++ b/Library/Homebrew/livecheck/strategy/github_latest.rb @@ -67,7 +67,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/gnome.rb b/Library/Homebrew/livecheck/strategy/gnome.rb index 01348ef321..7af002b9f5 100644 --- a/Library/Homebrew/livecheck/strategy/gnome.rb +++ b/Library/Homebrew/livecheck/strategy/gnome.rb @@ -55,7 +55,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/gnu.rb b/Library/Homebrew/livecheck/strategy/gnu.rb index a00e41ea9e..dac3db3ef6 100644 --- a/Library/Homebrew/livecheck/strategy/gnu.rb +++ b/Library/Homebrew/livecheck/strategy/gnu.rb @@ -59,7 +59,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/hackage.rb b/Library/Homebrew/livecheck/strategy/hackage.rb index a780c848a8..025ebe266a 100644 --- a/Library/Homebrew/livecheck/strategy/hackage.rb +++ b/Library/Homebrew/livecheck/strategy/hackage.rb @@ -52,7 +52,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/header_match.rb b/Library/Homebrew/livecheck/strategy/header_match.rb index 1e10286d6a..0e69f8c113 100644 --- a/Library/Homebrew/livecheck/strategy/header_match.rb +++ b/Library/Homebrew/livecheck/strategy/header_match.rb @@ -40,8 +40,7 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: T::Hash[String, String]) - .returns(T.any(T::Array[String], String))), + block: T.nilable(T.proc.params(arg0: T::Hash[String, String]).returns(T.nilable(String))), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) @@ -52,31 +51,40 @@ module Homebrew # Merge the headers from all responses into one hash merged_headers = headers.reduce(&:merge) - if block - match = yield merged_headers, regex + version = if block + case (value = block.call(merged_headers, regex)) + when String + value + when nil + return match_data + else + raise TypeError, "Return value of `strategy :header_match` block must be a string." + end else - match = nil + value = nil if (filename = merged_headers["content-disposition"]) if regex - match ||= filename[regex, 1] + value ||= filename[regex, 1] else v = Version.parse(filename, detected_from_url: true) - match ||= v.to_s unless v.null? + value ||= v.to_s unless v.null? end end if (location = merged_headers["location"]) if regex - match ||= location[regex, 1] + value ||= location[regex, 1] else v = Version.parse(location, detected_from_url: true) - match ||= v.to_s unless v.null? + value ||= v.to_s unless v.null? end end + + value end - match_data[:matches][match] = Version.new(match) if match + match_data[:matches][version] = Version.new(version) if version match_data end diff --git a/Library/Homebrew/livecheck/strategy/launchpad.rb b/Library/Homebrew/livecheck/strategy/launchpad.rb index 1257383a3e..d4f935b90e 100644 --- a/Library/Homebrew/livecheck/strategy/launchpad.rb +++ b/Library/Homebrew/livecheck/strategy/launchpad.rb @@ -50,7 +50,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/npm.rb b/Library/Homebrew/livecheck/strategy/npm.rb index f5610f507a..db3b062247 100644 --- a/Library/Homebrew/livecheck/strategy/npm.rb +++ b/Library/Homebrew/livecheck/strategy/npm.rb @@ -46,7 +46,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/page_match.rb b/Library/Homebrew/livecheck/strategy/page_match.rb index ac8750416f..b71ca9e9e7 100644 --- a/Library/Homebrew/livecheck/strategy/page_match.rb +++ b/Library/Homebrew/livecheck/strategy/page_match.rb @@ -45,13 +45,24 @@ module Homebrew # @param regex [Regexp] a regex used for matching versions in the # content # @return [Array] + sig { + params( + content: String, + regex: Regexp, + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), + ).returns(T::Array[String]) + } def self.page_matches(content, regex, &block) if block case (value = block.call(content, regex)) when String return [value] when Array - return value + 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 @@ -61,10 +72,10 @@ module Homebrew case match when String match - else + when Array match.first end - end.uniq + end.compact.uniq end # Checks the content at the URL for new versions, using the provided @@ -78,10 +89,12 @@ module Homebrew sig { params( url: String, - regex: T.nilable(Regexp), + regex: Regexp, cask: T.nilable(Cask::Cask), provided_content: T.nilable(String), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, provided_content: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/pypi.rb b/Library/Homebrew/livecheck/strategy/pypi.rb index f70a28469c..ecbd14c505 100644 --- a/Library/Homebrew/livecheck/strategy/pypi.rb +++ b/Library/Homebrew/livecheck/strategy/pypi.rb @@ -56,7 +56,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/sourceforge.rb b/Library/Homebrew/livecheck/strategy/sourceforge.rb index c5b67bc057..a6a9a9add2 100644 --- a/Library/Homebrew/livecheck/strategy/sourceforge.rb +++ b/Library/Homebrew/livecheck/strategy/sourceforge.rb @@ -62,7 +62,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/livecheck/strategy/sparkle.rb b/Library/Homebrew/livecheck/strategy/sparkle.rb index 15a81e617e..a34802405e 100644 --- a/Library/Homebrew/livecheck/strategy/sparkle.rb +++ b/Library/Homebrew/livecheck/strategy/sparkle.rb @@ -144,7 +144,7 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: Item).returns(String)), + block: T.nilable(T.proc.params(arg0: Item).returns(T.nilable(String))), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) @@ -156,19 +156,20 @@ module Homebrew content = match_data.delete(:content) if (item = item_from_content(content)) - match = if block - value = block.call(item) - - unless T.unsafe(value).is_a?(String) + version = if block + 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 - - value else item.bundle_version&.nice_version end - match_data[:matches][match] = Version.new(match) if match + match_data[:matches][version] = Version.new(version) if version end match_data diff --git a/Library/Homebrew/livecheck/strategy/xorg.rb b/Library/Homebrew/livecheck/strategy/xorg.rb index 7a05b185d4..6f6b94b0a4 100644 --- a/Library/Homebrew/livecheck/strategy/xorg.rb +++ b/Library/Homebrew/livecheck/strategy/xorg.rb @@ -85,7 +85,9 @@ module Homebrew url: String, regex: T.nilable(Regexp), cask: T.nilable(Cask::Cask), - block: T.nilable(T.proc.params(arg0: String).returns(T.any(T::Array[String], String))), + block: T.nilable( + T.proc.params(arg0: String, arg1: Regexp).returns(T.any(String, T::Array[String], NilClass)), + ), ).returns(T::Hash[Symbol, T.untyped]) } def self.find_versions(url, regex, cask: nil, &block) diff --git a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb index 22db3dddc2..43ac739c31 100644 --- a/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/electron_builder_spec.rb @@ -54,5 +54,14 @@ describe Homebrew::Livecheck::Strategy::ElectronBuilder do expect(version).to eq "1.2.4" end + + it "allows a nil return from a strategy block" do + expect(electron_builder.version_from_content(electron_builder_yaml) { next }).to eq(nil) + end + + it "errors on an invalid return type from a strategy block" do + expect { electron_builder.version_from_content(electron_builder_yaml) { 123 } } + .to raise_error(TypeError, "Return value of `strategy :electron_builder` block must be a string.") + end end end diff --git a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb index b0ff28eab9..24e5705d0e 100644 --- a/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb +++ b/Library/Homebrew/test/livecheck/strategy/page_match_spec.rb @@ -72,9 +72,13 @@ describe Homebrew::Livecheck::Strategy::PageMatch do end it "finds matching text in page content using a strategy block" do - expect(page_match.page_matches(page_content, regex) { |content| content.scan(regex).map(&:first).uniq }) + expect(page_match.page_matches(page_content, regex) { |content, regex| content.scan(regex).map(&:first).uniq }) .to eq(page_content_matches) end + + it "allows a nil return from a strategy block" do + expect(page_match.page_matches(page_content, regex) { next }).to eq([]) + end end describe "::find_versions?" do From af2c45b2970b7af07844ee74859b523faa4ba341 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 26 Jul 2021 20:34:12 -0400 Subject: [PATCH 2/3] Git: compact, uniq array from strategy block --- Library/Homebrew/livecheck/strategy/git.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/livecheck/strategy/git.rb b/Library/Homebrew/livecheck/strategy/git.rb index f8c1aee36f..50a0e509a2 100644 --- a/Library/Homebrew/livecheck/strategy/git.rb +++ b/Library/Homebrew/livecheck/strategy/git.rb @@ -103,7 +103,7 @@ module Homebrew when String match_data[:matches][value] = Version.new(value) when Array - value.each do |tag| + value.compact.uniq.each do |tag| match_data[:matches][tag] = Version.new(tag) end when nil From 83f261b6f2ac5cd3092ea9f17a96a6c68744ea83 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Mon, 26 Jul 2021 20:42:33 -0400 Subject: [PATCH 3/3] HeaderMatch: Refactor default header logic --- .../livecheck/strategy/header_match.rb | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/livecheck/strategy/header_match.rb b/Library/Homebrew/livecheck/strategy/header_match.rb index 0e69f8c113..af93c1f34d 100644 --- a/Library/Homebrew/livecheck/strategy/header_match.rb +++ b/Library/Homebrew/livecheck/strategy/header_match.rb @@ -24,6 +24,9 @@ module Homebrew # The `Regexp` used to determine if the strategy applies to the URL. URL_MATCH_REGEX = %r{^https?://}i.freeze + # The header fields to check when a `strategy` block isn't provided. + 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 @@ -62,23 +65,17 @@ module Homebrew end else value = nil + DEFAULT_HEADERS_TO_CHECK.each do |header_name| + header_value = merged_headers[header_name] + next if header_value.blank? - if (filename = merged_headers["content-disposition"]) if regex - value ||= filename[regex, 1] + value = header_value[regex, 1] else - v = Version.parse(filename, detected_from_url: true) - value ||= v.to_s unless v.null? - end - end - - if (location = merged_headers["location"]) - if regex - value ||= location[regex, 1] - else - v = Version.parse(location, detected_from_url: true) - value ||= v.to_s unless v.null? + v = Version.parse(header_value, detected_from_url: true) + value = v.to_s unless v.null? end + break if value.present? end value