-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Instance methods should not be shadowed by macros on the same scope #4724
Comments
Well, Same for calling a macro inside a method. It's expected for the macro to be expanded at compile time, not delayed to be a method call. Having to specify the type for all macro calls, for the unlikely case of having conflicts in macro and method names (which is a bad design) would be a huge drawback. Shall we have to call |
Well, the macro An example from my current rework of Kemal to use encapsulation instead of global scope: class MyApp < Kemal::Application
get "/" do |env|
"hello"
end
def initialize_instance_routes
get "/foo" do |env|
"bar"
end
def
end
app = MyApp.new
app.initialize_instance_routes
app.run Line 2 should call the maro, line 6 the instance method. It is never intended to call the macro from instance scope. I guess it would also be a possibility to give the macro a modifier that it should only be visible in class scope. |
Why don't we just disallow macros and methods of the same name in the same class? We've established that it's bad design and confusing, why not disallow it? |
Where have we established that it's bad design? ;) I support the idea to disallow macros and methods of the same name, if there was a way to designate a macro as being visible only from class scope, where there would be no collision with an instance method. That would actually be according to suggestion b). What about scoping a macro with class Foo
macro self.foo
"macro"
end
def foo
end
end |
It's definitely a bad design, and we should disallow it. I absolutely agree with @RX14 . It's super confusing. |
What do you think about the idea of (optionally) limiting macros to class scope just like class methods? It's clearly separated scope (class vs. instance) so no confusion. It's quite common to have class and instance methods of the same name, so why shouldn't it be possible to have a class macro instead of a class method? |
@straight-shoota you mean have macros act exactly the same as methods regarding |
Yes, exactly . Though the current behaviour makes |
@straight-shoota |
This is a continuation of #236 which gave methods in the same scope as the caller precedence over macros higher up in the scope hierarchy. But it is still inconvenient that macros shadow instance methods on the same scope:
https://carc.in/#/r/2dsx
Ideally, only the first and last call should invoke the macro.
As I have expressed in #236 (comment):
@bcardiff in #236 (comment):
I would suggest to either
a) remove macro lookup from the implicit scope entirely which would make macro calls from within an instance method the same as class methods.
b) give instance methods of the same name precedence over the macro.
In both cases it would be necessary to be able to call macros explicitly from instance scope as
Foo.my_macro
/self.class.my_macro
. The latter is actually not working, but it would be nice to have a way to explicitly call a macro on the current scope without naming the scope.The text was updated successfully, but these errors were encountered: