[RFC] Compiler: error on method redefinition in same scope

[asterite]

I wanted to do this for a long time, and this blog post finally encouraged me to do it.

With this PR the compiler will now error with code like this:

def bar
end

def bar # Error!
end

Or code like this:

class Foo
  def bar
  end

  def qux
  end

  def bar # Error!
  end
end

The idea is to detect cases where you redefine a method very close to where it was defined. This means you redefined it without first closing the type and reopening, or going to a different file.

I believe whenever you do that it's most surely a mistake: you didn't realize you already had a method definition and now you are redefining it.

In fact, this spotted a bug in the Log library. The second method redefines the first one:

    def emit(message : String) : Entry
      emit(message, Metadata.empty)
    end

    def emit(message : String, **kwargs) : Entry
      emit(message, kwargs)
    end

The reason is that no keyword arguments is also caught by the second overload. The special rule in the language to prevent this is to put an : Object type restriction which means "at least one keyword argument must be passed":

    def emit(message : String) : Entry
      emit(message, Metadata.empty)
    end

    def emit(message : String, **kwargs : Object) : Entry
      emit(message, kwargs)
    end

Methods defined through macros, or macro hooks, don't count in this check because it's often the case that such redefinitions are valid and out of the user's control.

I didn't add tests for this but I wanted to see first if you think this is something useful before spending more time on it.

[straight-shoota]

Might be a good idea 👍

What would you do about previous_def?

def bar
end

def bar
  previous_def
end

It clearly states that it refers to a previous definition of the method. Still, it doesn't seem like there would be a reasonable use case not to combine these methods directly into one.
Unless you're doing some macro stuff, but since that's excluded...

[n-rodriguez]

I think it could be useful for newcomers who are not very familiar with the type system. This kind of mistake can happen easily.

[HertzDevil]

If anything, the fact that the two overloads of #emit are considered redefinitions of each other is the real bug, and that appears to have been fixed in 0.36.0. The overload without the double splat is more specific and comes first, even when #10231 (comment) is considered.

[asterite]

This was just an experiment, so closing.