Add a new initialization step to validate IRB.conf's values (#953)

Currently, users can only find out that they have set a wrong value
for IRB configs when the value is used, with opaque error messages like
"comparison of Integer with true failed (TypeError)".

This commit adds a new initialization step to validate the values of
some IRB configs, so that users can find out about the wrong values
during the initialization of IRB.

lib/irb/context.rb

@@ -73,11 +73,12 @@ def initialize(irb, workspace = nil, input_method = nil)
 
       self.prompt_mode = IRB.conf[:PROMPT_MODE]
 
-      if IRB.conf[:SINGLE_IRB] or !defined?(IRB::JobManager)
-        @irb_name = IRB.conf[:IRB_NAME]
-      else
-        @irb_name = IRB.conf[:IRB_NAME]+"#"+IRB.JobManager.n_jobs.to_s
+      @irb_name = IRB.conf[:IRB_NAME]
+
+      unless IRB.conf[:SINGLE_IRB] or !defined?(IRB::JobManager)
+        @irb_name = @irb_name + "#" + IRB.JobManager.n_jobs.to_s
       end
+
       self.irb_path = "(" + @irb_name + ")"
 
       case input_method

lib/irb/init.rb

@@ -52,6 +52,7 @@ def IRB.setup(ap_path, argv: ::ARGV)
     IRB.init_error
     IRB.parse_opts(argv: argv)
     IRB.run_config
+    IRB.validate_config
     IRB.load_modules
 
     unless @CONF[:PROMPT][@CONF[:PROMPT_MODE]]
@@ -427,6 +428,40 @@ def IRB.irbrc_files
     @irbrc_files
   end
 
+  def IRB.validate_config
+    conf[:IRB_NAME] = conf[:IRB_NAME].to_s
+
+    irb_rc = conf[:IRB_RC]
+    unless irb_rc.nil? || irb_rc.respond_to?(:call)
+      raise_validation_error "IRB.conf[:IRB_RC] should be a callable object. Got #{irb_rc.inspect}."
+    end
+
+    back_trace_limit = conf[:BACK_TRACE_LIMIT]
+    unless back_trace_limit.is_a?(Integer)
+      raise_validation_error "IRB.conf[:BACK_TRACE_LIMIT] should be an integer. Got #{back_trace_limit.inspect}."
+    end
+
+    prompt = conf[:PROMPT]
+    unless prompt.is_a?(Hash)
+      msg = "IRB.conf[:PROMPT] should be a Hash. Got #{prompt.inspect}."
+
+      if prompt.is_a?(Symbol)
+        msg += " Did you mean to set `IRB.conf[:PROMPT_MODE]`?"
+      end
+
+      raise_validation_error msg
+    end
+
+    eval_history = conf[:EVAL_HISTORY]
+    unless eval_history.nil? || eval_history.is_a?(Integer)
+      raise_validation_error "IRB.conf[:EVAL_HISTORY] should be an integer. Got #{eval_history.inspect}."
+    end
+  end
+
+  def IRB.raise_validation_error(msg)
+    raise TypeError, msg, @irbrc_files
+  end
+
   # loading modules
   def IRB.load_modules
     for m in @CONF[:LOAD_MODULES]

test/irb/test_init.rb

@@ -268,15 +268,94 @@ def with_argv(argv)
     end
   end
 
+  class ConfigValidationTest < TestCase
+    def setup
+      @original_home = ENV["HOME"]
+      @original_irbrc = ENV["IRBRC"]
+      # To prevent the test from using the user's .irbrc file
+      ENV["HOME"] = Dir.mktmpdir
+      IRB.instance_variable_set(:@existing_rc_name_generators, nil)
+      super
+    end
+
+    def teardown
+      super
+      ENV["IRBRC"] = @original_irbrc
+      ENV["HOME"] = @original_home
+      File.unlink(@irbrc)
+    end
+
+    def test_irb_name_converts_non_string_values_to_string
+      assert_no_irb_validation_error(<<~'RUBY')
+        IRB.conf[:IRB_NAME] = :foo
+      RUBY
+
+      assert_equal "foo", IRB.conf[:IRB_NAME]
+    end
+
+    def test_irb_rc_name_only_takes_callable_objects
+      assert_irb_validation_error(<<~'RUBY', "IRB.conf[:IRB_RC] should be a callable object. Got :foo.")
+        IRB.conf[:IRB_RC] = :foo
+      RUBY
+    end
+
+    def test_back_trace_limit_only_accepts_integers
+      assert_irb_validation_error(<<~'RUBY', "IRB.conf[:BACK_TRACE_LIMIT] should be an integer. Got \"foo\".")
+        IRB.conf[:BACK_TRACE_LIMIT] = "foo"
+      RUBY
+    end
+
+    def test_prompt_only_accepts_hash
+      assert_irb_validation_error(<<~'RUBY', "IRB.conf[:PROMPT] should be a Hash. Got \"foo\".")
+        IRB.conf[:PROMPT] = "foo"
+      RUBY
+    end
+
+    def test_eval_history_only_accepts_integers
+      assert_irb_validation_error(<<~'RUBY', "IRB.conf[:EVAL_HISTORY] should be an integer. Got \"foo\".")
+        IRB.conf[:EVAL_HISTORY] = "foo"
+      RUBY
+    end
+
+    private
+
+    def assert_irb_validation_error(rc_content, error_message)
+      write_rc rc_content
+
+      assert_raise_with_message(TypeError, error_message) do
+        IRB.setup(__FILE__)
+      end
+    end
+
+    def assert_no_irb_validation_error(rc_content)
+      write_rc rc_content
+
+      assert_nothing_raised do
+        IRB.setup(__FILE__)
+      end
+    end
+
+    def write_rc(content)
+      @irbrc = Tempfile.new('irbrc')
+      @irbrc.write(content)
+      @irbrc.close
+      ENV['IRBRC'] = @irbrc.path
+    end
+  end
+
   class InitIntegrationTest < IntegrationTestCase
-    def test_load_error_in_rc_file_is_warned
-      write_rc <<~'IRBRC'
-        require "file_that_does_not_exist"
-      IRBRC
+    def setup
+      super
 
       write_ruby <<~'RUBY'
         binding.irb
       RUBY
+    end
+
+    def test_load_error_in_rc_file_is_warned
+      write_rc <<~'IRBRC'
+        require "file_that_does_not_exist"
+      IRBRC
 
       output = run_ruby_file do
         type "'foobar'"
@@ -293,10 +372,6 @@ def test_normal_errors_in_rc_file_is_warned
         raise "I'm an error"
       IRBRC
 
-      write_ruby <<~'RUBY'
-        binding.irb
-      RUBY
-
       output = run_ruby_file do
         type "'foobar'"
         type "exit"