Merge pull request #10309 from samford/livecheck-dsl-avoid-url-symbol-conversion

Livecheck#url: Don't convert URL symbol to string

Library/Homebrew/livecheck.rb

@@ -92,17 +92,11 @@ class Livecheck
   # @param val [String, Symbol] URL to check for version information
   # @return [String, nil]
   def url(val = nil)
-    @url = case val
+    case val
     when nil
-      return @url
-    when :url
-      @formula_or_cask.url.to_s
-    when :head, :stable
-      @formula_or_cask.send(val).url
-    when :homepage
-      @formula_or_cask.homepage
-    when String
-      val
+      @url
+    when String, :head, :homepage, :stable, :url
+      @url = val
     else
       raise TypeError, "Livecheck#url expects a String or valid Symbol"
     end

Library/Homebrew/livecheck/livecheck.rb

@@ -311,6 +311,25 @@ module Homebrew
       puts "#{formula_or_cask_s} : #{current_s} ==> #{latest_s}"
     end
 
+    sig do
+      params(
+        livecheck_url:   T.any(String, Symbol),
+        formula_or_cask: T.any(Formula, Cask::Cask),
+      ).returns(T.nilable(String))
+    end
+    def livecheck_url_to_string(livecheck_url, formula_or_cask)
+      case livecheck_url
+      when String
+        livecheck_url
+      when :url
+        formula_or_cask.url&.to_s if formula_or_cask.is_a?(Cask::Cask)
+      when :head, :stable
+        formula_or_cask.send(livecheck_url)&.url if formula_or_cask.is_a?(Formula)
+      when :homepage
+        formula_or_cask.homepage
+      end
+    end
+
     # Returns an Array containing the formula/cask URLs that can be used by livecheck.
     sig { params(formula_or_cask: T.any(Formula, Cask::Cask)).returns(T::Array[String]) }
     def checkable_urls(formula_or_cask)
@@ -396,7 +415,9 @@ module Homebrew
       livecheck_regex = livecheck.regex
       livecheck_strategy = livecheck.strategy
 
-      urls = [livecheck_url] if livecheck_url.present?
+      livecheck_url_string = livecheck_url_to_string(livecheck_url, formula_or_cask)
+
+      urls = [livecheck_url_string] if livecheck_url_string
       urls ||= checkable_urls(formula_or_cask)
 
       if debug
@@ -413,8 +434,13 @@ module Homebrew
       urls.each_with_index do |original_url, i|
         if debug
           puts
+          if livecheck_url.is_a?(Symbol)
+            # This assumes the URL symbol will fit within the available space
+            puts "URL (#{livecheck_url}):".ljust(18, " ") + original_url
+          else
             puts "URL:              #{original_url}"
           end
+        end
 
         # Skip Gists until/unless we create a method of identifying revisions
         if original_url.include?("gist.github.com")
@@ -514,15 +540,16 @@ module Homebrew
         }
 
         if json && verbose
-          version_info[:meta] = {
-            url:      {
-              original: original_url,
-            },
-            strategy: strategy.blank? ? nil : strategy_name,
-          }
+          version_info[:meta] = {}
+
+          version_info[:meta][:url] = {}
+          version_info[:meta][:url][:symbol] = livecheck_url if livecheck_url.is_a?(Symbol) && livecheck_url_string
+          version_info[:meta][:url][:original] = original_url
           version_info[:meta][:url][:processed] = url if url != original_url
           version_info[:meta][:url][:strategy] = strategy_data[:url] if strategy_data[:url] != url
           version_info[:meta][:url][:final] = strategy_data[:final_url] if strategy_data[:final_url]
+
+          version_info[:meta][:strategy] = strategy.present? ? strategy_name : nil
           version_info[:meta][:strategies] = strategies.map { |s| livecheck_strategy_names[s] } if strategies.present?
           version_info[:meta][:regex] = regex.inspect if regex.present?
           version_info[:meta][:cached] = true if strategy_data[:cached] == true

Library/Homebrew/test/formula_spec.rb

@@ -695,7 +695,7 @@ describe Formula do
         end
       end
 
-      expect(f.livecheck.url).to eq("https://brew.sh/test")
+      expect(f.livecheck.url).to eq(:homepage)
     end
   end
 

Library/Homebrew/test/livecheck/livecheck_spec.rb

@@ -6,6 +6,12 @@ require "livecheck/livecheck"
 describe Homebrew::Livecheck do
   subject(:livecheck) { described_class }
 
+  let(:cask_url) { "https://brew.sh/test-0.0.1.dmg" }
+  let(:head_url) { "https://github.com/Homebrew/brew.git" }
+  let(:homepage_url) { "https://brew.sh" }
+  let(:livecheck_url) { "https://formulae.brew.sh/api/formula/ruby.json" }
+  let(:stable_url) { "https://brew.sh/test-0.0.1.tgz" }
+
   let(:f) do
     formula("test") do
       desc "Test formula"
@@ -25,7 +31,7 @@ describe Homebrew::Livecheck do
       cask "test" do
         version "0.0.1,2"
 
-        url "https://brew.sh/test-0.0.1.tgz"
+        url "https://brew.sh/test-0.0.1.dmg"
         name "Test"
         desc "Test cask"
         homepage "https://brew.sh"
@@ -72,13 +78,63 @@ describe Homebrew::Livecheck do
     end
   end
 
+  describe "::livecheck_url_to_string" do
+    let(:f_livecheck_url) do
+      formula("test_livecheck_url") do
+        desc "Test Livecheck URL formula"
+        homepage "https://brew.sh"
+        url "https://brew.sh/test-0.0.1.tgz"
+        head "https://github.com/Homebrew/brew.git"
+      end
+    end
+
+    let(:c_livecheck_url) do
+      Cask::CaskLoader.load(+<<-RUBY)
+        cask "test_livecheck_url" do
+          version "0.0.1,2"
+
+          url "https://brew.sh/test-0.0.1.dmg"
+          name "Test"
+          desc "Test Livecheck URL cask"
+          homepage "https://brew.sh"
+        end
+      RUBY
+    end
+
+    it "returns a URL string when given a livecheck_url string" do
+      f_livecheck_url.livecheck.url(livecheck_url)
+      expect(livecheck.livecheck_url_to_string(livecheck_url, f_livecheck_url)).to eq(livecheck_url)
+    end
+
+    it "returns a URL symbol when given a valid livecheck_url symbol" do
+      f_livecheck_url.livecheck.url(:head)
+      expect(livecheck.livecheck_url_to_string(head_url, f_livecheck_url)).to eq(head_url)
+
+      f_livecheck_url.livecheck.url(:homepage)
+      expect(livecheck.livecheck_url_to_string(homepage_url, f_livecheck_url)).to eq(homepage_url)
+
+      c_livecheck_url.livecheck.url(:homepage)
+      expect(livecheck.livecheck_url_to_string(homepage_url, c_livecheck_url)).to eq(homepage_url)
+
+      f_livecheck_url.livecheck.url(:stable)
+      expect(livecheck.livecheck_url_to_string(stable_url, f_livecheck_url)).to eq(stable_url)
+
+      c_livecheck_url.livecheck.url(:url)
+      expect(livecheck.livecheck_url_to_string(cask_url, c_livecheck_url)).to eq(cask_url)
+    end
+
+    it "returns nil when not given a string or valid symbol" do
+      expect(livecheck.livecheck_url_to_string(nil, f_livecheck_url)).to eq(nil)
+      expect(livecheck.livecheck_url_to_string(nil, c_livecheck_url)).to eq(nil)
+      expect(livecheck.livecheck_url_to_string(:invalid_symbol, f_livecheck_url)).to eq(nil)
+      expect(livecheck.livecheck_url_to_string(:invalid_symbol, c_livecheck_url)).to eq(nil)
+    end
+  end
+
   describe "::checkable_urls" do
     it "returns the list of URLs to check" do
-      expect(livecheck.checkable_urls(f))
-        .to eq(
-          ["https://github.com/Homebrew/brew.git", "https://brew.sh/test-0.0.1.tgz", "https://brew.sh"],
-        )
-      expect(livecheck.checkable_urls(c)).to eq(["https://brew.sh/test-0.0.1.tgz", "https://brew.sh"])
+      expect(livecheck.checkable_urls(f)).to eq([head_url, stable_url, homepage_url])
+      expect(livecheck.checkable_urls(c)).to eq([cask_url, homepage_url])
     end
   end
 

Library/Homebrew/test/livecheck_spec.rb

@@ -5,111 +5,128 @@ require "formula"
 require "livecheck"
 
 describe Livecheck do
-  HOMEPAGE_URL = "https://example.com/"
-  STABLE_URL = "https://example.com/example-1.2.3.tar.gz"
-  HEAD_URL = "https://example.com/example.git"
-
   let(:f) do
     formula do
-      homepage HOMEPAGE_URL
-      url STABLE_URL
-      head HEAD_URL
+      homepage "https://brew.sh"
+      url "https://brew.sh/test-0.0.1.tgz"
+      head "https://github.com/Homebrew/brew.git"
     end
   end
-  let(:livecheckable) { described_class.new(f) }
+  let(:livecheckable_f) { described_class.new(f) }
+
+  let(:c) do
+    Cask::CaskLoader.load(+<<-RUBY)
+      cask "test" do
+        version "0.0.1,2"
+
+        url "https://brew.sh/test-0.0.1.dmg"
+        name "Test"
+        desc "Test cask"
+        homepage "https://brew.sh"
+      end
+    RUBY
+  end
+  let(:livecheckable_c) { described_class.new(c) }
 
   describe "#regex" do
     it "returns nil if not set" do
-      expect(livecheckable.regex).to be nil
+      expect(livecheckable_f.regex).to be nil
     end
 
     it "returns the Regexp if set" do
-      livecheckable.regex(/foo/)
-      expect(livecheckable.regex).to eq(/foo/)
+      livecheckable_f.regex(/foo/)
+      expect(livecheckable_f.regex).to eq(/foo/)
     end
 
     it "raises a TypeError if the argument isn't a Regexp" do
       expect {
-        livecheckable.regex("foo")
+        livecheckable_f.regex("foo")
       }.to raise_error(TypeError, "Livecheck#regex expects a Regexp")
     end
   end
 
   describe "#skip" do
     it "sets @skip to true when no argument is provided" do
-      expect(livecheckable.skip).to be true
-      expect(livecheckable.instance_variable_get(:@skip)).to be true
-      expect(livecheckable.instance_variable_get(:@skip_msg)).to be nil
+      expect(livecheckable_f.skip).to be true
+      expect(livecheckable_f.instance_variable_get(:@skip)).to be true
+      expect(livecheckable_f.instance_variable_get(:@skip_msg)).to be nil
     end
 
     it "sets @skip to true and @skip_msg to the provided String" do
-      expect(livecheckable.skip("foo")).to be true
-      expect(livecheckable.instance_variable_get(:@skip)).to be true
-      expect(livecheckable.instance_variable_get(:@skip_msg)).to eq("foo")
+      expect(livecheckable_f.skip("foo")).to be true
+      expect(livecheckable_f.instance_variable_get(:@skip)).to be true
+      expect(livecheckable_f.instance_variable_get(:@skip_msg)).to eq("foo")
     end
 
     it "raises a TypeError if the argument isn't a String" do
       expect {
-        livecheckable.skip(/foo/)
+        livecheckable_f.skip(/foo/)
       }.to raise_error(TypeError, "Livecheck#skip expects a String")
     end
   end
 
   describe "#skip?" do
     it "returns the value of @skip" do
-      expect(livecheckable.skip?).to be false
+      expect(livecheckable_f.skip?).to be false
 
-      livecheckable.skip
-      expect(livecheckable.skip?).to be true
+      livecheckable_f.skip
+      expect(livecheckable_f.skip?).to be true
     end
   end
 
   describe "#strategy" do
     it "returns nil if not set" do
-      expect(livecheckable.strategy).to be nil
+      expect(livecheckable_f.strategy).to be nil
     end
 
     it "returns the Symbol if set" do
-      livecheckable.strategy(:page_match)
-      expect(livecheckable.strategy).to eq(:page_match)
+      livecheckable_f.strategy(:page_match)
+      expect(livecheckable_f.strategy).to eq(:page_match)
     end
 
     it "raises a TypeError if the argument isn't a Symbol" do
       expect {
-        livecheckable.strategy("page_match")
+        livecheckable_f.strategy("page_match")
       }.to raise_error(TypeError, "Livecheck#strategy expects a Symbol")
     end
   end
 
   describe "#url" do
+    let(:url_string) { "https://brew.sh" }
+
     it "returns nil if not set" do
-      expect(livecheckable.url).to be nil
+      expect(livecheckable_f.url).to be nil
     end
 
-    it "returns the URL if set" do
-      livecheckable.url("foo")
-      expect(livecheckable.url).to eq("foo")
-
-      livecheckable.url(:homepage)
-      expect(livecheckable.url).to eq(HOMEPAGE_URL)
-
-      livecheckable.url(:stable)
-      expect(livecheckable.url).to eq(STABLE_URL)
-
-      livecheckable.url(:head)
-      expect(livecheckable.url).to eq(HEAD_URL)
+    it "returns a string when set to a string" do
+      livecheckable_f.url(url_string)
+      expect(livecheckable_f.url).to eq(url_string)
     end
 
-    it "raises a TypeError if the argument isn't a String or Symbol" do
+    it "returns the URL symbol if valid" do
+      livecheckable_f.url(:head)
+      expect(livecheckable_f.url).to eq(:head)
+
+      livecheckable_f.url(:homepage)
+      expect(livecheckable_f.url).to eq(:homepage)
+
+      livecheckable_f.url(:stable)
+      expect(livecheckable_f.url).to eq(:stable)
+
+      livecheckable_c.url(:url)
+      expect(livecheckable_c.url).to eq(:url)
+    end
+
+    it "raises a TypeError if the argument isn't a String or valid Symbol" do
       expect {
-        livecheckable.url(/foo/)
+        livecheckable_f.url(/foo/)
       }.to raise_error(TypeError, "Livecheck#url expects a String or valid Symbol")
     end
   end
 
   describe "#to_hash" do
     it "returns a Hash of all instance variables" do
-      expect(livecheckable.to_hash).to eq(
+      expect(livecheckable_f.to_hash).to eq(
         {
           "regex"    => nil,
           "skip"     => false,