Stop using ExtendCommandBundle internally (#925)

This module was used to extend both commands and helpers when they're not
separated. Now that they are, and we have a Command module, we should move
command-related logic to the Command module and update related references.

This will make the code easier to understand and refactor in the future.

lib/irb.rb

@@ -1120,7 +1120,7 @@ def build_statement(code)
 
       code.force_encoding(@context.io.encoding)
       if (command, arg = parse_command(code))
-        command_class = ExtendCommandBundle.load_command(command)
+        command_class = Command.load_command(command)
         Statement::Command.new(code, command_class, arg)
       else
         is_assignment_expression = @scanner.assignment_expression?(code, local_variables: @context.local_variables)
@@ -1142,7 +1142,7 @@ def parse_command(code)
       # Check visibility
       public_method = !!Kernel.instance_method(:public_method).bind_call(@context.main, command) rescue false
       private_method = !public_method && !!Kernel.instance_method(:method).bind_call(@context.main, command) rescue false
-      if ExtendCommandBundle.execute_as_command?(command, public_method: public_method, private_method: private_method)
+      if Command.execute_as_command?(command, public_method: public_method, private_method: private_method)
         [command, arg]
       end
     end

lib/irb/command.rb

@@ -18,12 +18,6 @@ class << self
       def register(name, command_class)
         @commands[name] = [command_class, []]
       end
-
-      # This API is for IRB's internal use only and may change at any time.
-      # Please do NOT use it.
-      def _register_with_aliases(name, command_class, *aliases)
-        @commands[name] = [command_class, aliases]
-      end
     end
   end
 end

lib/irb/command/help.rb

@@ -11,7 +11,7 @@ def execute(command_name)
           if command_name.empty?
             help_message
           else
-            if command_class = ExtendCommandBundle.load_command(command_name)
+            if command_class = Command.load_command(command_name)
               command_class.help_message || command_class.description
             else
               "Can't find command `#{command_name}`. Please check the command name and try again.\n\n"
@@ -23,7 +23,7 @@ def execute(command_name)
       private
 
       def help_message
-        commands_info = IRB::ExtendCommandBundle.all_commands_info
+        commands_info = IRB::Command.all_commands_info
         commands_grouped_by_categories = commands_info.group_by { |cmd| cmd[:category] }
 
         user_aliases = irb_context.instance_variable_get(:@user_aliases)

lib/irb/completion.rb

@@ -88,7 +88,7 @@ def retrieve_files_to_require_from_load_path
 
     def command_completions(preposing, target)
       if preposing.empty? && !target.empty?
-        IRB::ExtendCommandBundle.command_names.select { _1.start_with?(target) }
+        IRB::Command.command_names.select { _1.start_with?(target) }
       else
         []
       end

lib/irb/default_commands.rb

@@ -30,35 +30,91 @@
 require_relative "command/history"
 
 module IRB
-  ExtendCommand = Command
-
-  # Installs the default irb extensions command bundle.
-  module ExtendCommandBundle
-    # See #install_alias_method.
+  module Command
     NO_OVERRIDE = 0
-    # See #install_alias_method.
     OVERRIDE_PRIVATE_ONLY = 0x01
-    # See #install_alias_method.
     OVERRIDE_ALL = 0x02
 
-    Command._register_with_aliases(:irb_context, Command::Context,
+    class << self
+      # This API is for IRB's internal use only and may change at any time.
+      # Please do NOT use it.
+      def _register_with_aliases(name, command_class, *aliases)
+        @commands[name] = [command_class, aliases]
+      end
+
+      def all_commands_info
+        user_aliases = IRB.CurrentContext.command_aliases.each_with_object({}) do |(alias_name, target), result|
+          result[target] ||= []
+          result[target] << alias_name
+        end
+
+        commands.map do |command_name, (command_class, aliases)|
+          aliases = aliases.map { |a| a.first }
+
+          if additional_aliases = user_aliases[command_name]
+            aliases += additional_aliases
+          end
+
+          display_name = aliases.shift || command_name
+          {
+            display_name: display_name,
+            description: command_class.description,
+            category: command_class.category
+          }
+        end
+      end
+
+      def command_override_policies
+        @@command_override_policies ||= commands.flat_map do |cmd_name, (cmd_class, aliases)|
+          [[cmd_name, OVERRIDE_ALL]] + aliases
+        end.to_h
+      end
+
+      def execute_as_command?(name, public_method:, private_method:)
+        case command_override_policies[name]
+        when OVERRIDE_ALL
+          true
+        when OVERRIDE_PRIVATE_ONLY
+          !public_method
+        when NO_OVERRIDE
+          !public_method && !private_method
+        end
+      end
+
+      def command_names
+        command_override_policies.keys.map(&:to_s)
+      end
+
+      # Convert a command name to its implementation class if such command exists
+      def load_command(command)
+        command = command.to_sym
+        commands.each do |command_name, (command_class, aliases)|
+          if command_name == command || aliases.any? { |alias_name, _| alias_name == command }
+            return command_class
+          end
+        end
+        nil
+      end
+    end
+
+    _register_with_aliases(:irb_context, Command::Context,
       [
         [:context, NO_OVERRIDE],
         [:conf, NO_OVERRIDE],
       ],
     )
 
-    Command._register_with_aliases(:irb_exit, Command::Exit,
+    _register_with_aliases(:irb_exit, Command::Exit,
       [:exit, OVERRIDE_PRIVATE_ONLY],
       [:quit, OVERRIDE_PRIVATE_ONLY],
       [:irb_quit, OVERRIDE_PRIVATE_ONLY]
     )
 
-    Command._register_with_aliases(:irb_exit!, Command::ForceExit,
+    _register_with_aliases(:irb_exit!, Command::ForceExit,
       [:exit!, OVERRIDE_PRIVATE_ONLY]
     )
 
-    Command._register_with_aliases(:irb_current_working_workspace, Command::CurrentWorkingWorkspace,
+    _register_with_aliases(:irb_current_working_workspace, Command::CurrentWorkingWorkspace,
       [:cwws, NO_OVERRIDE],
       [:pwws, NO_OVERRIDE],
       [:irb_print_working_workspace, OVERRIDE_ALL],
@@ -70,7 +126,7 @@ module ExtendCommandBundle
       [:irb_pwb, OVERRIDE_ALL],
     )
 
-    Command._register_with_aliases(:irb_change_workspace, Command::ChangeWorkspace,
+    _register_with_aliases(:irb_change_workspace, Command::ChangeWorkspace,
       [:chws, NO_OVERRIDE],
       [:cws, NO_OVERRIDE],
       [:irb_chws, OVERRIDE_ALL],
@@ -80,170 +136,128 @@ module ExtendCommandBundle
       [:cb, NO_OVERRIDE],
     )
 
-    Command._register_with_aliases(:irb_workspaces, Command::Workspaces,
+    _register_with_aliases(:irb_workspaces, Command::Workspaces,
       [:workspaces, NO_OVERRIDE],
       [:irb_bindings, OVERRIDE_ALL],
       [:bindings, NO_OVERRIDE],
     )
 
-    Command._register_with_aliases(:irb_push_workspace, Command::PushWorkspace,
+    _register_with_aliases(:irb_push_workspace, Command::PushWorkspace,
       [:pushws, NO_OVERRIDE],
       [:irb_pushws, OVERRIDE_ALL],
       [:irb_push_binding, OVERRIDE_ALL],
       [:irb_pushb, OVERRIDE_ALL],
       [:pushb, NO_OVERRIDE],
     )
 
-    Command._register_with_aliases(:irb_pop_workspace, Command::PopWorkspace,
+    _register_with_aliases(:irb_pop_workspace, Command::PopWorkspace,
       [:popws, NO_OVERRIDE],
       [:irb_popws, OVERRIDE_ALL],
       [:irb_pop_binding, OVERRIDE_ALL],
       [:irb_popb, OVERRIDE_ALL],
       [:popb, NO_OVERRIDE],
     )
 
-    Command._register_with_aliases(:irb_load, Command::Load)
-    Command._register_with_aliases(:irb_require, Command::Require)
-    Command._register_with_aliases(:irb_source, Command::Source,
+    _register_with_aliases(:irb_load, Command::Load)
+    _register_with_aliases(:irb_require, Command::Require)
+    _register_with_aliases(:irb_source, Command::Source,
       [:source, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb, Command::IrbCommand)
-    Command._register_with_aliases(:irb_jobs, Command::Jobs,
+    _register_with_aliases(:irb, Command::IrbCommand)
+    _register_with_aliases(:irb_jobs, Command::Jobs,
       [:jobs, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_fg, Command::Foreground,
+    _register_with_aliases(:irb_fg, Command::Foreground,
       [:fg, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_kill, Command::Kill,
+    _register_with_aliases(:irb_kill, Command::Kill,
       [:kill, OVERRIDE_PRIVATE_ONLY]
     )
 
-    Command._register_with_aliases(:irb_debug, Command::Debug,
+    _register_with_aliases(:irb_debug, Command::Debug,
       [:debug, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_edit, Command::Edit,
+    _register_with_aliases(:irb_edit, Command::Edit,
       [:edit, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_break, Command::Break)
-    Command._register_with_aliases(:irb_catch, Command::Catch)
-    Command._register_with_aliases(:irb_next, Command::Next)
-    Command._register_with_aliases(:irb_delete, Command::Delete,
+    _register_with_aliases(:irb_break, Command::Break)
+    _register_with_aliases(:irb_catch, Command::Catch)
+    _register_with_aliases(:irb_next, Command::Next)
+    _register_with_aliases(:irb_delete, Command::Delete,
       [:delete, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_step, Command::Step,
+    _register_with_aliases(:irb_step, Command::Step,
       [:step, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_continue, Command::Continue,
+    _register_with_aliases(:irb_continue, Command::Continue,
       [:continue, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_finish, Command::Finish,
+    _register_with_aliases(:irb_finish, Command::Finish,
       [:finish, NO_OVERRIDE]
     )
-    Command._register_with_aliases(:irb_backtrace, Command::Backtrace,
+    _register_with_aliases(:irb_backtrace, Command::Backtrace,
       [:backtrace, NO_OVERRIDE],
       [:bt, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_debug_info, Command::Info,
+    _register_with_aliases(:irb_debug_info, Command::Info,
       [:info, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_help, Command::Help,
+    _register_with_aliases(:irb_help, Command::Help,
       [:help, NO_OVERRIDE],
       [:show_cmds, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_show_doc, Command::ShowDoc,
+    _register_with_aliases(:irb_show_doc, Command::ShowDoc,
       [:show_doc, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_info, Command::IrbInfo)
+    _register_with_aliases(:irb_info, Command::IrbInfo)
 
-    Command._register_with_aliases(:irb_ls, Command::Ls,
+    _register_with_aliases(:irb_ls, Command::Ls,
       [:ls, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_measure, Command::Measure,
+    _register_with_aliases(:irb_measure, Command::Measure,
       [:measure, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_show_source, Command::ShowSource,
+    _register_with_aliases(:irb_show_source, Command::ShowSource,
       [:show_source, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_whereami, Command::Whereami,
+    _register_with_aliases(:irb_whereami, Command::Whereami,
       [:whereami, NO_OVERRIDE]
     )
 
-    Command._register_with_aliases(:irb_history, Command::History,
+    _register_with_aliases(:irb_history, Command::History,
       [:history, NO_OVERRIDE],
       [:hist, NO_OVERRIDE]
     )
+  end
 
-    def self.all_commands_info
-      user_aliases = IRB.CurrentContext.command_aliases.each_with_object({}) do |(alias_name, target), result|
-        result[target] ||= []
-        result[target] << alias_name
-      end
-
-      Command.commands.map do |command_name, (command_class, aliases)|
-        aliases = aliases.map { |a| a.first }
-
-        if additional_aliases = user_aliases[command_name]
-          aliases += additional_aliases
-        end
-
-        display_name = aliases.shift || command_name
-        {
-          display_name: display_name,
-          description: command_class.description,
-          category: command_class.category
-        }
-      end
-    end
-
-    def self.command_override_policies
-      @@command_override_policies ||= Command.commands.flat_map do |cmd_name, (cmd_class, aliases)|
-        [[cmd_name, OVERRIDE_ALL]] + aliases
-      end.to_h
-    end
-
-    def self.execute_as_command?(name, public_method:, private_method:)
-      case command_override_policies[name]
-      when OVERRIDE_ALL
-        true
-      when OVERRIDE_PRIVATE_ONLY
-        !public_method
-      when NO_OVERRIDE
-        !public_method && !private_method
-      end
-    end
-
-    def self.command_names
-      command_override_policies.keys.map(&:to_s)
-    end
+  ExtendCommand = Command
 
-    # Convert a command name to its implementation class if such command exists
-    def self.load_command(command)
-      command = command.to_sym
-      Command.commands.each do |command_name, (command_class, aliases)|
-        if command_name == command || aliases.any? { |alias_name, _| alias_name == command }
-          return command_class
-        end
-      end
-      nil
-    end
+  # For backward compatibility, we need to keep this module:
+  # - As a container of helper methods
+  # - As a place to register commands with the deprecated def_extend_command method
+  module ExtendCommandBundle
+    # For backward compatibility
+    NO_OVERRIDE = Command::NO_OVERRIDE
+    OVERRIDE_PRIVATE_ONLY = Command::OVERRIDE_PRIVATE_ONLY
+    OVERRIDE_ALL = Command::OVERRIDE_ALL
 
     # Deprecated. Doesn't have any effect.
     @EXTEND_COMMANDS = []
 
     # Drepcated. Use Command.regiser instead.
     def self.def_extend_command(cmd_name, cmd_class, _, *aliases)
       Command._register_with_aliases(cmd_name, cmd_class, *aliases)
-      @@command_override_policies = nil
+      Command.class_variable_set(:@@command_override_policies, nil)
     end
   end
 end