Removed redundant documentation, use database_cache as a block, and use symbolic keys over string keys in function calls.

Library/Homebrew/dev-cmd/linkage.rb

@@ -19,22 +19,22 @@ module Homebrew
   module_function
 
   def linkage
-    ARGV.kegs.each do |keg|
-      ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1
-      database_cache = DatabaseCache.new("linkage")
-      result = LinkageChecker.new(keg, database_cache)
-      result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild")
+    DatabaseCache.new(:linkage) do |database_cache|
+      ARGV.kegs.each do |keg|
+        ohai "Checking #{keg.name} linkage" if ARGV.kegs.size > 1
 
-      if ARGV.include?("--test")
-        result.display_test_output
-        Homebrew.failed = true if result.broken_dylibs?
-      elsif ARGV.include?("--reverse")
-        result.display_reverse_output
-      else
-        result.display_normal_output
+        result = LinkageChecker.new(keg, database_cache)
+        result.flush_cache_and_check_dylibs if ARGV.include?("--rebuild")
+
+        if ARGV.include?("--test")
+          result.display_test_output
+          Homebrew.failed = true if result.broken_dylibs?
+        elsif ARGV.include?("--reverse")
+          result.display_reverse_output
+        else
+          result.display_normal_output
+        end
       end
-
-      database_cache.close
     end
   end
 end

Library/Homebrew/extend/os/mac/formula_cellar_checks.rb

@@ -64,10 +64,11 @@ module FormulaCellarChecks
   def check_linkage
     return unless formula.prefix.directory?
     keg = Keg.new(formula.prefix)
-    database_cache = DatabaseCache.new("linkage")
-    checker = LinkageChecker.new(keg, database_cache, formula)
-    checker.flush_cache_and_check_dylibs
-    database_cache.close
+
+    DatabaseCache.new(:linkage) do |database_cache|
+      checker = LinkageChecker.new(keg, database_cache, formula)
+      checker.flush_cache_and_check_dylibs
+    end
 
     return unless checker.broken_dylibs?
     output = <<~EOS

Library/Homebrew/os/mac/cache_store.rb

@@ -2,143 +2,97 @@ require "dbm"
 require "json"
 
 #
-# `DatabaseCache` is a class acting as an interface to a persistent storage
-# mechanism residing in the `HOMEBREW_CACHE`
+# `DatabaseCache` acts as an interface to a persistent storage mechanism
+# residing in the `HOMEBREW_CACHE`
 #
 class DatabaseCache
-  # Name of the database cache file located at <HOMEBREW_CACHE>/<name>.db
-  #
-  # @return [String]
-  attr_accessor :name
+  # Users have read and write, but not execute permissions
+  DATABASE_MODE = 0666
 
-  # Instantiates new `DatabaseCache` object
+  # Opens and yields a database in read/write mode
   #
-  # @param  [String] name
-  # @return [nil]
+  # DBM::WRCREAT: Creates the database if it does not already exist
   def initialize(name)
-    @name = name
-  end
-
-  # Memoized `DBM` database object with on-disk database located in the
-  # `HOMEBREW_CACHE`
-  #
-  # @return [DBM] db
-  def db
-    @db ||= DBM.open("#{HOMEBREW_CACHE}/#{name}", 0666, DBM::WRCREAT)
-  end
-
-  # Close the `DBM` database object after usage
-  #
-  # @return [nil]
-  def close
-    db.close
+    @db = DBM.open("#{HOMEBREW_CACHE}/#{name}.db", DATABASE_MODE, DBM::WRCREAT)
+    yield(@db)
+    @db.close
   end
 end
 
 #
-# `CacheStore` is an abstract base class which provides methods to mutate and
-# fetch data from a persistent storage mechanism
-#
-# @abstract
+# `CacheStore` provides methods to mutate and fetch data from a persistent
+# storage mechanism
 #
 class CacheStore
-  # Instantiates a new `CacheStore` class
-  #
-  # @param  [DatabaseCache] database_cache
-  # @return [nil]
   def initialize(database_cache)
-    @db = database_cache.db
+    @database_cache = database_cache
   end
 
   # Inserts new values or updates existing cached values to persistent storage
   # mechanism
-  #
-  # @abstract
-  # @param  [Any]
-  # @return [nil]
   def update!(*)
     raise NotImplementedError
   end
 
   # Fetches cached values in persistent storage according to the type of data
   # stored
-  #
-  # @abstract
-  # @param  [Any]
-  # @return [Any]
-  def fetch(*)
+  def fetch_type(*)
     raise NotImplementedError
   end
 
   # Deletes data from the cache based on a condition defined in a concrete class
-  #
-  # @abstract
-  # @return [nil]
   def flush_cache!
     raise NotImplementedError
   end
 
   protected
 
-  # A class instance providing access to the `DBM` database object
+  attr_reader :database_cache
+
+  # Parses `DBM` stored `String` into ruby `Hash`
   #
-  # @return [DBM]
-  attr_reader :db
+  # DBM stores ruby objects as a ruby `String`. Hence, when fetching the data,
+  # to convert the ruby string back into a ruby `Hash`, the string is converted
+  # into a JSON compatible string, where it may be parsed by the JSON.parse
+  # function
+  def string_to_hash(string)
+    JSON.parse(string.gsub("=>", ":"))
+  end
 end
 
 #
-# `LinkageStore` is a concrete class providing methods to fetch and mutate
-# linkage-specific data used by the `brew linkage` command
-#
-# If the cache hasn't changed, don't do extra processing in `LinkageChecker`.
-# Instead, just fetch the data stored in the cache
+# `LinkageStore` provides methods to fetch and mutate linkage-specific data used
+# by the `brew linkage` command
 #
 class LinkageStore < CacheStore
-  # Types of dylibs of the form (label -> array)
-  HASH_LINKAGE_TYPES = %w[brewed_dylibs reverse_links].freeze
+  HASH_LINKAGE_TYPES = [:brewed_dylibs, :reverse_links].freeze
 
-  # The keg name for the `LinkageChecker` class
-  #
-  # @return [String]
-  attr_reader :key
-
-  # Initializes new `LinkageStore` class
-  #
-  # @param  [String]        keg_name
-  # @param  [DatabaseCache] database_cache
-  # @return [nil]
   def initialize(keg_name, database_cache)
-    @key = keg_name
+    @keg_name = keg_name
     super(database_cache)
   end
 
-  # Inserts new values or updates existing cached values to persistent storage
-  # mechanism according to the type of data
-  #
-  # @param  [Hash] path_values
-  # @param  [Hash] hash_values
-  # @return [nil]
   def update!(
     path_values: {
-      "system_dylibs" => %w[], "variable_dylibs" => %w[], "broken_dylibs" => %w[],
-      "indirect_deps" => %w[], "undeclared_deps" => %w[], "unnecessary_deps" => %w[]
+      system_dylibs: %w[],
+      variable_dylibs: %w[],
+      broken_dylibs: %w[],
+      indirect_deps: %w[],
+      undeclared_deps:  %w[],
+      unnecessary_deps: %w[],
     },
     hash_values: {
-      "brewed_dylibs" => {}, "reverse_links" => {}
+      brewed_dylibs: {},
+      reverse_links: {},
     }
   )
-    db[key] = {
+    database_cache[keg_name] = {
       "path_values" => format_path_values(path_values),
       "hash_values" => format_hash_values(hash_values),
     }
   end
 
-  # Fetches cached values in persistent storage according to the type of data
-  # stored
-  #
-  # @param  [String] type
-  # @return [Any]
-  def fetch(type:)
+  def fetch_type(type)
     if HASH_LINKAGE_TYPES.include?(type)
       fetch_hash_values(type: type)
     else
@@ -146,60 +100,36 @@ class LinkageStore < CacheStore
     end
   end
 
-  # A condition for where to flush the cache
-  #
-  # @return [String]
   def flush_cache!
-    db.delete(key)
+    database_cache.delete(keg_name)
   end
 
   private
 
-  # Fetches a subset of paths where the name = `key`
-  #
-  # @param  [String] type
-  # @return [Array[String]]
+  attr_reader :keg_name
+
   def fetch_path_values(type:)
-    return [] unless db.key?(key) && !db[key].nil?
-    string_to_hash(db[key])["path_values"][type]
+    return [] if !database_cache.key?(keg_name) || database_cache[keg_name].nil?
+    string_to_hash(database_cache[keg_name])["path_values"][type.to_s]
   end
 
-  # Fetches a subset of paths and labels where the name = `key`. Formats said
-  # paths/labels into `key => [value]` syntax expected by `LinkageChecker`
-  #
-  # @param  [String] type
-  # @return [Hash]
   def fetch_hash_values(type:)
-    return {} unless db.key?(key) && !db[key].nil?
-    string_to_hash(db[key])["hash_values"][type]
-  end
-
-  # Parses `DBM` stored `String` into ruby `Hash`
-  #
-  # @param [String] string
-  # @return [Hash]
-  def string_to_hash(string)
-    JSON.parse(string.gsub("=>", ":"))
+    return {} if !database_cache.key?(keg_name) || database_cache[keg_name].nil?
+    string_to_hash(database_cache[keg_name])["hash_values"][type.to_s]
   end
 
   # Formats the linkage data for `path_values` into a kind which can be parsed
-  # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s.
-  #
-  # @param  [Hash(String, Set(String))] hash
-  # @return [Hash(String, Array(String))]
+  # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s
   def format_path_values(hash)
-    hash.each_with_object({}) { |(k, v), h| h[k] = v.to_a }
+    hash.each_with_object({}) { |(k, v), h| h[k.to_s] = v.to_a }
   end
 
   # Formats the linkage data for `hash_values` into a kind which can be parsed
   # by the `string_to_hash` method. Converts ruby `Set`s to `Array`s, and
   # converts ruby `Pathname`s to `String`s
-  #
-  # @param  [Hash(String, Set(Pathname))] hash
-  # @return [Hash(String, Array(String))]
   def format_hash_values(hash)
     hash.each_with_object({}) do |(outer_key, outer_values), outer_hash|
-      outer_hash[outer_key] = outer_values.each_with_object({}) do |(k, v), h|
+      outer_hash[outer_key.to_s] = outer_values.each_with_object({}) do |(k, v), h|
         h[k] = v.to_a.map(&:to_s)
       end
     end

Library/Homebrew/os/mac/linkage_checker.rb

@@ -15,37 +15,37 @@ class LinkageChecker
   # 'Hash-type' cache values
 
   def brewed_dylibs
-    @brewed_dylibs ||= store.fetch(type: "brewed_dylibs")
+    @brewed_dylibs ||= store.fetch_type(:brewed_dylibs)
   end
 
   def reverse_links
-    @reverse_links ||= store.fetch(type: "reverse_links")
+    @reverse_links ||= store.fetch_type(:reverse_links)
   end
 
   # 'Path-type' cached values
 
   def system_dylibs
-    @system_dylibs ||= store.fetch(type: "system_dylibs")
+    @system_dylibs ||= store.fetch_type(:system_dylibs)
   end
 
   def broken_dylibs
-    @broken_dylibs ||= store.fetch(type: "broken_dylibs")
+    @broken_dylibs ||= store.fetch_type(:broken_dylibs)
   end
 
   def variable_dylibs
-    @variable_dylibs ||= store.fetch(type: "variable_dylibs")
+    @variable_dylibs ||= store.fetch_type(:variable_dylibs)
   end
 
   def undeclared_deps
-    @undeclared_deps ||= store.fetch(type: "undeclared_deps")
+    @undeclared_deps ||= store.fetch_type(:undeclared_deps)
   end
 
   def indirect_deps
-    @indirect_deps ||= store.fetch(type: "indirect_deps")
+    @indirect_deps ||= store.fetch_type(:indirect_deps)
   end
 
   def unnecessary_deps
-    @unnecessary_deps ||= store.fetch(type: "unnecessary_deps")
+    @unnecessary_deps ||= store.fetch_type(:unnecessary_deps)
   end
 
   def flush_cache_and_check_dylibs
@@ -207,8 +207,6 @@ class LinkageChecker
   end
 
   # Helper function to reset dylib values when building cache
-  #
-  # @return [nil]
   def reset_dylibs!
     store.flush_cache!
     @system_dylibs    = Set.new
@@ -222,21 +220,19 @@ class LinkageChecker
   end
 
   # Updates data store with package path values
-  #
-  # @return [nil]
   def store_dylibs!
     store.update!(
       path_values: {
-        "system_dylibs"    => @system_dylibs,
-        "variable_dylibs"  => @variable_dylibs,
-        "broken_dylibs"    => @broken_dylibs,
-        "indirect_deps"    => @indirect_deps,
-        "undeclared_deps"  => @undeclared_deps,
-        "unnecessary_deps" => @unnecessary_deps,
+        system_dylibs: @system_dylibs,
+        variable_dylibs: @variable_dylibs,
+        broken_dylibs: @broken_dylibs,
+        indirect_deps: @indirect_deps,
+        undeclared_deps: @undeclared_deps,
+        unnecessary_deps: @unnecessary_deps,
       },
       hash_values: {
-        "brewed_dylibs"    => @brewed_dylibs,
-        "reverse_links"    => @reverse_links,
+        brewed_dylibs: @brewed_dylibs,
+        reverse_links: @reverse_links,
       },
     )
   end