From c15c5c6f1b6d8c7f92e1e52b3987b7eecb487f40 Mon Sep 17 00:00:00 2001
From: Marc-Andre Lafortune <github@marc-andre.ca>
Date: Mon, 24 Sep 2018 18:48:01 -0400
Subject: [PATCH] Always check default type. Deprecate incompatible types
 without explicit option given [#621]

---
 lib/thor.rb               |  7 +++++++
 lib/thor/base.rb          | 15 +++++++++------
 lib/thor/parser/option.rb | 13 ++++++++++---
 spec/thor_spec.rb         | 24 ++++++++++++++++++++++--
 4 files changed, 48 insertions(+), 11 deletions(-)

diff --git a/lib/thor.rb b/lib/thor.rb
index 1409623b3..f004981ce 100644
--- a/lib/thor.rb
+++ b/lib/thor.rb
@@ -339,6 +339,13 @@ def disable_required_check?(command) #:nodoc:
       command && disable_required_check.include?(command.name.to_sym)
     end
 
+    def deprecation_warning(message) #:nodoc:
+      unless ENV['THOR_SILENCE_DEPRECATION']
+        warn "Deprecation warning: #{message}\n" +
+          'You can silence deprecations warning by setting the environment variable THOR_SILENCE_DEPRECATION.'
+      end
+    end
+
   protected
 
     def stop_on_unknown_option #:nodoc:
diff --git a/lib/thor/base.rb b/lib/thor/base.rb
index c5f004e3e..b9d207751 100644
--- a/lib/thor/base.rb
+++ b/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, options.merge(:check_default_type => check_default_type))
       end
 
       # Receives a hash of options, parse them and add to the scope. This is a
diff --git a/lib/thor/parser/option.rb b/lib/thor/parser/option.rb
index f434b4704..163423229 100644
--- a/lib/thor/parser/option.rb
+++ b/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,15 @@ 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`.'
+        end
+      end
     end
 
     def dasherized?
diff --git a/spec/thor_spec.rb b/spec/thor_spec.rb
index fd1e2a609..c938906a9 100644
--- a/spec/thor_spec.rb
+++ b/spec/thor_spec.rb
@@ -694,12 +694,32 @@ 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
+          allow_incompatible_default_type!
+
+          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