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

Missing documentation for methods converting between enum and number types #15275

Open
straight-shoota opened this issue Dec 13, 2024 · 2 comments

Comments

@straight-shoota
Copy link
Member

straight-shoota commented Dec 13, 2024

All enum types have a public .new method that converts a numerical value to an enum value. This method is a primitive. The def is added for every EnumType by the compiler. It uses the specific base type as type restriction, so it cannot be the same for all enum types.

metaclass.as(ModuleType).add_def Def.new("new", [Arg.new("value", restriction: Path.global(@base_type.to_s))], Primitive.new("enum_new", self))

These methods are missing in the API docs (technically because the doc generator skips methods without location).
I'm pretty sure they generally should be considered part of the public API of enum types.

Similarly, the #value method does the inverse, turning an enum value into a number
This method is actually somewhat documented as Enum#value. This was introduced in #11947 with some previous discussion on this.

crystal/src/enum.cr

Lines 111 to 126 in 5915e3b

# Returns the underlying value held by the enum instance.
#
# ```
# enum Color
# Red
# Green
# Blue
# end
#
# Color::Red.value # => 0
# Color::Green.value # => 1
# Color::Blue.value # => 2
# ```
def value : Int
previous_def
end

This is not entirely correct though:

  • The method is actually defined on each individual enum type, not on Enum. Enum#value doesn't exist. If it was callable on Enum directly, this method definition would be invalid because there's no previous_def (I don't think it's possible to get an instance of Enum and call that method directly, so this is only theoretical).
  • The return type is actually the individual enum type's base type, not Int. This is just a generalization because the documentation is generic for Enum#value and shared between all enum types.

We should document the actual method defs on the individual enum types, not a fake Enum#value method.

The most trivial first step is to add locations to the autogenerated methods. That should make them at least show up in the API docs.

However that still doesn't produce a documentation comment, unless we hard code it into the compiler. We usually write API documentation exclusively in the standard library, even for compiler features (ref #8327).

It should be possible to solve this by explicitly adding defs for each enum type with some macro hooks. For example, I could imagine the following code doing that (note: it does not work currently)

struct Enum
  macro inherited
    # Returns the enum member represented by the given numerical value.
    @[Primitive(:enum_new)]
    def self.new(value : {{ @type.base_type }})
    end

    # Returns the numerical value of this enum member.
    @[Primitive(:enum_value)]
    def value : {{ @type.base_type }}
    end
  end
end

These are the issues:

  • Enum types do not trigger the inherited macro.
  • Enum types don't show up in Enum.subclasses either, which would allow using a global trigger (interestingly, Enum is part of all enum type's ancestors though: {{ Signal.ancestors }} # => [Enum, Comparable(Enum), Value, Object]).
  • There seems to be no way to retrieve an enum type's base type from a TypeNode.

It should be possible to change the compiler and make this work. The issues appear to be useful to fix in their own right even without this specific use case.

That would allow documenting these primitive methods explicitly. And I suppose with this we could even remove the def generation from the compiler.

In case this doesn't work out, we can still hard code some documentation into the compiler.

@HertzDevil
Copy link
Contributor

For that matter, we define the docs for the Crystal::* constants internally

@straight-shoota
Copy link
Member Author

Ah yeah that's a an example of the opposite case.
But I think it would still be better if we can get the documentation in Stalin. And I believe that this should work like in the example code I provided (or in a similar way), and we're just missing compiler support for a few components.

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

2 participants