Always check default type. Deprecate incompatible types without explicit option given [#621]

Author: marcandre

lib/thor/base.rb

@@ -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
@@ -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

lib/thor/parser/option.rb

@@ -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!
@@ -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?

spec/thor_spec.rb

@@ -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