Would like to be able to lint usages of "private-ish" APIs

[matanlurey]

This issue supersedes #29643 and would close angulardart/angular#930.

AngularDart requires all methods and fields in a class annotated with @Component() are public, so that the corresponding (generated) <file>.template.dart can access those methods and fields.

For example:

// hello.dart

@Component(
  selector: 'my-comp',
  template: 'Hello {{name}}!',
)
class MyComp {
  String name;
}

... generates, approximately the following ...

// hello.template.dart

class _ViewMyComp extends View {
  Element _rootEl;
  MyComp _context;

  @override
  void build() {
    _rootEl = new Element.tagName('my-comp');
    _rootEl.append(new Text(_context.name));
  }
}

This is the only intended use of the String name field. For example, it could be tightly bound to other state, it isn't valid to be changed outside of the class's knowledge. If we made name private (_name), it would satisfy that requirement, but then could not be used from .template.dart.

So, this should lint:

// not_hello.dart

void withHelloComponent(MyComp comp) {
  // LINT: visible_for_template
  comp.name = 'Haha!';
}

Specification

• An annotated class, field, or method may be referenced in the same Dart library.
• An annotated class, field, or method may be referenced in <file>.template.dart for <file>.dart
• Any other usage of the annotated member should lint

FAQ

Can't you just make .template.dart a part file?

Theoretically. This would take multiple man years, which is much higher than the cost of adding this lint. It's possible we might go down this route eventually, but it should be considered at this point functionally impossible.

Can't this just live in the angular_analyzer_plugin

Theoretically. We haven't found a good mechanism to run this, at build-time, in google3. Also not all users use the angular_analyzer_plugin (@MichaelRFairhurst should know precise numbers), so we'd need everyone to be able to see the impact of this (similar to Flutter's @required).

What about more "general" lints

If we ever implemented visibility keywords, or something like @internal or @visibleForCodegen, or @friend[ly], and it would be able to at least mostly match the semantics expressed above then we would use them instead, and happily delete this lint or the request for this lint.

Why is this important

For one, AngularDart currently heavily collides with "Effective Dart" (/cc @munificent). We're working on that - but one of the areas outside of our control this one. We'd much prefer we had an easily understandable standard way of saying "basically private":

@Component(
  selector: 'my-comp',
  template: 'Hello {{name}}!',
)
class MyComp {
  // Does not require explicit, boiler-plate documentation stating why this is public.
  @visibleForTemplate
  String name;
}

... the alternative, or the status quo, is this:

class MyComp {
  /// **WARNING**: Do not use outside the template language.
  String name;
}

[jmesserly]

Brainstorming a few ways to make this more broadly useful.

We could introduce @friendOnly and @Friend annotations:

@Friend("hello.template.dart");
@Component(
  selector: 'my-comp',
  template: 'Hello {{name}}!',
)
class MyComp {
  @friendOnly
  String name;
}

Another similar encoding would be @VisibleTo("<file name>"). For example:

const _template = "hello.template.dart";
@Component(
  selector: 'my-comp',
  template: 'Hello {{name}}!',
)
class MyComp {
  @VisibleTo(_template)
  String name;
}

Thoughts? Other ideas?

[matanlurey]

Definitely, that's an option. I think I sort of didn't correctly explain the semantics, though, I'd need anything annotated with @visibleForTemplate to be accessible to any .template.dart file in any library/package. So being forced to be precise like above won't work, at least for AngularDart.

(Another option is @visibleForCodegen, but I don't think it reads as well for AngularDart)

[jmesserly]

Oh gotcha! That's funny ... I almost added a third example for @VisibleTo that looks like this:

@Component(
  selector: 'my-comp',
  template: 'Hello {{name}}!',
)
class MyComp {
  @visibleToTemplate
  String name;
}

const visibleToTemplate = VisibleTo("*.template.dart");

... but I figured that might be too much visibility. Sounds like it's actually the right level of visibility for Angular.

EDIT: @Friend could also allow wildcards, e.g. @Friend("*.template.dart")

[matanlurey]

Yeah, that could work.

To be clear, I think ultimately having proper visibility modifiers (as a language feature or annotations) or better yet, better static meta-programming, is my ideal solution. Until then though, I think something like @visibleForTemplate has pretty small scope, and could be a good experiment for further work.

[jmesserly]

RE: @visibleForCodegen ... well, it's always possible to define your own alias:

// defined in Angular package somewhere
import 'package:meta/meta.dart' show visibleForCodegen;
const visibleToTemplate = visibleForCodegen;

I presume the linter has access to the resolved constant. (If it doesn't, it should.)

[matanlurey]

That would also be fine to me.

[bwilkerson]

I presume the linter has access to the resolved constant.

You're correct, it does.

[matanlurey]

To clarify to stakeholders, we added this in angular.meta:
https://github.com/dart-lang/angular/blob/master/angular/lib/src/meta.dart#L56

We would now need support for the annotation added into the analyzer (or linter).

[srawlins]

I'll comment real quick on how this will interact with @protected and @visibleForTesting:

All three of these annotations read as "I wanted to make this member private, but it also should be accessible by X." So combining them just unions the added visibility over what private would have given:

• @protected @visibleForTesting members can only be accessed from within their library (private access), plus from subclasses, plus from within tests (I've seen these in the wild).
• @visibleForTemplate @visibleForTesting members can only be accessed from within their library (private access), plus from within any template file, plus from within tests (I imagine there are plausible use cases?).
• I doubt there is a use case for @protected @visibleForTemplate, as components are rarely subclassed. Either way, the union logic is the same.

[matanlurey]

Closed via @srawlins' 7818db2.