Merge pull request #9456 from reitermarkus/bump-unversioned-casks

Fix wrong version mapping in `bump-unversioned-casks`.

Library/Homebrew/cli/named_args.rb

@@ -129,13 +129,21 @@ module Homebrew
       # Keep existing paths and try to convert others to tap, formula or cask paths.
       # If a cask and formula with the same name exist, includes both their paths
       # unless `only` is specified.
-      def to_paths(only: nil)
+      sig { params(only: T.nilable(Symbol), recurse_tap: T::Boolean).returns(T::Array[Pathname]) }
+      def to_paths(only: nil, recurse_tap: false)
         @to_paths ||= {}
         @to_paths[only] ||= downcased_unique_named.flat_map do |name|
           if File.exist?(name)
             Pathname(name)
           elsif name.count("/") == 1 && !name.start_with?("./", "/")
-            Tap.fetch(name).path
+            tap = Tap.fetch(name)
+
+            if recurse_tap
+              next tap.formula_files if only == :formula
+              next tap.cask_files if only == :cask
+            end
+
+            tap.path
           else
             next Formulary.path(name) if only == :formula
             next Cask::CaskLoader.path(name) if only == :cask

Library/Homebrew/dev-cmd/bump-unversioned-casks.rb

@@ -17,7 +17,7 @@ module Homebrew
   def self.bump_unversioned_casks_args
     Homebrew::CLI::Parser.new do
       usage_banner <<~EOS
-        `bump-unversioned-casks` [<options>] [<tap>]
+        `bump-unversioned-casks` [<options>] [<cask>|<tap>]
 
         Check all casks with unversioned URLs in a given <tap> for updates.
       EOS
@@ -28,7 +28,7 @@ module Homebrew
       flag   "--state-file=",
              description: "File for caching state."
 
-      named 1
+      min_named 1
     end
   end
 
@@ -43,11 +43,10 @@ module Homebrew
     end
     state_file.dirname.mkpath
 
-    tap = Tap.fetch(args.named.first)
-
     state = state_file.exist? ? JSON.parse(state_file.read) : {}
 
-    cask_files = tap.cask_files
+    cask_files = args.named.to_paths(only: :cask, recurse_tap: true)
+
     unversioned_cask_files = cask_files.select do |cask_file|
       url = cask_file.each_line do |line|
         url = line[/\s*url\s+"([^"]+)"\s*/, 1]
@@ -59,7 +58,7 @@ module Homebrew
 
     unversioned_casks = unversioned_cask_files.map { |path| Cask::CaskLoader.load(path) }
 
-    ohai "Unversioned Casks: #{unversioned_casks.count}"
+    ohai "Unversioned Casks: #{unversioned_casks.count} (#{state.size} cached)"
 
     checked, unchecked = unversioned_casks.partition { |c| state.key?(c.full_name) }
 

Library/Homebrew/test/unversioned_cask_checker_spec.rb

@@ -16,6 +16,7 @@ describe Homebrew::UnversionedCaskChecker do
       ["1.0", "1"]           => "1.0",
       ["1.0", "0"]           => "1.0",
       ["1.2.3.4000", "4000"] => "1.2.3.4000",
+      ["5", "5.0.45"]        => "5.0.45",
     }
 
     expected_mappings.each do |(short_version, version), expected_version|

Library/Homebrew/unversioned_cask_checker.rb

@@ -72,18 +72,15 @@ module Homebrew
     def self.decide_between_versions(short_version, version)
       return short_version if short_version == version
 
-      short_version_match = short_version&.match?(/\A\d+(\.\d+)+\Z/)
-      version_match = version&.match?(/\A\d+(\.\d+)+\Z/)
+      if short_version && version
+        return version if version.match?(/\A\d+(\.\d+)+\Z/) && version.start_with?("#{short_version}.")
+        return short_version if short_version.match?(/\A\d+(\.\d+)+\Z/) && short_version.start_with?("#{version}.")
 
-      if short_version_match && version_match
-        return version if version.length > short_version.length && version.start_with?("#{short_version}.")
-        return short_version if short_version.length > version.length && short_version.start_with?("#{version}.")
-      end
+        if short_version.match?(/\A\d+(\.\d+)*\Z/) && version.match?(/\A\d+\Z/)
+          return short_version if short_version.start_with?("#{version}.") || short_version.end_with?(".#{version}")
 
-      if short_version&.match?(/\A\d+(\.\d+)*\Z/) && version&.match?(/\A\d+\Z/)
-        return short_version if short_version.start_with?("#{version}.") || short_version.end_with?(".#{version}")
-
-        return "#{short_version},#{version}"
+          return "#{short_version},#{version}"
+        end
       end
 
       short_version || version

docs/Manpage.md

@@ -899,7 +899,7 @@ present, "revision 1" will be added.
 * `--message`:
   Append *`message`* to the default commit message.
 
-### `bump-unversioned-casks` [*`options`*] [*`tap`*]
+### `bump-unversioned-casks` [*`options`*] [*`cask`*|*`tap`*]
 
 Check all casks with unversioned URLs in a given *`tap`* for updates.
 

manpages/brew.1

@@ -1245,7 +1245,7 @@ Print what would be done rather than doing it\.
 \fB\-\-message\fR
 Append \fImessage\fR to the default commit message\.
 .
-.SS "\fBbump\-unversioned\-casks\fR [\fIoptions\fR] [\fItap\fR]"
+.SS "\fBbump\-unversioned\-casks\fR [\fIoptions\fR] [\fIcask\fR|\fItap\fR]"
 Check all casks with unversioned URLs in a given \fItap\fR for updates\.
 .
 .TP