Using std library with private enums

[konovod]

module A
  
  private enum X
    A1
    A2
    A3
  end
  
  def self.foo
    X.values.each {|z| c = z}
    return nil
  end
  
end

A.foo

https://carc.in/#/r/1ufx
Module A declares a private enum, in order to don't open it in public interface (obviously), but instead to use it in the implementation. Expected behavior - module is, well, using it. Current behaviour: things like Enum#values are a macroses, so they don't work with private enum.

Error in line 16: instantiating 'A:Module#foo()'

in line 10: instantiating 'A::X:Class#values()'

in /usr/lib/crystal/enum.cr:329: expanding macro

    {% if @type.has_attribute?("Flags") %}
    ^

in macro 'macro_100019376' /usr/lib/crystal/enum.cr:329, line 2:

   1. 
>  2.       [A::X::A1, A::X::A2, A::X::A3]
   3.     

private constant A::X::A1 referenced

Of course the public enum can be used instead, it adds only a single symbol to namespace so the issue isn't very serious, but still looks like a bug. Maybe private enums should be just forbidden if there is no easy way to fix it.

[makenowjust]

It is same problem as #3858. I suggest the solution but @asterite closed this pull request. We wish @asterite to solve it.

[konovod]

I searched issues for "enum"s, so didn't found it. So there is also #3878 where issue with dup was solved, but your solution is more general at the cost of breaking something (I don't quite understand what exactly it is breaking though - macros inside comments?).

[bcardiff]

The problem is that when macros are expanded they might reference to private types. That is, types that are only accesible to the caller. The expansion somehow lives in other context. Like if it would be an external reference to a private type, hence, it is not visible.

Basically any {{@type}} expand to a syntactic reference to the type where the visibility rules applies.

The following code also fails to compile on private enum

private enum Foo
  Bar,
  Baz,
  Qux
end

Foo.each do |e|
  pp e
end

[asterite]

One solution is to remove private types from the language. It was probably a bad idea in the first place.

[luislavena]

@asterite is because is a hard problem to solve or because is conceptually flawed?

I find useful been able to decide the surface size that my libraries expose, avoiding me to deal with future breakage and possible deprecation warnings.

Perhaps a more controlled scenario will be able to use protected in that context?

Thank you.

[refi64]

Personally, I always liked the way Python does private stuff: shove an underscore in front. If someone uses it anyway, then they know that it'll probably end up breaking at some point and it's not your fault. However, then you don't really need runtime support behind it.

[asterite]

@luislavena It's because it breaks with methods like Enum.values and such. I think it's better to have that working and simply mark private types with :nodoc:.

[bcardiff]

Private came handy to manage defs and sample types in specs mainly. Since everything share the namespace at the end of the day. I think those are the main benefits of private. In that sense, I consider acceptable if the private top level methods and types are just prefixed with something the user won't easily type, but when expanded the {{@type}} it would work.

[ysbaddaden]

Private came handy to manage defs and sample types in specs mainly

Sounds like a smell: adding features to the language to fix limits in Spec's implementation?

I would say we don't even need this for specs. For example, instead of a file-private free_tcp_port method, there could be a SocketSpec module with a free_tcp_port method, with the benefit of being usable from different spec files.

[straight-shoota]

It seems this discussion has been a bit mixed up between private types and private methods. While it may be an option to remove private visibility entirely, I'm sure we need to discuss types and methods (variables, constants) separately because they have very different effects.

We should keep this discussion focused on private types. Private types are actually causing issues like the one mentioned in the OP. They are can really be confusing. For example, different types can share the same name:

# -- private_foo.cr
private class Foo
end

def foo1
  Foo.new
end

# -- foo.cr
require "./private_foo"

private class Foo
end

foo2 = Foo.new

typeof(foo1)                 # => Foo
typeof(foo2)                 # => Foo
typeof(foo1) == typeof(foo2) # => false

The point is, types can't be completely encapsulated. They're always accessible through its instances and thus even a private type is visible and can be used outside the "private bubble". It just can't be directly addressed by name, that's it.
But that leads to issues like the one with enums. This could also be with any macro that expects types to be unique and visible.

There are also benefits to private types. But we need to differentiate between file-private and namespace-private.

# this is file-private, only visible in the file where it is defined
private class Foo
end

module Bar
  # this is namespace-private, only visible in the Bar namespace
  private class Baz
  end
end

File-private types allow you to define simple helper types and you don't need to worry if the same type might also be defined elsewhere, when you just need it inside that file. This can be handy for spec files.
I'd still consider it a rather niche feature, and probably does not have that many use cases. I don't think it would be a big loss. The established mechanism to avoid naming conflicts types is to place them in unique namespaces.
However, for this use case the idea of prefixing a private type's name could be a good alternative to the current implementation.

Private types in namespaces are primarily useful to declare a type to be internal for purposes of the namespace and shouldn't be used from outside. But of course, you still can do that. For example by reopening the namespace. You can even do alias PublicBaz = Baz to get a publicly visible name for a private type.
If the visibility restriction on private types was simply removed, nothing really would change except that issues like the one mentioned in the OP wouldn't occur. And you could accidentally access that type from other namespaces. I don't think that's a huge problem, though and could be worth it for avoiding real issues.

[konovod]

I like private types exactly for how they make code simpler - no need to prefix them or place in an unique namespace.
I agree that private types on a module level could be removed if they can't be encapsulated properly, but they are handy to have at least at file level.