Sort completion candidates

Method candidates were returned in method table order, which looks
random in the completion dialog. Sort all candidates in one place at
the end of completion_candidates and drop the per-branch sorts.
Candidates starting with an underscore go last, so that completing an
empty method name does not begin with __id__, __send__ and other
internals.

Author: kryzhovnik

lib/repl_type_completor/result.rb

@@ -52,13 +52,13 @@ def completion_candidates
           scope_constants = type.types.flat_map do |t|
             scope.table_module_constants(t.module_or_class) if t.is_a?(Types::SingletonType)
           end
-          (scope_constants.compact | type.constants.map(&:to_s)).sort
+          scope_constants.compact | type.constants.map(&:to_s)
         else
-          scope.constants.sort | RESERVED_WORDS
+          scope.constants | RESERVED_WORDS
         end
       in [:ivar, name, scope]
-        ivars = scope.instance_variables.sort
-        name == '@' ? ivars + scope.class_variables.sort : ivars
+        ivars = scope.instance_variables
+        name == '@' ? ivars + scope.class_variables : ivars
       in [:cvar, name, scope]
         scope.class_variables
       in [:gvar, name, scope]
@@ -70,13 +70,13 @@ def completion_candidates
         keys = receiver_type.types.grep(Types::InstanceType).select do |t|
           Hash == t.klass
         end.flat_map do |t|
-          t.instances.flat_map(&:keys).grep(key_type).uniq.sort
+          t.instances.flat_map(&:keys).grep(key_type).uniq
         end
         if key_type == Symbol
           keys = Symbol.all_symbols if keys.empty? && name.size >= 1
           filter_symbol_candidates(keys, name, limit: 100)
         else
-          keys.select { _1.start_with?(name) }.sort
+          keys
         end
       in [:call, name, type, self_call]
         (self_call ? type.all_methods : type.methods).map(&:to_s) - HIDDEN_METHODS
@@ -87,9 +87,12 @@ def completion_candidates
       end
 
       candidates.filter_map do
-        _1[name.size..] if _1.start_with?(name)
+        _1 if _1.start_with?(name)
       rescue EncodingError
-      end
+      end.sort_by do
+        # Alphabetical order, but internal methods (leading underscore) last
+        [_1.start_with?('_') ? 1 : 0, _1]
+      end.map { _1[name.size..] }
     rescue Exception => e
       ReplTypeCompletor.handle_exception(e)
       []

test/repl_type_completor/test_repl_type_completor.rb

@@ -293,6 +293,33 @@ def with_failing_method(klass, method_name, message)
       klass.define_method(method_name, original_method)
     end
 
+    def test_completion_candidates_are_sorted
+      xzyb, xzya, xzyc = 'b', 'a', 'c'
+      lvar_binding = binding
+
+      {
+        '1.' => empty_binding,     # [:call] with empty method name
+        "''.s" => empty_binding,   # [:call] with method name prefix
+        'Math::' => empty_binding, # [:call_or_const]
+        '$std' => empty_binding,   # [:gvar]
+        'xzy' => lvar_binding      # [:lvar_or_method]
+      }.each do |code, bind|
+        candidates = ReplTypeCompletor.analyze(code, binding: bind).completion_candidates
+        refute_empty candidates, "Expected completion candidates of #{code.inspect} not to be empty"
+        underscore, regular = candidates.partition { _1.start_with?('_') }
+        assert_equal regular.sort + underscore.sort, candidates, "Expected completion candidates of #{code.inspect} to be sorted with underscore methods last"
+      end
+
+      # Internal methods (leading underscore, e.g. __id__ and __send__) are listed last
+      candidates = ReplTypeCompletor.analyze('1.', binding: empty_binding).completion_candidates
+      assert candidates.any? { _1.start_with?('_') }
+      refute candidates.first.start_with?('_')
+      assert candidates.last.start_with?('_')
+
+      # Local variables xzyb, xzya, xzyc are defined in non-alphabetical order
+      assert_equal [xzya, xzyb, xzyc], ReplTypeCompletor.analyze('xzy', binding: lvar_binding).completion_candidates
+    end
+
     def test_analyze_error
       with_failing_method(ReplTypeCompletor.singleton_class, :analyze_code, 'error_in_analyze_code') do
         assert_nil ReplTypeCompletor.analyze('1.', binding: binding)