gleam icon indicating copy to clipboard operation
gleam copied to clipboard

✨ Introduce an `@internal` attribute for individual declarations.

Open hayleigh-dot-dev opened this issue 6 months ago • 21 comments

Background

Currently we have the internal_modules entry in our gleam.toml as a way of excluding certain modules from the generated documentation. Any pub code is always importable, but by excluding it from the docs it is de facto internal and not part of the public API.

This solution works well when you have large chunks of internal functionality, but quickly gets clunky when you have small bits of internal functionality next to public code.

It is good practice to define custom types as pub opaque unless there is good reason to do otherwise: this means consumers can't pattern match on your custom types and the implementation details remain hidden. Because of this though, it is not possible to have a pub opaque type part of the public API and a separate internal module that can pattern match on that type.

Propsal

I'm proposing an @internal attribute that can be attached to declarations that hides that declaration from the generated documentation in an otherwise-public module:

pub opaque type Wibble {
  Wibble
  Wobble
}

@internal 
pub fn encode(wibbble: Wibble) -> Json {
  case wibble {
    Wibble -> ...
    Wobble -> ...
  }
} 

Here this encode function needs access to the structure of the Wibble type in order to encode it to JSON properly, but the function itself is not part of the public API. This is handy if we have other parts of a package that might need to know how to encode a Wibble.


Prior art

Elixir has this functionality by setting @doc false, see here

hayleigh-dot-dev avatar Dec 23 '23 01:12 hayleigh-dot-dev

Seems reasonable to me. We will see if anyone has any thoughts or adjustments or objections.

It should also be excluded from autocompletions from other packages.

lpil avatar Dec 23 '23 09:12 lpil

Makes sense to me. Gonna keep my eye on this issue and start working on it when it's final.

massivefermion avatar Jan 08 '24 17:01 massivefermion

I could use this too, I like the current @internal proposal.

erikareads avatar Jan 09 '24 02:01 erikareads

Won't this be a little bit confusing? First, because it seems somewhat symmetric to @external while having nothing to do with it. Second, because then you can have something marked as pub that isn't exactly public? Users would then be able to use items that are not documented. Are breaking changes to these items allowed without affecting semver?

Looking at Rust for alternative ideas, there are two things that I understand would satisfy the current proposal's goals: there's the #[doc(hidden)] attribute, and there's the pub(crate) visibility modifier, both reasonably self-explanatory.

Couldn't we go one of these ways?

cauebs avatar Jan 09 '24 15:01 cauebs

@cauebs brings up a very good point. It can't be @internal. Maybe @hidden? or @no_doc? something like that!

massivefermion avatar Jan 09 '24 15:01 massivefermion

We already have a notion of internal that works this way:

# Modules that should be considered "internal" and will not be included in
# generated documentation. Note this currently only affects documentation;
# public types and functions defined in these modules are still public.
#
# Items in this list are "globs" that are matched against module names. See:
# https://docs.rs/glob/latest/glob/struct.Pattern.html
#
# The default value is as below, with the `name` of your project substituted in
# place of "my_app".
internal_modules = [
  "my_app/internal",
  "my_app/internal/*",
]

Having public functions that aren't visible is useful for FFI and Testing.

Having an @library or something that matches pub(crate) would be a stronger guarantee for what I need, but that will likely require more significant work.

erikareads avatar Jan 09 '24 15:01 erikareads

I know, but when it's a config in gleam.toml, there is no implicit association with @external but having both @external and @internal when they have nothing to do with each other is confusing!

massivefermion avatar Jan 09 '24 15:01 massivefermion

Won't this be a little bit confusing? First, because it seems somewhat symmetric to @external while having nothing to do with it.

It hasn't come up yet with internal modules, but I can see what you mean. We could rename them both to something else if we have a better name.

In OOP this is sometimes called protected, but I'm not a fan of that name.

Second, because then you can have something marked as pub that isn't exactly public? Users would then be able to use items that are not documented.

Aye, that's the goal of the feature. It's a statement about the contract between the user and this program.

Are breaking changes to these items allowed without affecting semver?

They would be, aye.

lpil avatar Jan 09 '24 16:01 lpil

hidden_modules = [...]
@hidden
pub type HiddenThing

erikareads avatar Jan 10 '24 08:01 erikareads

I'm not a huge fan of hidden as I think that name describes a consequence of what they are, but not what they are. The contract of them being externally private is far more important than them being hidden from documentation.

lpil avatar Jan 10 '24 09:01 lpil

@restricted, @reserved, @exclusive?

massivefermion avatar Jan 10 '24 11:01 massivefermion

Perhaps @unsupported ?

lpil avatar Jan 10 '24 11:01 lpil

Perhaps @unsupported ?

I don't think it accurately describes what it does. The advantage with my three suggestions is that anyone seeing them for the first time, without any explanation, could arrive at some vague conclusion of what they do!

massivefermion avatar Jan 10 '24 11:01 massivefermion

You say that, but I don't know what any of those three words mean in this context. They're not restricted or reserved in any way, and I'm not sure what they're exclusive of.

I find @unsupported and @internal more clear as they're functions for which support is not provided if you use them due to their being internal to the package itself.

lpil avatar Jan 10 '24 15:01 lpil

I don't agree that @internal has some confusing conflation with @external. If you take more than a second to consider it, an internal dual to @external makes no logical sense: all gleam things are internal except for externals, you'd immediately realise that such an annotation can't possibly mean anything to do with @external.

@internal describes, precisely, what the annotation does. It marks a declaration as intended for internal use. It is still public and consumable by user code, by design, so any word that implies it is somehow hidden from those people doesn't make sense.

  • @restricted implies it cannot be used.
  • @reserved implies the name is unique.
  • @exclusive implies it cannot be used.

hayleigh-dot-dev avatar Jan 10 '24 15:01 hayleigh-dot-dev

my bad! Although I still don't see @unsupported implying what we mean here. Just to make myself clear, from the perspective of the user of the package, using the definition from that package is unsupported, but from the perspective of the author, it doesn't make sense, what is it that is not supported?! I just don't see it. Sorry, I think I'm starting to ramble again, last words, I'm now more on the side of hayleigh's @internal.

massivefermion avatar Jan 10 '24 16:01 massivefermion

Remember that the purpose of this annotation is to hide a declaration from the generated docs. That means anyone who sees this annotation is necessarily looking at the source code! That means they are beyond the realms of a normal package consumer, and are now someone,, looking at the internals!

From that perspective, it becomes immediately valuable to see a pub declaration with an @internal attribute because its telling you - non-author snooping at the source - that this particular function is for internal use and you should use at your own risk.

I think I've convinced myself further that @internal is the right word.


Unrelatedly, to add more support to this issue in general. It would be nice to be able to mark the main function of a module as internal too. I always find it very odd when I'm looking at package docs with a main just hanging out.

hayleigh-dot-dev avatar Jan 10 '24 20:01 hayleigh-dot-dev

from the perspective of the user of the package, using the definition from that package is unsupported, but from the perspective of the author, it doesn't make sense, what is it that is not supported?! I just don't see it.

This is the same for pub. Those functions are public to the module author, but not that programmers' users.

The programmer wrote this code, they are making statements about it to other people rather than themselves.

lpil avatar Jan 11 '24 11:01 lpil

we have pub type Foo, but we could also entertain rust type pub (module) type Foo for scoped public-ness.

in this respect, i could put my pub (module) opaque type in internal/*, and because it's a pub (module) ..., it's permitted to be consumed in my module.

i may not be grokking the proposal perfectly, but I prefer something like the rust strategy over @internal. in the example above, encode was decorated with @internal to work around an issue with Wibble. my intuition suggests that fixing the scoping with Wibble can and should be handled at Wibble, rather than by consumers of Wibble.

🤔

cdaringe avatar Jan 18 '24 02:01 cdaringe

I agree with Hayleigh and @internal also feels very natural once someone is also exposed to the idea of internal modules: the annotation basically does the same thing but on a per-definition basis instead of being applied to a whole module. That's something I felt I could use a couple of times: "it would be really nice to hide this function from the public API but I don't want (or I can't) move this to an internal module", to which a natural answer is "make it an internal definition!"

Also to add to the issue, we would need to warn if any type marked as @internal is inadvertently exposed in the public api (just like suggested here #2234)

giacomocavalieri avatar Jan 18 '24 07:01 giacomocavalieri

@cdaringe This is similar but not the same as making an item public or private for a package or set of modules.

I would also be interested in a system for drawing lines within a package to enforce certain relationships between subsystems, but we don't have a good design yet. Rust's system is capable but I've never seen anyone do more than pub(crate), which makes me think it's not a good design- it doesn't matter if it's capable of doing something if no one wants to use it.

If we determine a good design for this it can be added in future. Both this internal system and a more powerful public/private system have their own uses.

lpil avatar Jan 18 '24 12:01 lpil

I think this can be closed now 🥳

giacomocavalieri avatar Mar 25 '24 22:03 giacomocavalieri

Thank you!!

lpil avatar Mar 25 '24 22:03 lpil