Unclear documentation of @internal

[sigurdm]

https://pub.dev/documentation/meta/latest/meta/internal-constant.html says

Tools, such as the analyzer, can provide feedback if

the declaration is declared in a package's public API, or is exposed from a package's public API, or
the declaration is private, an unnamed extension, a static member of a private class, mixin, or extension, a value of a private enum, or a constructor of a private class, or
the declaration is referenced outside the package in which it is declared.

It seems the last line kind of contradicts the two others. That case should only happen if the other ones are not upheld.

Also it seems the analyzer only reports about top-level @internal elements being exported.

// lib/src/a.dart
import 'package:meta/meta.dart';

@internal // This causes warning
class Exported {
  @internal // this does not
  void shouldNotBeExported() {}
}

// lib/main.dart
export 'src/a.dart'; // Warning is issued here

[rrousselGit]

I don't think the example given about "top level" is correct

Afaik the lint only applies to public libraries.

Given:

@internal
class A {}

Then if the source is located in lib/a.dart, we get a warning. But if it's in lib/src/a.dart, we don't.

---

Afaik the warning on top-level elements is on purpose. It's to tell packages that those should not be exported.

But we can't easily do the same thing for class members, as Dart has no mean to do something like export 'a.dart' hide MyClass.myMember;

So a warning in those cases would make @internal unusable

[rrousselGit]

In short we have 3 things at play:

1. Warnings when a third-party package uses "internal" code:

// my_package/lib/a.dart
class A {
 @internal
  int value
}

// another_package/lib/main.dart

print(a.value); // warn

2. Warnings when a package incorrectly uses @internal

Such as:

@internal // useless since private
class _A {}

3. Warnings when top-level elements are exported. Cf the previous comment

IMO the docs could be improved by splitting the description in 3 parts ; one for each cause.

[sigurdm]

Then if the source is located in lib/a.dart, we get a warning. But if it's in lib/src/a.dart, we don't.

But if, as in my example above, the definition is in lib/src/a.dart but it is re-exported from a top-level you get the warning. I guess that is the usefulness (to warn someone that they should not re-export a whole library as it contains internal parts).

But we can't easily do the same thing for class members, as Dart has no mean to do something like export 'a.dart' hide MyClass.myMember;

Could it not warn if the surrounding class was exported (as that implies the member is also exported).

To me it is really confusing that the same annotation has two related but actually quite different meanings:

• Help me not export this by mistake.
• Help consumers of my package not importing this by mistake.

I can kind of see the use of both of these, but it is not clear why the same annotation has both of these meanings.

Really I'd like a mode where you explicitly mark the elements you want to be public.... but that is another topic ...

[rrousselGit]

To me it is really confusing that the same annotation has two related but actually quite different meanings:

• Help me not export this by mistake.
• Help consumers of my package not importing this by mistake.

I'm an avid user of @internal and I've never used it for either of those reasons.
I use it to mark various elements as private, without prefixing them with _ due to it being inconvenient.

For example, I often use it inside code-generators.
It's quite common that the "annotation" part of code-generation also includes classes/functions that are meant to be used by the generated code ; but aren't meant to be used directly by users of the package.

For that, I tend to do:

// An annotation for using the code-generator
const myAnnotation = ...;

// Some utils that the generated code will rely on
@internal
class $Util {}

Then the generated code will do:

// ignore_for_file: internal_usage (or whatever the error code is)

part of 'file.dart';

// TODO: Use $Util somehow

Really I'd like a mode where you explicitly mark the elements you want to be public.... but that is another topic ...

Off topic, but I implemented such a lint for my own personal usage 3 weeks ago.
Source: https://github.com/rrousselGit/riverpod/blob/master/packages/internal_lint/lib/src/lints/show_all.dart

It's a lint that's the reverse of @internal: @Public.inLibrary('libraryName').

TL;DR:

// src/source.dart
@Public.inLibrary('misc.dart')
class A {}

class B {}

@internal
class C {}

// my_package/lib/misc.dart
export 'src/source.dart' show A;

// my_package/lib/my_package.dart
export 'src/source.dart' show B;

And this warns if:

• a library is exporting elements that it shouldn't
  (For ex: my_package/my_package.dart shouldn't export A)
• aa library forgot to export elements that it should've
  (For ex: if B was not exported in my_package/my_package.dart)

One of the reasons I've implemented this is: It empowers export ... show.
I can now use export ... show and be confident that when adding a new public API, I didn't forget to export it.

[sigurdm]

Right - I think it would be useful with a third annotation for that (though I think it falls somewhat under the second use case I mentioned: "Help consumers of my package not importing this by mistake")

[rrousselGit]

I'm not sure I agree. I think it's just the annotation's name that is a bit unfortunate. I'd name it @private

[sigurdm]

If you read the original design discussions of @internal it is not clear at all to me that people agreed on what it would be used for: #28066.

[bwilkerson]

Could it not warn if the surrounding class was exported (as that implies the member is also exported).

I suppose that it could, but it would be an interesting, and possibly confusing, new way to use annotations. We have annotations (such as doNotStore) that can be applied to containers (libraries, classes, etc.) to apply to all of the members in that container, but we haven't yet defined any annotations that can be applied to a member that would apply to all of the containers of that member (which is the semantics that I think you're asking for).

Really I'd like a mode where you explicitly mark the elements you want to be public.... but that is another topic ...

Off topic, but I implemented such a lint for my own personal usage 3 weeks ago.

@stereotype441

Interesting. We recently added a similar sort of lint for the analyzer package. Maybe it would be worth unifying them and making it more widely available.

[sigurdm]

but we haven't yet defined any annotations that can be applied to a member that would apply to all of the containers of that member (which is the semantics that I think you're asking for)

I'm not sure that's how I look at it. It "technically" still applies only to the member - just there is no way to export the container without also exporting the member, so it becomes a bit moot...

[bwilkerson]

Could it not warn if the surrounding class was exported (as that implies the member is also exported).

If marking a member as being @internal means that we warn when the container is exported, then it's equivalent to putting @internal on the container. In my mind, that's equivalent to saying that the annotation is implicitly on the container. We have annotations that are imply the same property holds for the members, but not the other way around.

I would argue that it's better to just put the annotation on the container in that case because it seems to me that putting it on the container will make it less likely for someone to miss the fact that that class can't be exported than it would be if the annotation were on some member somewhere inside the container.

And if we start allowing it on members then we have to deal with the question of overridden members. Would that mean that any subclass of the class with an @internal member that overrides the @internal member is also implicitly not to be exported? I think allowing it on members makes it too complex.