From 424582ec9b660c10cdf717a6a4d723e2b3cbca99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Johannes=20M=C3=BCller?= <straightshoota@gmail.com>
Date: Wed, 13 Jan 2021 16:21:36 +0100
Subject: [PATCH] Refactor check_single_def_error_message (#10196)

* Refactor check_single_def_error_message

* Extract no_overload_matches_message
---
 src/compiler/crystal/semantic/call_error.cr | 107 ++++++++++----------
 1 file changed, 51 insertions(+), 56 deletions(-)

diff --git a/src/compiler/crystal/semantic/call_error.cr b/src/compiler/crystal/semantic/call_error.cr
index 381be1bc888a..109505089a25 100644
--- a/src/compiler/crystal/semantic/call_error.cr
+++ b/src/compiler/crystal/semantic/call_error.cr
@@ -114,9 +114,8 @@ class Crystal::Call
     end
 
     if defs_matching_args_size.size > 0
-      str = IO::Memory.new
-      if check_single_def_error_message(defs_matching_args_size, named_args_types, str)
-        raise str.to_s
+      if message = single_def_error_message(defs, named_args)
+        raise message
       else
         if block && defs_matching_args_size.all? { |a_def| !a_def.yields }
           raise "'#{full_name(owner, def_name)}' is not expected to be invoked with a block, but a block was given"
@@ -147,33 +146,8 @@ class Crystal::Call
       owner_trace = inner_exception
     end
 
-    arg_names = [] of Array(String)
-
     message = String.build do |msg|
-      unless check_single_def_error_message(defs, named_args_types, msg)
-        msg << "no overload matches '#{full_name(owner, def_name)}'"
-        unless args.empty?
-          msg << " with type"
-          msg << 's' if arg_types.size > 1 || named_args_types
-          msg << ' '
-          arg_types.join(msg, ", ")
-        end
-
-        if named_args_types
-          named_args_types.each do |named_arg|
-            msg << ", "
-            msg << named_arg.name
-            msg << ": "
-            msg << named_arg.type
-          end
-        end
-
-        msg << '\n'
-
-        defs.each do |a_def|
-          arg_names << a_def.args.map(&.name)
-        end
-      end
+      no_overload_matches_message(msg, full_name(owner, def_name), defs, args, arg_types, named_args_types)
 
       msg << "Overloads are:"
       append_matches(defs, arg_types, msg)
@@ -182,7 +156,8 @@ class Crystal::Call
         cover = matches.cover
         if cover.is_a?(Cover)
           missing = cover.missing
-          uniq_arg_names = arg_names.uniq!
+
+          uniq_arg_names = defs.map(&.args.map(&.name)).uniq!
           uniq_arg_names = uniq_arg_names.size == 1 ? uniq_arg_names.first : nil
           unless missing.empty?
             msg << "\nCouldn't find overloads for these types:"
@@ -211,6 +186,32 @@ class Crystal::Call
     raise message, owner_trace
   end
 
+  private def no_overload_matches_message(io, full_name, defs, args, arg_types, named_args_types)
+    if message = single_def_error_message(defs, named_args_types)
+      io << message
+      return
+    end
+
+    io << "no overload matches '#{full_name}'"
+    unless args.empty?
+      io << " with type"
+      io << 's' if arg_types.size > 1 || named_args_types
+      io << ' '
+      arg_types.join(io, ", ")
+    end
+
+    if named_args_types
+      named_args_types.each do |named_arg|
+        io << ", "
+        io << named_arg.name
+        io << ": "
+        io << named_arg.type
+      end
+    end
+
+    io << '\n'
+  end
+
   private def raise_undefined_method(owner, def_name, obj)
     check_macro_wrong_number_of_arguments(def_name)
 
@@ -298,7 +299,9 @@ class Crystal::Call
     all_arguments_sizes.uniq!.sort!
 
     raise(String.build do |str|
-      unless check_single_def_error_message(defs, named_args_types, str)
+      if single_message = single_def_error_message(defs, named_args_types)
+        str << single_message
+      else
         str << "wrong number of arguments for '"
         str << full_name(owner, def_name)
         str << "' (given "
@@ -331,23 +334,24 @@ class Crystal::Call
     end
   end
 
-  # If there's only one def that could match, and there are named
-  # arguments in this call, we can give a better error message.
-  def check_single_def_error_message(defs, named_args, io)
-    return false unless defs.size == 1
+  def single_def_error_message(defs, named_args)
+    if defs.size == 1
+      missing_argument_message(defs.first, named_args)
+    end
+  end
 
-    a_def = defs.first
+  def missing_argument_message(a_def, named_args)
+    missing_args = extract_missing_args(a_def, named_args)
+    return unless missing_args
 
-    if msg = check_named_args_and_splats(a_def, named_args)
-      io << msg
-      io.puts
-      return true
+    if missing_args.size == 1
+      "missing argument: #{missing_args.first}"
+    else
+      "missing arguments: #{missing_args.join ", "}"
     end
-
-    false
   end
 
-  def check_named_args_and_splats(a_def, named_args)
+  def extract_missing_args(a_def, named_args)
     splat_index = a_def.splat_index
     return if !splat_index && !named_args
     return if splat_index == a_def.args.size - 1
@@ -376,16 +380,9 @@ class Crystal::Call
       missing_args << arg.external_name
     end
 
-    case missing_args.size
-    when 0
-      # Nothing
-    when 1
-      return "missing argument: #{missing_args.first}"
-    else
-      return "missing arguments: #{missing_args.join ", "}"
-    end
+    return if missing_args.size.zero?
 
-    return nil
+    missing_args
   end
 
   def append_error_when_no_matching_defs(owner, def_name, all_arguments_sizes, real_args_size, min_splat, defs, io)
@@ -529,10 +526,8 @@ class Crystal::Call
     return unless macros.is_a?(Array(Macro))
     macros = macros.reject &.visibility.private?
 
-    if macros.size == 1
-      if msg = check_named_args_and_splats(macros.first, named_args)
-        raise msg
-      end
+    if msg = single_def_error_message(macros, named_args)
+      raise msg
     end
 
     all_arguments_sizes = Set(String).new