Skip to content
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

Inference of type from namespace #8685

Open
straight-shoota opened this issue Jan 13, 2020 · 8 comments
Open

Inference of type from namespace #8685

straight-shoota opened this issue Jan 13, 2020 · 8 comments

Comments

@straight-shoota
Copy link
Member

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
Copy link
Contributor

j8r commented Jan 13, 2020

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

@kimburgess
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member Author

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 #8685 (comment) 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
Copy link
Member

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
Copy link
Member Author

straight-shoota commented Feb 21, 2020

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
Copy link
Member Author

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
Copy link
Member Author

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 #8685 (comment) 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants