Crystal: Remove top-level methods and macros

[asterite]

(extracted from https://github.com/crystal-lang/crystal/issues/4439#issuecomment-302931547)

Right now methods and macros defined at the top-level pollute the global namespaces. The recommendation is not to use them, but they are available. And, for example, Kemal defines a whole bunch of global methods, macros and constants: get, post, ws, error, content_for, CONTENT_FOR_BLOCKS, yield_content, render, halt, add_context_storage_type and probably more. I also saw some other libraries that just don't care about polluting the global namespace, maybe because they don't know that defining a top-level method has that consequence. I can imagine eventually they'd have to be refactored to avoid clashing problems.

To solve this, one idea that came to my mind is this:

• Methods and macros defined in a file at the top-level are now always file-private
• Methods defined in a module can be used by using include

For example:

# file: "kemal.cr"
module Kemal
  def get(path, &block)
    #...
  end
end

# file: "main.cr"
require "kemal"

include Kemal

get "/" do
  # ...
end

In this way there's absolutely no way to pollute the global namespace of methods and macros, because such namespace ceases to exist.

Using Kemal's DSL becomes just one line longer, and the get method is only available in that file, not in files required by "main.cr".

And Kemal could extend self so we could write Kemal.get "/" {} instead if we don't want to pollute that file's methods.

If we do this, we loose the ability to use puts, raise, record and others, because these were top-level methods/macros. To solve this, we can define a Kernel module, put these definitions there, and let all files automatically include this Kernel module. This seems like bringing the global namespace back again, but it's not: to pollute the global namespace one would have to define methods in Kernel, but that practice would be discouraged. Of course we can't force users not to reopen Kernel, but at least writing this simple and innocent looking code:

# file: "my_library.cr"
def use_my_library
  #
end

won't pollute the global namespace anymore, you have to be conscious about reopening Kernel and adding more methods to it.

This also brings another breakage: spec defines describe and it as global methods. We'd need to move them to the Spec module and then you'd have to do:

require "spec"

include Spec

describe "Something" do
  it "does something" do
    #...
  end
end

But in my opinion, even though it's a line longer, it's better because the definitions of describe and it won't exist in other files.

(there's still the "issue" that should and should_not are added to every type, we can think of a solution to that in another issue, or simply start using expect or assert)

We can do the same for some methods that colorize defines, and for the not very commonly used lazy, future and delay methods (we could put them in a Concurrent module).

Another nice thing that we can do with this change is to apply the same things for types: if you include a module in the top-level, you can refer to types inside that module, but those types won't be available in other files:

require "http/client"

include HTTP

# No need to write HTTP::Client multiple times, and
# Client can't be used in other files without doing include HTTP
client1 = Client.new(...)
client2 = Client.new(...)

This is specially nice to avoid having to write long type names, although a solution for that already exists, which is a bit more verbose but maybe cleaner:

require "http/client"

private alias Client = HTTP::Client

I can imagine this step as a first change towards a cleaner require system. Maybe include Kemal isn't very clear because how do we know what methods that module defines? Maybe include Kemal, :get could be better and more explicit. Same goes for types provided by a module. But that's a much bigger change.

[RX14]

@asterite If all top-level declarations were file private by default, surely alias Client = HTTP::Client at the top level would become enough to reference long types. This is a good thing and I'd definitely use that syntax.

[akzhan]

I suppose we need something more high-level than require.

Yes, it's about import, just like (pseudocode)

def import( name, symbols_to_import... )
  require name
  if symbols_to_import.empty?
    include name
    return
  end
  symbols_to_import.each do |symbol|
    if symbol.is_a? Type
       private alias symbol = name.symbol
    elsif symbol.is_a? Module 
       include symbol
    end
  end
end

[ysbaddaden]

I like the goal. Don't polute the global namespace, let me include what I want to be global (or include it in another module).

The drawback is having to include Kemal or Spec (for example) in each and every file, which may be annoying over time. Same for methods becoming file private​, which may be confusing (at first).

I think I'd like having to include Kemal or Spec once, but I guess that would somewhat defeat the purpose?

[asterite]

Yes, a more general solution can't work with require, or at least require has to be extended to define a list of names to bring into the current file (so it would probably work like the import of #4439, but allowing to expose types and methods)

[faultyserver]

At first glance, this is starting to remind of Java (everything must be defined inside a class) and Python (having to require every dependency at the top of every file), why I left them, and why I like Ruby so much.

I think it's good and idiomatic for libraries to provide both a "global-polluting", easy-to-use system, as well as a way to namespace the library if the user is worried about collisions.

Sinatra is a good example of this:

require 'sinatra'

get 'thing' do
  # This just works out of the box and is simple and intuitive
end

---

require 'sinatra/base'

# Now the only thing in the top-level is `Sinatra::Base`
class SomeApp < Sinatra::Base
  # And things like `get`, and `post` are scoped inside this class
  get 'thing' do
  end
end

With that, I don't think Crystal _needs_ anything in the language to do this, it should just be considered good, idiomatic practice.

---

Methods and macros defined in a file at the top-level are now always file-private. Methods defined in a module can be used by using include.

I'm concerned that this would be more surprising to users than it is helpful. For example, I defined a macro render2 that I use with Kemal in it's own file:

# helpers/render2.cr
macro render2(filename, path="src/templates", layout=nil)
  {% if layout %}
    render "#{{{path}}}/#{{{filename}}}.ecr", "templates/layouts/#{{{layout}}}.ecr"
  {% else %}
    render "#{{{path}}}/#{{{filename}}}.ecr"
  {% end %}
end

# controllers/thing_controller.cr
require "helpers/*"

render2(...)

If top-level macros are now file-private, how would I implement this to make it usable in other files? Would I have to create a module and include it everywhere I want to use it?

# helpers/render2.cr
module Helpers
  def render2(filename, path="src/templates", layout=nil)
    {% if layout %}
      render "#{{{path}}}/#{{{filename}}}.ecr", "templates/layouts/#{{{layout}}}.ecr"
    {% else %}
      render "#{{{path}}}/#{{{filename}}}.ecr"
    {% end %}
  end
end

# controllers/things_controller.cr
require "helpers/*"
include Helpers

render2(...)

I think the latter is good practice, I just don't think it should be strictly enforced.

[asterite]

Mmm... let's leave things are they are now and wait and see if things break :-)

[straight-shoota]

@faultyserver You wouldn't need the require "helpers/*" in every file, just the include Helpers. Alternatively you could reopen the Kemal module and insert your Helpers there. Then your custom render2 can be included together with every other Kemal method. You're effectively only polluting the Kemal namespace instead of global.

[ysbaddaden]

Maybe a "good practices" section in the crystal book would be interesting? We could list this idea of not polluting the global namespace by default, or as requested by the user (e.g. the Sinatra example above), along many other ideas :)

[bcardiff]

@asterite as for https://github.com/crystal-lang/crystal/issues/4439#issuecomment-302931547 and this issue, the cornerstone for that to work is that the include should not affect the global namespace like it now does. If the include applies only to the lexical scope (like ruby's using), or at least file scope, then it could work to avoid pollution in the global namespace.

I think changing include semantics or introducing using with the lexical scope would allow better organization of the code in this sense, with some of the required duplicates of the include/using in every file. I don't see it as a big price to pay (or maybe some construct over that could appear).

[faultyserver]

@straight-shoota you're right. The example I used has a pretty well-defined best practice as you described.

I guess I'm just not onboard with forcing things into modules in general. I like it as an idiom, but not so much as a compiler restriction.

@ysbaddaden I think a "good practices" section would be a great addition. Many languages (and companies) have style guides, but few have _official_ "convention guides".

[faultyserver]

@bcardiff I like the idea of a separate keyword (e.g., using) for this new behavior as it would be opt-in, rather than opt-out.

That way, if a user knows that they want Spec's contents in the top-level namespace they can still use include Spec (assuming it gets namespaced in the first place) and have it available in every subsequent file.

But, it the user wants more control of where Spec's contents are available, they can write using Spec instead to treat it as file-local.

[RX14]

@bcardiff How about adding a private include instead of using which works at the top level and adds members to the top level namespace in the current file only. It seems more consistent to me, if a bit longer.

[faultyserver]

@RX14 private include seems a little reminiscent of macro def to me... I like that it's consistent, but I'm not sure how it would work implementation wise.

[RX14]

@faultyserver macro def is still there, it's just inferred automatically in nearly all cases now. private include can't so I think it's fine. Implementation wise it wouldn't be harder than private def i'm sure.

[faultyserver]

Yeah, I'm not entirely sure what it is that connects the two in my mind.

I think the more pressing thing for me is that private include implies to me that include returns a value that is made private (disclaimer: I don't actually know how Crystal implements private).

Something like this makes sense to me:

private class Thing
end

In my Ruby-trained mind, that says "make a class Thing, and send that to private", similar to how you can do something like this in Ruby:

class Thing
  def foo
  end

  private :foo

  # equivalent to
  private def foo
  end
end

Interestingly, though, private constants in Ruby don't work this way:

class Person
  class Secret
  end
  private_constant :Secret

  private :Secret # does nothing
end

In any case, I'm probably just wrong for thinking about this from a Ruby perspective :)

[faultyserver]

FWIW, there's an existing, open issue (#3319) that suggests different semantics for using, so maybe this discussion would be better suited there?

[RX14]

@faultyserver Yeah, class Foo has no return value in crystal. It's simply a keyword modifier.

[paulcsmith]

FWIW I also like the idea of private include MyModule. I tried to do that and it didn't work, but it seems pretty intuitive (to me at least). I'm also ok with things as they are now though.