Refactor Cleanup.

Library/Homebrew/cask/lib/hbc/cli/cleanup.rb

@@ -1,3 +1,7 @@
+require "cleanup"
+
+using CleanupRefinement
+
 module Hbc
   class CLI
     class Cleanup < AbstractCommand
@@ -30,16 +34,12 @@ module Hbc
         outdated_only? && file && file.stat.mtime > OUTDATED_TIMESTAMP
       end
 
-      def incomplete?(file)
-        file.extname == ".incomplete"
-      end
-
       def cache_incompletes
-        cache_files.select(&method(:incomplete?))
+        cache_files.select(&:incomplete?)
       end
 
       def cache_completes
-        cache_files.reject(&method(:incomplete?))
+        cache_files.reject(&:incomplete?)
       end
 
       def disk_cleanup_size

Library/Homebrew/cleanup.rb

@@ -1,6 +1,71 @@
 require "utils/bottles"
 require "formula"
 
+module CleanupRefinement
+  refine Pathname do
+    def incomplete?
+      extname.end_with?(".incomplete")
+    end
+
+    def nested_cache?
+      directory? && ["glide_home", "java_cache", "npm_cache"].include?(basename.to_s)
+    end
+
+    def prune?(days)
+      return false unless days
+      return true if days.zero?
+
+      # TODO: Replace with ActiveSupport's `.days.ago`.
+      mtime < ((@time ||= Time.now) - days * 60 * 60 * 24)
+    end
+
+    def stale?(scrub = false)
+      return false unless file?
+
+      stale_formula?(scrub)
+    end
+
+    private
+
+    def stale_formula?(scrub)
+      return false unless HOMEBREW_CELLAR.directory?
+
+      version = if to_s.match?(Pathname::BOTTLE_EXTNAME_RX)
+        begin
+          Utils::Bottles.resolve_version(self)
+        rescue
+          self.version
+        end
+      else
+        self.version
+      end
+
+      return false unless version
+      return false unless (name = basename.to_s[/\A(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1])
+
+      formula = begin
+        Formulary.from_rack(HOMEBREW_CELLAR/name)
+      rescue FormulaUnavailableError, TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError
+        return false
+      end
+
+      if version.is_a?(PkgVersion)
+        return true if formula.pkg_version > version
+      elsif formula.version > version
+        return true
+      end
+
+      return true if scrub && !formula.installed?
+
+      return true if Utils::Bottles.file_outdated?(formula, self)
+
+      false
+    end
+  end
+end
+
+using CleanupRefinement
+
 module Homebrew
   module Cleanup
     @disk_cleanup_size = 0
@@ -43,25 +108,22 @@ module Homebrew
       unremovable_kegs << keg
     end
 
+    DEFAULT_LOG_DAYS = 14
+
     def cleanup_logs
       return unless HOMEBREW_LOGS.directory?
       HOMEBREW_LOGS.subdirs.each do |dir|
-        cleanup_path(dir) { dir.rmtree } if prune?(dir, days_default: 14)
+        cleanup_path(dir) { dir.rmtree } if dir.prune?(ARGV.value("prune")&.to_i || DEFAULT_LOG_DAYS)
       end
     end
 
     def cleanup_cache(cache = HOMEBREW_CACHE)
       return unless cache.directory?
       cache.children.each do |path|
-        if path.to_s.end_with? ".incomplete"
+        next cleanup_path(path) { path.unlink } if path.incomplete?
-          cleanup_path(path) { path.unlink }
+        next cleanup_path(path) { FileUtils.rm_rf path } if path.nested_cache?
-          next
+
-        end
+        if path.prune?(ARGV.value("prune")&.to_i)
-        if %w[glide_home java_cache npm_cache].include?(path.basename.to_s) && path.directory?
-          cleanup_path(path) { FileUtils.rm_rf path }
-          next
-        end
-        if prune?(path)
           if path.file?
             cleanup_path(path) { path.unlink }
           elsif path.directory? && path.to_s.include?("--")
@@ -70,42 +132,13 @@ module Homebrew
           next
         end
 
-        next unless path.file?
+        next cleanup_path(path) { path.unlink } if path.stale?(ARGV.switch?("s"))
-        file = path
-
-        if file.to_s =~ Pathname::BOTTLE_EXTNAME_RX
-          version = begin
-                      Utils::Bottles.resolve_version(file)
-                    rescue
-                      file.version
-                    end
-        else
-          version = file.version
-        end
-        next unless version
-        next unless (name = file.basename.to_s[/\A(.*?)\-\-?(?:#{Regexp.escape(version)})/, 1])
-
-        next unless HOMEBREW_CELLAR.directory?
-
-        begin
-          f = Formulary.from_rack(HOMEBREW_CELLAR/name)
-        rescue FormulaUnavailableError, TapFormulaAmbiguityError, TapFormulaWithOldnameAmbiguityError
-          next
-        end
-
-        file_is_stale = if version.is_a?(PkgVersion)
-          f.pkg_version > version
-        else
-          f.version > version
-        end
-
-        if file_is_stale || ARGV.switch?("s") && !f.installed? || Utils::Bottles.file_outdated?(f, file)
-          cleanup_path(file) { file.unlink }
-        end
       end
     end
 
     def cleanup_path(path)
+      disk_usage = path.disk_usage
+
       if ARGV.dry_run?
         puts "Would remove: #{path} (#{path.abv})"
       else
@@ -113,7 +146,7 @@ module Homebrew
         yield
       end
 
-      update_disk_cleanup_size(path.disk_usage)
+      update_disk_cleanup_size(disk_usage)
     end
 
     def cleanup_lockfiles
@@ -143,26 +176,5 @@ module Homebrew
       end
       workers.map(&:join)
     end
-
-    def prune?(path, options = {})
-      @time ||= Time.now
-
-      path_modified_time = path.mtime
-      days_default = options[:days_default]
-
-      prune = ARGV.value "prune"
-
-      return true if prune == "all"
-
-      prune_time = if prune
-        @time - 60 * 60 * 24 * prune.to_i
-      elsif days_default
-        @time - 60 * 60 * 24 * days_default.to_i
-      end
-
-      return false unless prune_time
-
-      path_modified_time < prune_time
-    end
   end
 end

Library/Homebrew/test/cleanup_spec.rb

@@ -2,10 +2,32 @@ require "test/support/fixtures/testball"
 require "cleanup"
 require "fileutils"
 
+using CleanupRefinement
+
+describe CleanupRefinement do
+  describe "::prune?" do
+    alias_matcher :be_pruned, :be_prune
+
+    subject(:path) { HOMEBREW_CACHE/"foo" }
+
+    before do
+      path.mkpath
+    end
+
+    it "returns true when path_modified_time < days_default" do
+      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24)
+      expect(path.prune?(1)).to be true
+    end
+
+    it "returns false when path_modified_time >= days_default" do
+      expect(path.prune?(2)).to be false
+    end
+  end
+end
+
 describe Homebrew::Cleanup do
   let(:ds_store) { Pathname.new("#{HOMEBREW_PREFIX}/Library/.DS_Store") }
   let(:lock_file) { Pathname.new("#{HOMEBREW_LOCK_DIR}/foo") }
-  let(:sec_in_a_day) { 60 * 60 * 24 }
 
   around do |example|
     begin
@@ -131,13 +153,13 @@ describe Homebrew::Cleanup do
     end
 
     it "cleans up logs if older than 14 days" do
-      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 15)
+      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 15 * 60 * 60 * 24)
       described_class.cleanup_logs
       expect(path).not_to exist
     end
 
     it "does not clean up logs less than 14 days old" do
-      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2)
+      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24)
       described_class.cleanup_logs
       expect(path).to exist
     end
@@ -212,7 +234,7 @@ describe Homebrew::Cleanup do
       foo = (HOMEBREW_CACHE/"--foo")
       foo.mkpath
       allow(ARGV).to receive(:value).with("prune").and_return("1")
-      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2)
+      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - 2 * 60 * 60 * 24)
       described_class.cleanup_cache
       expect(foo).not_to exist
     end
@@ -257,23 +279,4 @@ describe Homebrew::Cleanup do
       end
     end
   end
-
-  describe "::prune?" do
-    alias_matcher :be_pruned, :be_prune
-
-    before do
-      foo.mkpath
-    end
-
-    let(:foo) { HOMEBREW_CACHE/"foo" }
-
-    it "returns true when path_modified_time < days_default" do
-      allow_any_instance_of(Pathname).to receive(:mtime).and_return(Time.now - sec_in_a_day * 2)
-      expect(described_class).to be_pruned(foo, days_default: "1")
-    end
-
-    it "returns false when path_modified_time >= days_default" do
-      expect(described_class).not_to be_pruned(foo, days_default: "2")
-    end
-  end
 end