Skip to content

pkg/meta: @visibleForCodegen #29643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
matanlurey opened this issue May 17, 2017 · 6 comments
Closed

pkg/meta: @visibleForCodegen #29643

matanlurey opened this issue May 17, 2017 · 6 comments
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).

Comments

@matanlurey
Copy link
Contributor

Example

// foo.dart.

@visibleForCodegen
final someField = new Foo();
// foo.g.dart

import 'foo.dart';

// Refer to `someField` without a warning.

Rationale

Without using partial files (part/part of ), it's impossible to use private (_) variables, which forces developers to write pseudo-public code so that it can be accessed by code generation. However, it serves no other use, and might not even be part of a fully tested public API suite.

@visibleForCodegen would mean:

  • An annotated class, field, or method may be referenced in the same Dart library.
  • An annotated class, field, or method may be referenced in a different library if it is generated
    • This can be by convention, i.e. foo.dart and foo.g.dart or foo.template.dart
    • Or explicit, i.e. another @generated annotation on the generated file

Alternatives

Something like @friendly, which means "public to the same package". This is ~the same as @visibleForTesting, except without any requirement for it to be only used in the test/ directory.

/cc @kevmoo @alorenzen

@bwilkerson bwilkerson added the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label May 17, 2017
@bwilkerson
Copy link
Member

Language team:

I think it's rather telling that we have several requests for annotations to effectively broaden the visibility control within the language, things such as @protected, @visibleForOverriding, @visibleForTesting, and now @visibleForCodegen. Should we extend the language to cover some or all of these questions, or continue to add annotation-based checking to the analyzer?

@MichaelRFairhurst
Copy link
Contributor

The annotations story seems kinda just broken without mirrors.

When its possible to do

@MyAnnotation(_foo)

and the only non-deprecated way that that can be used in the browser is via codegen, which cannot access that value (_foo), and therefore will either produce broken code or have to (the author of each code generator must write this check) detect that '_foo' is private and raise a custom error, in either case the developer needs to make "_foo" public and add a special privacy annotation to it instead.

One possible solution is to make private disallowed inside annotations, which is a breaking change and not necessary for those using mirrors. (and has a lot of edge cases... @_foo, @x(() => _foo}), etc.)

I'd say the real solution is that annotations have been gutted too much, and either should be replaced (not seriously suggesting this) or a static, tree-shakable API for accessing annotation data and/or annotated data should be developed. Wherein you could simply say something like [email protected], which would be the constant value of _someField, no privacy rule changes required, purely static so that all unused X.@Foo data is discarded by dart2js.

Maybe both of these things should be done at once? Decide what to do about annotations long term, while this privacy change thing is introduced temporarily?

@matanlurey
Copy link
Contributor Author

@bwilkerson:

Language team:

I think it's rather telling that we have several requests for annotations to effectively broaden the visibility control within the language, things such as @protected, @visibleForOverriding, @visibleForTesting, and now @visibleForCodegen. Should we extend the language to cover some or all of these questions, or continue to add annotation-based checking to the analyzer?

Thanks for bringing this up. BTW, this is no way a "we must have this annotation", but rather just trying to used established mechanisms (analysis with a common annotation) to get around a problem folks have been reporting.

I personally think Dart should invest in actual visibility control, but beyond this thread maybe?

/cc @leafpetersen @floitschG @munificent


@MichaelRFairhurst: I think we should keep what the point of annotations are outside of this thread, I don't want to expand scope beyond this particular request and today's tooling.

@munificent
Copy link
Member

I think it's rather telling that we have several requests for annotations to effectively broaden the visibility control within the language, things such as @Protected, @visibleForOverriding, @VisibleForTesting, and now @visibleForCodegen. Should we extend the language to cover some or all of these questions

Yes, I personally think we should consider more fine-grained access control modifiers. Keeping things simple is good, but it's possible to be too simple. My five-minute sketch would be to add:

  • Protected, which treats the identifier as public (i.e. not name mangled like private identifiers), but makes it a static error to invoke the member when the enclosing class is not a subclass of where the member is declared.

    Given that privacy is library based, it might be worth considering making this more library based as well. Something like all protected identifiers in library A are visible inside the bodies of classes in library B as long as they subclass some class in library A. I think that might open up some interesting and useful patterns and give you something akin to "family protected".

    We'd have to decide how this interacts with dynamic callsites. I'm not too picky since I don't consider those very important.

  1. Friend libraries, which give other libraries access to a library's private identifiers. This would let you do white box testing by having your test library be a friend of the main library. It should also help with generated code.

    I know it's anathema to some, but I would allow a library to declare itself to be a friend of some other library. This prevents a library from encapsulating itself, but it means a test can crack open a library without the main library having to mention its test libraries. I don't look at access control as a strict security system — you can always modify the source anyway — and more of a velvet rope indication "you aren't supposed to be here".

    The semantics and syntax of this are really tricky though. You need some way of disambiguating private identifiers which may clash with the main library's.

@matanlurey
Copy link
Contributor Author

@munificent:

I think it's rather telling that we have several requests for annotations to effectively broaden the visibility control within the language, things such as @protected, @visibleForOverriding, @visibleForTesting, and now @visibleForCodegen. Should we extend the language to cover some or all of these questions

Yes, I personally think we should consider more fine-grained access control modifiers. Keeping things simple is good, but it's possible to be too simple. My five-minute sketch would be to add:

  • Protected, which treats the identifier as public (i.e. not name mangled like private identifiers), but makes it a static error to invoke the member when the enclosing class is not a subclass of where the member is declared.

    Given that privacy is library based, it might be worth considering making this more library based as well. Something like all protected identifiers in library A are visible inside the bodies of classes in library B as long as they subclass some class in library A. I think that might open up some interesting and useful patterns and give you something akin to "family protected".

    We'd have to decide how this interacts with dynamic callsites. I'm not too picky since I don't consider those very important.

I don't consider dynamic call sites important, so +1 from me.

  1. Friend libraries, which give other libraries access to a library's private identifiers. This would let you do white box testing by having your test library be a friend of the main library. It should also help with generated code.

    I know it's anathema to some, but I would allow a library to declare itself to be a friend of some other library. This prevents a library from encapsulating itself, but it means a test can crack open a library without the main library having to mention its test libraries. I don't look at access control as a strict security system — you can always modify the source anyway — and more of a velvet rope indication "you aren't supposed to be here".

    The semantics and syntax of this are really tricky though. You need some way of disambiguating private identifiers which may clash with the main library's.

It wouldn't really be private, right? It would be public except a static error to reference outside of certain libraries, and obviously tools like dartdoc could take advantage of this knowledge to avoid generating public APIs, etc.

@munificent
Copy link
Member

It wouldn't really be private, right? It would be public except a static error to reference outside of certain libraries

I was thinking it would actually be private. That ensures you don't run into accidental name collisions when, say, inheriting from the class in a non-friend library. But, again, it's only a half-formed sketch. Making it more public-ish might be a better approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language).
Projects
None yet
Development

No branches or pull requests

4 participants