Merge pull request #18622 from Homebrew/livecheck/error-on-invalid-url-symbol

livecheck: error on invalid url symbol

Library/Homebrew/livecheck/livecheck.rb

@@ -508,10 +508,10 @@ module Homebrew
       params(
         livecheck_url:       T.any(String, Symbol),
         package_or_resource: T.any(Formula, Cask::Cask, Resource),
-      ).returns(T.nilable(String))
+      ).returns(String)
     }
     def self.livecheck_url_to_string(livecheck_url, package_or_resource)
-      case livecheck_url
+      livecheck_url_string = case livecheck_url
       when String
         livecheck_url
       when :url
@@ -521,6 +521,12 @@ module Homebrew
       when :homepage
         package_or_resource.homepage unless package_or_resource.is_a?(Resource)
       end
+
+      if livecheck_url.is_a?(Symbol) && !livecheck_url_string
+        raise ArgumentError, "`url #{livecheck_url.inspect}` does not reference a checkable URL"
+      end
+
+      livecheck_url_string
     end
 
     # Returns an Array containing the formula/cask/resource URLs that can be used by livecheck.
@@ -846,7 +852,7 @@ module Homebrew
       livecheck_strategy = livecheck.strategy
       livecheck_strategy_block = livecheck.strategy_block
 
-      livecheck_url_string = livecheck_url_to_string(livecheck_url, resource)
+      livecheck_url_string = livecheck_url_to_string(livecheck_url, resource) if livecheck_url
 
       urls = [livecheck_url_string] if livecheck_url_string
       urls ||= checkable_urls(resource)

Library/Homebrew/test/livecheck/livecheck_spec.rb

@@ -36,6 +36,15 @@ RSpec.describe Homebrew::Livecheck do
     end
   end
 
+  let(:f_stable_url_only) do
+    stable_url_s = stable_url
+
+    formula("test_stable_url_only") do
+      desc "Test formula with only a stable URL"
+      url stable_url_s
+    end
+  end
+
   let(:r) { f.resources.first }
 
   let(:c) do
@@ -56,6 +65,17 @@ RSpec.describe Homebrew::Livecheck do
     RUBY
   end
 
+  let(:c_no_checkable_urls) do
+    Cask::CaskLoader.load(+<<-RUBY)
+      cask "test_no_checkable_urls" do
+        version "1.2.3"
+
+        name "Test"
+        desc "Test cask with no checkable URLs"
+      end
+    RUBY
+  end
+
   describe "::resolve_livecheck_reference" do
     context "when a formula/cask has a livecheck block without formula/cask methods" do
       it "returns [nil, []]" do
@@ -167,9 +187,35 @@ RSpec.describe Homebrew::Livecheck do
     end
 
     it "returns nil when not given a string or valid symbol" do
-      expect(livecheck.livecheck_url_to_string(:invalid_symbol, f_livecheck_url)).to be_nil
+      error_text = "`url :%<symbol>s` does not reference a checkable URL"
-      expect(livecheck.livecheck_url_to_string(:invalid_symbol, c_livecheck_url)).to be_nil
+
-      expect(livecheck.livecheck_url_to_string(:invalid_symbol, r_livecheck_url)).to be_nil
+      # Invalid symbol in any context
+      expect { livecheck.livecheck_url_to_string(:invalid_symbol, f_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
+      expect { livecheck.livecheck_url_to_string(:invalid_symbol, c_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
+      expect { livecheck.livecheck_url_to_string(:invalid_symbol, r_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :invalid_symbol))
+
+      # Valid symbol in provided context but referenced URL is not present
+      expect { livecheck.livecheck_url_to_string(:head, f_stable_url_only) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :head))
+      expect { livecheck.livecheck_url_to_string(:homepage, f_stable_url_only) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :homepage))
+      expect { livecheck.livecheck_url_to_string(:homepage, c_no_checkable_urls) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :homepage))
+      expect { livecheck.livecheck_url_to_string(:url, c_no_checkable_urls) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :url))
+
+      # Valid symbol but not in the provided context
+      expect { livecheck.livecheck_url_to_string(:head, c_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :head))
+      expect { livecheck.livecheck_url_to_string(:homepage, r_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :homepage))
+      expect { livecheck.livecheck_url_to_string(:stable, c_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :stable))
+      expect { livecheck.livecheck_url_to_string(:url, f_livecheck_url) }
+        .to raise_error(ArgumentError, format(error_text, symbol: :url))
     end
   end
 
@@ -189,6 +235,8 @@ RSpec.describe Homebrew::Livecheck do
       expect(livecheck.checkable_urls(c)).to eq([cask_url, homepage_url])
       expect(livecheck.checkable_urls(r)).to eq([resource_url])
       expect(livecheck.checkable_urls(f_duplicate_urls)).to eq([stable_url, head_url])
+      expect(livecheck.checkable_urls(f_stable_url_only)).to eq([stable_url])
+      expect(livecheck.checkable_urls(c_no_checkable_urls)).to eq([])
     end
   end