Skip to content

Add a hint: "@internal" annotation #28066

Closed
@matanlurey

Description

@matanlurey

Proposed specification:

Used to annotate compilation unit `c`.

Indicates that the unit should not be used outside of this package, and that any breaking changes that occur to the unit will not be considered a breaking change for the package.

Tools such as the analyzer, can provide feedback if:
- the compilation unit is made visible in a dart file residing in a "public" file (i.e. not in lib/src)

I'm finding this increasingly necessary when working on large packages.

For example, assume I have the following in lib/src/internal.dart:

import 'package:meta/meta.dart';

@internal
class KeyValuePair<K, V> {
  final K key;
  final V value;
  const KeyValuePair(this.key, this.value);
}

And later in my application I write the following in lib/my_package.dart:

export 'package:my_package/src/internal.dart`
// ^ WARNING: Exporting symbol `KeyValuePair`, which is marked `@internal`.

Activity

bwilkerson

bwilkerson commented on Dec 10, 2016

@bwilkerson
Member

According to https://www.dartlang.org/tools/pub/package-layout#implementation-files, everything inside the 'src' directory is already suppose to be treated as internal implementation detail. Would it be sufficient to have a lint rule that flagged uses of code inside 'src' from anywhere outside the defining package?

matanlurey

matanlurey commented on Dec 10, 2016

@matanlurey
ContributorAuthor

Not really, because you actually would want to export public classes. For example:

// Assume this is lib/my_package.dart

// OK!
export 'src/internal.dart' show PublicInterfaceIWantToExpose;

// Oops!
export 'src/internal.dart' show ScaryThingIMeantToKeepPrivate;

Lots of the SDK/core packages work this way

bwilkerson

bwilkerson commented on Dec 10, 2016

@bwilkerson
Member

The lint rule I was proposing would only create a lint when a library in another package's 'src' directory is being imported. It wouldn't check for uses of re-exported classes.

matanlurey

matanlurey commented on Dec 10, 2016

@matanlurey
ContributorAuthor

The @internal is more for the package author themselves to keep their public API under check.

added
P2A bug or feature request we're likely to work on
type-enhancementA request for a change that isn't a bug
and removed
devexp-linterIssues with the analyzer's support for the linter package
on Jan 2, 2017
srawlins

srawlins commented on Jan 5, 2018

@srawlins
Member

I like this annotation. In addition to the original end goal:

the compilation unit is made visible in a dart file residing in a "public" file (i.e. not in lib/src)

the analyzer should warn if an @internal identifier is being referenced outside a package. The problem now is: what is a package? As you note at #31775, it's a pub concept. I don't think analyzer has any code that identifies (A) what "package" a library comes from, or (B) whether a library is in the "public" part of a package. Womp womp. Only the linter cares if you import from src/, implementation_imports.

srawlins

srawlins commented on Jan 5, 2018

@srawlins
Member

As far as your original goal though, you don't need any concept of whether two libraries are part of the same package (A), you just need the public API concept (B). Maybe this can be simply defined:

File paths that match ^.*/lib/[^/]+.dart$ comprise the public API. Files that do not match do not.

  • /Users/me/code/lib/a.dart public
  • /Users/me/code/lib/src/fakeout/lib/b.dart public
  • /Users/me/code/lib/src/a.dart not public
  • /Users/me/code/lib/sublib/public-api/a.dart not public
  • /Users/me/code/test/a.dart not public

I think this is better than looking at how libraries were imported. I'm pretty sure relative import paths could skirt any enforcement at that level. Import tracking would also require following imports and exports all the way up to an element's definition.

lrhn

lrhn commented on Jan 8, 2018

@lrhn
Member

How do you handle a library in lib exporting a library in lib/src?

I'd rather go for defining libraries being in "the same package" as having URIs starting with package:pkgname/ with the same pkgname.

This is not a language semantics issue, just an optional analyzer hint, so there is no harm in understanding pub packages - it's tooling, just like pub itself.

srawlins

srawlins commented on Jan 8, 2018

@srawlins
Member

But in what sense does a library "have a URI"? For example, if the analyzer is examining /Users/me/code/test/foo_test.dart (with a package root of /Users/me/code), what URI does it "have"? If that file has some imports:

import 'test_helper.dart';
import '../lib/src/foo.dart';

What URIs do those "have"?

lrhn

lrhn commented on Jan 11, 2018

@lrhn
Member

Inside Dart, all libraries have one exact URL, all distinct (because if they are the same, it's the same library). This is well defined, and hasn't changed in a long time. You can import the same file twice, using different URLs, and it will be considered two different libraries (introducing different classes and having separate global states), which is why you should avoid doing that.

Imports with relative paths are resolved against the library URL.
In this case, most likely the URL of the foo_test.dart file is file:///Users/me/code/test/foo_test.dart, and the URL of the second import is therefore file:///Users/me/code/lib/src/foo.dart. That's different from package:code/src/foo.dart, so you import both, you will get two different libraries .

So, import the foo.dart file as package:code/src/foo.dart. In fact, all files outside of the lib dir should use the package URI to reference files inside the lib dir, so that the actual package libraries are always referenced in the same way.

46 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @nex3@kevmoo@srawlins@matanlurey@zoechi

      Issue actions

        Add a hint: "@internal" annotation · Issue #28066 · dart-lang/sdk