Don't document elements with @internal

[sigurdm]

Makes members with the @internal or @visibleForTesting documents not part of the public interface.

#2418

[rrousselGit]

Honestly I'm a bit skeptical with hiding visibleForTesting. Those are visible, just under some conditions

visibleForTesting allows any test suite to access those utils, not just the test folder of the defining package.
A package can define a visibleForTesting element. And apps can use it in their tests.

So it is public API.

[sigurdm]

And apps can use it in their tests.

According to the documentation it only allows test suites from the same package to access it:

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

Not sure if that is how the warnings actually work

[sigurdm]

I have tried out the warnings. And indeed it seems you can import a @visibleForTesting from anywhere without getting a warning. So either we need to fix the docs or the implementation. (Or maybe add a separate annotation)

[srawlins]

which is in the test folder of the defining package.

Oops, yeah. That's now how the analyzer runs things. But also, I think I prefer the analyzer's interpretation, over the documented behavior ("of the defining package"). We could change the docs.

[rrousselGit]

I like the current behavior of visibleForTesting.
If it only worked in the defining package, it'd pretty much be a duplicate of @internal

I use VisibleForTesting in ways that allow usage outside the package.
For instance, Riverpod has a Notifier class defined as:

abstract class Notifier<T> {
  @protected
  @visibleForTesting
  T state;
}

So within production code, "state" is protected, to promote good separation of concern.
But tests have direct access to "state" so that folks can more easily initialize their notifiers.

---

We could also imagine that a package defining Foo would also ship an isFoo Matcher for testing sake. And mark that matcher as visibleForTesting

This avoids having to make two packages to separate dev dependencies from non-dev ones, without compromising tree shaking.

[sigurdm]

Not sure where to go from here.

It seems that both of these annotations have a vague relation re. what it means for the annotatee to the exported.

Sounds like we could use 4 annotations:

• @visibleForOwnTests
• @visibleForOthersTests
• @warnIfExportedFromPackage
• @warnIfImportedInPackage

All of these seem to have at least some use cases. And the documentation around @internal and @visibleForTesting is vague.

Filed dart-lang/sdk#60731
and dart-lang/sdk#60732

[rrousselGit]

We're not limited to applying a single annotation.

So given:

• @visibleForOwnTests
• @visibleForOthersTests

We could combine visibleForTesting+internal to achieve both:

@internal
@visibleForTesting
int a; // only usable in the tests of the defining package

@visibleForTesting
int b; // usable by any test

I'm not sure what the current behavior is, but that feels like a good solution

---

As for the other two proposals, I'm not sure how they relate to @internal

The annotation has a really good behavior. I wouldn't change it. Maybe the Doc needs a little bit of rewording, but no code change.

[srawlins]

Could we make a smaller incremental change, just addressing the @internal request?