crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.48k stars 1.62k forks source link

Inference of type from namespace #8685

Open straight-shoota opened 4 years ago

straight-shoota commented 4 years ago

When you declare a type with a namespaced name and the namespace has not been defined yet, the compiler automatically assumes it's a module.

module Foo::Bar
end

class Foo # Error: Foo is not a class, it's a module
end

The resulting error is inconvenient, because it's not really correct. Foo is technically supposed to be a class and it's only a module because the compiler decides so based on the order things are seen. It's relatively easy to work around this, you just need to make sure the definitions are in the expected order. This however often leads to require sections at the bottom of a file which kind of hides them away or adding more files to get more granular control.

I was thinking if we could improve this situation. Basically, when the compiler sees a reference to Foo::Bar and doesn't already know Foo yet, the only explicit statement is that Foo is a namespace and contains Bar. We could add some kind of flag to signal that Foo's exact type is not known and in case we see an explicit definition like module Foo or struct Foo, it can be transformed to the respective type. Defaulting to module in case there is no such clarification is fine, obviously.

j8r commented 4 years ago

It can easily happen when using require "somedir/**", when subdirectories are required before the *.cr subfiles.

kimburgess commented 4 years ago

Also frequently occurs when you follow the recommended file / directory structure and have nested classes, structs, enums etc in their own files.

$ tree .
.
├── foo
│   ├── bar.cr
│   ├── baz.cr
│   └── qux.cr
└── foo.cr

foo.cr:

require "./foo/*
class Foo
  # ...
end

./foo/bar.cr:

class Foo::Bar
  # ...
end
asterite commented 4 years ago

Another alternative, which is much simpler to implement and probably won't have unexpected consequences (for example: what happens if you access that type using macros before it's finished) is to remove that "feature". If you do class Foo::Bar and Foo doesn't exist, you get an error. Exactly like in Ruby.

I added that feature because I thought it would be convenient for always using Foo as a module namespace (mainly for shards authors), but it seems people are using it for other types too.

So another alternative is to document this better, saying that this is meant to be used for modules, and just modules, mainly for shards namespaces.

straight-shoota commented 4 years ago

I very much like the simplicity.

But regarding usability it would really be nice to have an easy solution to use cases as described in https://github.com/crystal-lang/crystal/issues/8685#issuecomment-583821411 Currently, you only have the option to move require "./foo/bar" at the end of the file (or at least after the first definition of Foo) or specify class Foo; class Bar explicitly in the required file. Both are not very elegant and I hope there can be a way to make this easier.

Blacksmoke16 commented 4 years ago

Probably related, https://play.crystal-lang.org/#/r/8lt0

private abstract struct Athena::Routing::Param; end

private record Athena::Routing::Parameter(T) < Athena::Routing::Param, value : T
Showing last frame. Use --error-trace for full trace.

There was a problem expanding macro 'record'

Called macro defined in /usr/lib/crystal/macros.cr:60:1

 60 | macro record(name, *properties)

Which expanded to:

 > 1 | struct Athena::Routing::Parameter(T) < Athena::Routing::Param
                                              ^---------------------
Error: undefined constant Athena::Routing::Param

Defining the structs within a module, or removing private fixes the issue.

straight-shoota commented 4 years ago

That's an unrelated visibility issue. Private constants are only visible inside the parent namespace, but your code references private, namespaced constant Athena::Routing::Param from the top-level namespace. It's just not visible there. This is just the same as:

private class Foo::Bar
end

Foo::Bar # Error: undefined constant Foo::Bar

In case you want to discuss changing this, please open a new issue (or check whether this has already been discussed before).

straight-shoota commented 4 years ago

I guess the error message is wrong, though. Or at least, it should be more specific and notify that a private constant was referenced, see #8831.

straight-shoota commented 3 years ago

I encountered this error under very weird circumstances. It appeared without any code change, and reproduces only on GitHub actions (dunno why, maybe filesystem entry order?). The worst part is that the error doesn't say where the first definition as ˋmoduleˋ was. https://github.com/straight-shoota/crinja/runs/3007606661

It seems the suggestion in https://github.com/crystal-lang/crystal/issues/8685#issuecomment-584147027 is probably the best solution to the problem. But it's a breaking change and we can't introduce it before 2.0. Until then, it would be helpful to enhance the error message to show the location of the first namespace defintion to help figure out where the require-order problem comes from.

crysbot commented 5 months ago

This issue has been mentioned on Crystal Forum. There might be relevant details there:

https://forum.crystal-lang.org/t/class-as-namespace/6871/5