Merge pull request #7384 from asterite/feature/improve-undefined-meth…

…od-error

Compiler: always mention the scope (and with ... yield scope, if any) on undefined method error

spec/compiler/semantic/did_you_mean_spec.cr

@@ -237,8 +237,6 @@ describe "Semantic: did you mean" do
   end
 
   it "suggests a better alternative to logical operators (#2715)" do
-    message = "undefined method 'and'"
-    message = " (did you mean '&&'?)".colorize.yellow.bold.to_s
     assert_error %(
       def rand(x : Int32)
       end
@@ -252,7 +250,8 @@ describe "Semantic: did you mean" do
       if "a".bytes and 1
         1
       end
-      ), message
+      ),
+      "undefined method 'and' for top-level (did you mean '&&'?)"
   end
 
   it "says did you mean in instance var declaration" do

spec/compiler/semantic/module_spec.cr

@@ -914,7 +914,7 @@ describe "Semantic: module" do
         end
       end
       Moo.new),
-      "undefined local variable or method 'allocate'#{" (modules cannot be instantiated)".colorize.yellow.bold}"
+      "undefined local variable or method 'allocate' for Moo:Module (modules cannot be instantiated)"
   end
 
   it "gives error when trying to instantiate with allocate" do
@@ -924,7 +924,7 @@ describe "Semantic: module" do
         end
       end
       Moo.allocate),
-      "undefined method 'allocate' for Moo:Module#{" (modules cannot be instantiated)".colorize.yellow.bold}"
+      "undefined method 'allocate' for Moo:Module (modules cannot be instantiated)"
   end
 
   it "uses type declaration inside module" do

spec/compiler/semantic/yield_with_scope_spec.cr

@@ -186,4 +186,23 @@ describe "Semantic: yield with scope" do
       Bar.new.bar
       )) { int32 }
   end
+
+  it "mentions with yield scope and current scope in error" do
+    assert_error %(
+      def foo
+        with 1 yield
+      end
+
+      class Foo
+        def bar
+          foo do
+            baz
+          end
+        end
+      end
+
+      Foo.new.bar
+      ),
+      "undefined local variable or method 'baz' for Int32 (with ... yield) and Foo (current scope)"
+  end
 end

spec/spec_helper.cr

@@ -47,7 +47,7 @@ def semantic(code : String, wants_doc = false, inject_primitives = true)
 end
 
 def semantic(node : ASTNode, wants_doc = false)
-  program = Program.new
+  program = new_program
   program.wants_doc = wants_doc
   node = program.normalize node
   node = program.semantic node
@@ -56,7 +56,7 @@ end
 
 def semantic_result(str, flags = nil, inject_primitives = true)
   str = inject_primitives(str) if inject_primitives
-  program = Program.new
+  program = new_program
   program.flags.concat(flags.split) if flags
   input = parse str
   input = program.normalize input
@@ -66,7 +66,7 @@ def semantic_result(str, flags = nil, inject_primitives = true)
 end
 
 def assert_normalize(from, to, flags = nil)
-  program = Program.new
+  program = new_program
   program.flags.concat(flags.split) if flags
   normalizer = Normalizer.new(program)
   from_nodes = Parser.parse(from)
@@ -79,7 +79,7 @@ def assert_expand(from : String, to)
 end
 
 def assert_expand(from_nodes : ASTNode, to)
-  to_nodes = LiteralExpander.new(Program.new).expand(from_nodes)
+  to_nodes = LiteralExpander.new(new_program).expand(from_nodes)
   to_nodes.to_s.strip.should eq(to.strip)
 end
 
@@ -113,7 +113,7 @@ def assert_macro(macro_args, macro_body, call_args, expected, expected_pragmas =
 end
 
 def assert_macro(macro_args, macro_body, expected, expected_pragmas = nil, flags = nil)
-  program = Program.new
+  program = new_program
   program.flags.concat(flags.split) if flags
   sub_node = yield program
   assert_macro_internal program, sub_node, macro_args, macro_body, expected, expected_pragmas
@@ -137,6 +137,12 @@ def codegen(code, inject_primitives = true, debug = Crystal::Debug::None)
   result.program.codegen(result.node, single_module: false, debug: debug)[""].mod
 end
 
+private def new_program
+  program = Program.new
+  program.color = false
+  program
+end
+
 class Crystal::SpecRunOutput
   @output : String
 
@@ -183,7 +189,7 @@ def run(code, filename = nil, inject_primitives = true, debug = Crystal::Debug::
 
     SpecRunOutput.new(output)
   else
-    Program.new.run(code, filename: filename, debug: debug)
+    new_program.run(code, filename: filename, debug: debug)
   end
 end
 

src/compiler/crystal/semantic/call_error.cr

@@ -92,23 +92,38 @@ class Crystal::Call
       similar_name = owner.lookup_similar_def_name(def_name, self.args.size, block)
 
       error_msg = String.build do |msg|
-        if obj && owner != program
-          msg << "undefined method '#{def_name}' for #{owner}"
-        elsif convert_to_logical_operator(def_name)
-          msg << "undefined method '#{def_name}'"
-          similar_name = convert_to_logical_operator(def_name)
+        if obj
+          could_be_local_variable = false
+        elsif logical_op = convert_to_logical_operator(def_name)
+          similar_name = logical_op
+          could_be_local_variable = false
         elsif args.size > 0 || has_parentheses?
-          msg << "undefined method '#{def_name}'"
+          could_be_local_variable = false
         else
+          # This check is for the case `a if a = 1`
           similar_name = parent_visitor.lookup_similar_var_name(def_name) unless similar_name
           if similar_name == def_name
-            # This check is for the case `a if a = 1`
-            msg << "undefined method '#{def_name}'"
+            could_be_local_variable = false
           else
-            msg << "undefined local variable or method '#{def_name}'"
+            could_be_local_variable = true
           end
         end
 
+        if could_be_local_variable
+          msg << "undefined local variable or method '#{def_name}'"
+        else
+          msg << "undefined method '#{def_name}'"
+        end
+
+        owner_name = owner.is_a?(Program) ? "top-level" : owner.to_s
+        with_scope = @with_scope
+
+        if with_scope && !obj && with_scope != owner
+          msg << " for #{with_scope} (with ... yield) and #{owner_name} (current scope)"
+        else
+          msg << " for #{owner_name}"
+        end
+
         if def_name == "allocate" && owner.is_a?(MetaclassType) && owner.instance_type.module?
           msg << colorize(" (modules cannot be instantiated)").yellow.bold
         end