Livecheck::Options: Rework as T::Struct
As suggested, this reworks `Options` to subclass `T::Struct`, which simplifies the implementation and makes it easier to maintain. One noteworthy difference in switching to `T::Struct` is that `#serialize` omits `nil` values but I don't _think_ this should be a problem for us. In terms of changes, I modified `#merge` to remove a now-unnecessary `compact` call and updated related tests. Co-authored-by: Douglas Eichelberger <697964+dduugg@users.noreply.github.com>
This commit is contained in:
		
							parent
							
								
									b6eb945320
								
							
						
					
					
						commit
						8afa354c35
					
				@ -179,13 +179,9 @@ class Livecheck
 | 
			
		||||
  def url(url = T.unsafe(nil), homebrew_curl: nil, post_form: nil, post_json: nil)
 | 
			
		||||
    raise ArgumentError, "Only use `post_form` or `post_json`, not both" if post_form && post_json
 | 
			
		||||
 | 
			
		||||
    if homebrew_curl || post_form || post_json
 | 
			
		||||
      @options = @options.merge({
 | 
			
		||||
        homebrew_curl:,
 | 
			
		||||
        post_form:,
 | 
			
		||||
        post_json:,
 | 
			
		||||
      }.compact)
 | 
			
		||||
    end
 | 
			
		||||
    @options.homebrew_curl = homebrew_curl unless homebrew_curl.nil?
 | 
			
		||||
    @options.post_form = post_form unless post_form.nil?
 | 
			
		||||
    @options.post_json = post_json unless post_json.nil?
 | 
			
		||||
 | 
			
		||||
    case url
 | 
			
		||||
    when nil
 | 
			
		||||
 | 
			
		||||
@ -8,34 +8,15 @@ module Homebrew
 | 
			
		||||
    #
 | 
			
		||||
    # Option values use a `nil` default to indicate that the value has not been
 | 
			
		||||
    # set.
 | 
			
		||||
    class Options
 | 
			
		||||
    class Options < T::Struct
 | 
			
		||||
      # Whether to use brewed curl.
 | 
			
		||||
      sig { returns(T.nilable(T::Boolean)) }
 | 
			
		||||
      attr_reader :homebrew_curl
 | 
			
		||||
      prop :homebrew_curl, T.nilable(T::Boolean)
 | 
			
		||||
 | 
			
		||||
      # Form data to use when making a `POST` request.
 | 
			
		||||
      sig { returns(T.nilable(T::Hash[Symbol, String])) }
 | 
			
		||||
      attr_reader :post_form
 | 
			
		||||
      prop :post_form, T.nilable(T::Hash[Symbol, String])
 | 
			
		||||
 | 
			
		||||
      # JSON data to use when making a `POST` request.
 | 
			
		||||
      sig { returns(T.nilable(T::Hash[Symbol, String])) }
 | 
			
		||||
      attr_reader :post_json
 | 
			
		||||
 | 
			
		||||
      # @param homebrew_curl whether to use brewed curl
 | 
			
		||||
      # @param post_form form data to use when making a `POST` request
 | 
			
		||||
      # @param post_json JSON data to use when making a `POST` request
 | 
			
		||||
      sig {
 | 
			
		||||
        params(
 | 
			
		||||
          homebrew_curl: T.nilable(T::Boolean),
 | 
			
		||||
          post_form:     T.nilable(T::Hash[Symbol, String]),
 | 
			
		||||
          post_json:     T.nilable(T::Hash[Symbol, String]),
 | 
			
		||||
        ).void
 | 
			
		||||
      }
 | 
			
		||||
      def initialize(homebrew_curl: nil, post_form: nil, post_json: nil)
 | 
			
		||||
        @homebrew_curl = homebrew_curl
 | 
			
		||||
        @post_form = post_form
 | 
			
		||||
        @post_json = post_json
 | 
			
		||||
      end
 | 
			
		||||
      prop :post_json, T.nilable(T::Hash[Symbol, String])
 | 
			
		||||
 | 
			
		||||
      # Returns a `Hash` of options that are provided as arguments to `url`.
 | 
			
		||||
      sig { returns(T::Hash[Symbol, T.untyped]) }
 | 
			
		||||
@ -47,25 +28,13 @@ module Homebrew
 | 
			
		||||
        }
 | 
			
		||||
      end
 | 
			
		||||
 | 
			
		||||
      # Returns a `Hash` of all instance variables, using `Symbol` keys.
 | 
			
		||||
      sig { returns(T::Hash[Symbol, T.untyped]) }
 | 
			
		||||
      def to_h
 | 
			
		||||
        {
 | 
			
		||||
          homebrew_curl:,
 | 
			
		||||
          post_form:,
 | 
			
		||||
          post_json:,
 | 
			
		||||
        }
 | 
			
		||||
      end
 | 
			
		||||
 | 
			
		||||
      # Returns a `Hash` of all instance variables, using `String` keys.
 | 
			
		||||
      sig { returns(T::Hash[String, T.untyped]) }
 | 
			
		||||
      def to_hash
 | 
			
		||||
        {
 | 
			
		||||
          "homebrew_curl" => @homebrew_curl,
 | 
			
		||||
          "post_form"     => @post_form,
 | 
			
		||||
          "post_json"     => @post_json,
 | 
			
		||||
        }
 | 
			
		||||
      end
 | 
			
		||||
      def to_hash = serialize
 | 
			
		||||
 | 
			
		||||
      # Returns a `Hash` of all instance variables, using `Symbol` keys.
 | 
			
		||||
      sig { returns(T::Hash[Symbol, T.untyped]) }
 | 
			
		||||
      def to_h = serialize.transform_keys(&:to_sym)
 | 
			
		||||
 | 
			
		||||
      # Returns a new object formed by merging `other` values with a copy of
 | 
			
		||||
      # `self`.
 | 
			
		||||
@ -78,7 +47,7 @@ module Homebrew
 | 
			
		||||
        return dup if other.empty?
 | 
			
		||||
 | 
			
		||||
        this_hash = to_h
 | 
			
		||||
        other_hash = other.is_a?(Options) ? other.to_h.compact : other
 | 
			
		||||
        other_hash = other.is_a?(Options) ? other.to_h : other
 | 
			
		||||
        return dup if this_hash == other_hash
 | 
			
		||||
 | 
			
		||||
        new_options = this_hash.merge(other_hash)
 | 
			
		||||
@ -96,15 +65,11 @@ module Homebrew
 | 
			
		||||
 | 
			
		||||
      # Whether the object has only default values.
 | 
			
		||||
      sig { returns(T::Boolean) }
 | 
			
		||||
      def empty?
 | 
			
		||||
        @homebrew_curl.nil? && @post_form.nil? && @post_json.nil?
 | 
			
		||||
      end
 | 
			
		||||
      def empty? = serialize.empty?
 | 
			
		||||
 | 
			
		||||
      # Whether the object has any non-default values.
 | 
			
		||||
      sig { returns(T::Boolean) }
 | 
			
		||||
      def present?
 | 
			
		||||
        !@homebrew_curl.nil? || !@post_form.nil? || !@post_json.nil?
 | 
			
		||||
      end
 | 
			
		||||
      def present? = !empty?
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
 | 
			
		||||
@ -39,21 +39,19 @@ RSpec.describe Homebrew::Livecheck::Options do
 | 
			
		||||
 | 
			
		||||
  describe "#to_h" do
 | 
			
		||||
    it "returns a Hash of all instance variables" do
 | 
			
		||||
      expect(options.new.to_h).to eq({
 | 
			
		||||
        homebrew_curl: nil,
 | 
			
		||||
        post_form:     nil,
 | 
			
		||||
        post_json:     nil,
 | 
			
		||||
      })
 | 
			
		||||
      # `T::Struct.serialize` omits `nil` values
 | 
			
		||||
      expect(options.new.to_h).to eq({})
 | 
			
		||||
 | 
			
		||||
      expect(options.new(**args).to_h).to eq(args)
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe "#to_hash" do
 | 
			
		||||
    it "returns a Hash of all instance variables, using String keys" do
 | 
			
		||||
      expect(options.new.to_hash).to eq({
 | 
			
		||||
        "homebrew_curl" => nil,
 | 
			
		||||
        "post_form"     => nil,
 | 
			
		||||
        "post_json"     => nil,
 | 
			
		||||
      })
 | 
			
		||||
      # `T::Struct.serialize` omits `nil` values
 | 
			
		||||
      expect(options.new.to_hash).to eq({})
 | 
			
		||||
 | 
			
		||||
      expect(options.new(**args).to_hash).to eq(args.transform_keys(&:to_s))
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
@ -65,6 +63,8 @@ RSpec.describe Homebrew::Livecheck::Options do
 | 
			
		||||
        .to eq(options.new(**merged_hash))
 | 
			
		||||
      expect(options.new(**args).merge(args))
 | 
			
		||||
        .to eq(options.new(**args))
 | 
			
		||||
      expect(options.new(**args).merge({}))
 | 
			
		||||
        .to eq(options.new(**args))
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
@ -82,6 +82,10 @@ RSpec.describe Homebrew::Livecheck::Options do
 | 
			
		||||
    it "returns false if any instance variables differ" do
 | 
			
		||||
      expect(options.new == options.new(**args)).to be false
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    it "returns false if other object is not the same class" do
 | 
			
		||||
      expect(options.new == :other).to be false
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe "#empty?" do
 | 
			
		||||
 | 
			
		||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user