Fixed #236: macro lookup conflicts with method lookup when including into toplevel

Ary Borenszweig · Ary Borenszweig · commit 6dffcff1e47a · 2017-05-20T13:13:57.000-03:00
diff --git a/spec/compiler/semantic/doc_spec.cr b/spec/compiler/semantic/doc_spec.cr
@@ -145,7 +145,7 @@ describe "Semantic: doc" do
     ), wants_doc: true
     program = result.program
     foo = program.types["Foo"]
-    bar = foo.metaclass.lookup_macros("bar").not_nil!.first
+    bar = foo.metaclass.lookup_macros("bar").as(Array(Macro)).first
     bar.doc.should eq("Hello")
   end
 
diff --git a/spec/compiler/semantic/macro_spec.cr b/spec/compiler/semantic/macro_spec.cr
@@ -1026,4 +1026,38 @@ describe "Semantic: macro" do
       res.program.types.has_key?("D").should be_false
     end
   end
+
+  it "finds method before macro (#236)" do
+    assert_type(%(
+      macro global
+        1
+      end
+
+      class Foo
+        def global
+          'a'
+        end
+
+        def bar
+          global
+        end
+      end
+
+      Foo.new.bar
+      )) { char }
+  end
+
+  it "finds macro and method at the same scope" do
+    assert_type(%(
+      macro global(x)
+        1
+      end
+
+      def global(x, y)
+        'a'
+      end
+
+      {global(1), global(1, 2)}
+      )) { tuple_of [int32, char] }
+  end
 end
diff --git a/src/compiler/crystal/semantic/call.cr b/src/compiler/crystal/semantic/call.cr
@@ -637,23 +637,29 @@ class Crystal::Call
   end
 
   def lookup_macro
-    in_macro_target &.lookup_macro(name, args, named_args)
+    in_macro_target do |target|
+      result = target.lookup_macro(name, args, named_args)
+      case result
+      when Macro
+        return result
+      when Type::DefInMacroLookup
+        return nil
+      end
+    end
   end
 
   def in_macro_target
     if with_scope = @with_scope
-      with_scope = with_scope.metaclass unless with_scope.metaclass?
       macros = yield with_scope
       return macros if macros
     end
 
     node_scope = scope
     node_scope = node_scope.base_type if node_scope.is_a?(VirtualType)
-    node_scope = node_scope.metaclass unless node_scope.metaclass?
 
     macros = yield node_scope
-    if !macros && node_scope.metaclass? && node_scope.instance_type.module?
-      macros = yield program.object.metaclass
+    if !macros && node_scope.module?
+      macros = yield program.object
     end
 
     macros ||= yield program
diff --git a/src/compiler/crystal/semantic/call_error.cr b/src/compiler/crystal/semantic/call_error.cr
@@ -484,7 +484,7 @@ class Crystal::Call
     return if obj && !obj.is_a?(Path)
 
     macros = in_macro_target &.lookup_macros(def_name)
-    return unless macros
+    return unless macros.is_a?(Array(Macro))
     macros = macros.reject &.visibility.private?
 
     if macros.size == 1
diff --git a/src/compiler/crystal/semantic/method_missing.cr b/src/compiler/crystal/semantic/method_missing.cr
@@ -23,16 +23,8 @@ module Crystal
     end
 
     def lookup_method_missing
-      # method_missing is actually stored in the metaclass
-      method_missing = metaclass.lookup_macro("method_missing", ONE_ARG, nil)
-      return method_missing if method_missing
-
-      parents.try &.each do |parent|
-        method_missing = parent.lookup_method_missing
-        return method_missing if method_missing
-      end
-
-      nil
+      a_macro = lookup_macro("method_missing", ONE_ARG, nil)
+      a_macro.is_a?(Macro) ? a_macro : nil
     end
 
     def define_method_from_method_missing(method_missing, signature, original_call)
diff --git a/src/compiler/crystal/semantic/semantic_visitor.cr b/src/compiler/crystal/semantic/semantic_visitor.cr
@@ -248,15 +248,15 @@ abstract class Crystal::SemanticVisitor < Crystal::Visitor
       macro_scope = macro_scope.remove_alias
 
       the_macro = macro_scope.metaclass.lookup_macro(node.name, node.args, node.named_args)
-      node.raise "private macro '#{node.name}' called for #{obj}" if the_macro && the_macro.visibility.private?
+      node.raise "private macro '#{node.name}' called for #{obj}" if the_macro.is_a?(Macro) && the_macro.visibility.private?
     when Nil
       return false if node.name == "super" || node.name == "previous_def"
       the_macro = node.lookup_macro
     else
       return false
     end
 
-    return false unless the_macro
+    return false unless the_macro.is_a?(Macro)
 
     # If we find a macro outside a def/block and this is not the first pass it means that the
     # macro was defined before we first found this call, so it's an error
diff --git a/src/compiler/crystal/types.cr b/src/compiler/crystal/types.cr
@@ -376,25 +376,57 @@ module Crystal
       defs.try(&.has_key?(name)) || parents.try(&.any?(&.has_def?(name)))
     end
 
+    record DefInMacroLookup
+
+    # Looks up a macro with the give name and matching the given args
+    # and named_args. Returns:
+    # - a `Macro`, if found
+    # - `nil`, if not found
+    # - `DefInMacroLookup` if not found and a Def was found instead
+    #
+    # In the case of `DefInMacroLookup`, it means that macros shouldn't
+    # be looked up in implicit enclosing scopes such as Object
+    # or the Program.
     def lookup_macro(name, args : Array, named_args)
-      if macros = self.macros.try &.[name]?
+      # Macros are always stored in a type's metaclass
+      macros_scope = self.metaclass? ? self : self.metaclass
+
+      if macros = macros_scope.macros.try &.[name]?
         match = macros.find &.matches?(args, named_args)
         return match if match
       end
 
-      instance_type.parents.try &.each do |parent|
-        parent_macro = parent.metaclass.lookup_macro(name, args, named_args)
+      # First check if there are defs at this scope with that name.
+      # If so, make that a priority in the lookup and don't consider
+      # macro matches.
+      if has_def?(name)
+        return DefInMacroLookup.new
+      end
+
+      parents.try &.each do |parent|
+        parent_macro = parent.lookup_macro(name, args, named_args)
         return parent_macro if parent_macro
       end
 
       nil
     end
 
+    # Looks up macros with the given name. Returns:
+    # - an Array of Macro if found
+    # - `nil` if not found
+    # - `DefInMacroLookup` if not found and some Defs were found instead
     def lookup_macros(name)
-      if macros = self.macros.try &.[name]?
+      # Macros are always stored in a type's metaclass
+      macros_scope = self.metaclass? ? self : self.metaclass
+
+      if macros = macros_scope.macros.try &.[name]?
         return macros
       end
 
+      if has_def?(name)
+        return DefInMacroLookup.new
+      end
+
       parents.try &.each do |parent|
         parent_macros = parent.lookup_macros(name)
         return parent_macros if parent_macros
@@ -950,6 +982,14 @@ module Crystal
     def vars?
       @vars
     end
+
+    def metaclass?
+      true
+    end
+
+    def metaclass
+      self
+    end
   end
 
   # Abstract base type for classes and structs
