Skip to content

Commit

Permalink
Always check default type. Deprecate incompatible types without expli…
Browse files Browse the repository at this point in the history
…cit option given [rails#621]
  • Loading branch information
marcandre committed Oct 2, 2018
1 parent b2b1e7a commit 7d199c0
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 11 deletions.
15 changes: 9 additions & 6 deletions lib/thor/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,17 +153,20 @@ def check_unknown_options?(config) #:nodoc:

# If you want to raise an error when the default value of an option does not match
# the type call check_default_type!
# This is disabled by default for compatibility.
# This will be the default; for compatibility a deprecation warning is issued if necessary.
def check_default_type!
@check_default_type = true
end

def check_default_type #:nodoc:
@check_default_type ||= from_superclass(:check_default_type, false)
# If you want to use defaults that don't match the type of an option,
# either specify `check_default_type: false` or call `allow_incompatible_default_type!`
def allow_incompatible_default_type!
@check_default_type = false
end

def check_default_type? #:nodoc:
!!check_default_type
def check_default_type #:nodoc:
@check_default_type = from_superclass(:check_default_type, nil) unless defined?(@check_default_type)
@check_default_type
end

# If true, option parsing is suspended as soon as an unknown option or a
Expand Down Expand Up @@ -564,7 +567,7 @@ def is_thor_reserved_word?(word, type) #:nodoc:
# options<Hash>:: Described in both class_option and method_option.
# scope<Hash>:: Options hash that is being built up
def build_option(name, options, scope) #:nodoc:
scope[name] = Thor::Option.new(name, options.merge(:check_default_type => check_default_type?))
scope[name] = Thor::Option.new(name, {:check_default_type => check_default_type}.merge!(options))
end

# Receives a hash of options, parse them and add to the scope. This is a
Expand Down
14 changes: 11 additions & 3 deletions lib/thor/parser/option.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ def #{type}?

def validate!
raise ArgumentError, "An option cannot be boolean and required." if boolean? && required?
validate_default_type! if @check_default_type
validate_default_type!
end

def validate_default_type!
Expand All @@ -127,8 +127,16 @@ def validate_default_type!
when Hash, Array, String
@default.class.name.downcase.to_sym
end

raise ArgumentError, "Expected #{@type} default value for '#{switch_name}'; got #{@default.inspect} (#{default_type})" unless default_type == @type
if default_type != @type
err = "Expected #{@type} default value for '#{switch_name}'; got #{@default.inspect} (#{default_type})"
if @check_default_type
raise ArgumentError, err
elsif @check_default_type == nil
Thor.deprecation_warning "#{err}.\n" +
'This will be rejected in the future unless you explicitly pass the options `check_default_type: false`' +
' or call `allow_incompatible_default_type!` in your code'
end
end
end

def dasherized?
Expand Down
22 changes: 20 additions & 2 deletions spec/thor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -694,12 +694,30 @@ def unknown(*args)
expect(klass.start(%w(unknown foo --bar baz))).to eq(%w(foo))
end

it "does not check the default type when check_default_type! is not called" do
it "issues a deprecation warning on incompatible types by default" do
expect do
Class.new(Thor) do
option "bar", :type => :numeric, :default => "foo"
end
end.not_to raise_error
end.to output(/^Deprecation warning/).to_stderr
end

it "allows incompatible types if allow_incompatible_default_type! is called" do
expect do
Class.new(Thor) do
allow_incompatible_default_type!

option "bar", :type => :numeric, :default => "foo"
end
end.not_to output.to_stderr
end

it "allows incompatible types if `check_default_type: false` is given" do
expect do
Class.new(Thor) do
option "bar", :type => :numeric, :default => "foo", :check_default_type => false
end
end.not_to output.to_stderr
end

it "checks the default type when check_default_type! is called" do
Expand Down

0 comments on commit 7d199c0

Please sign in to comment.