Sdk: LibraryElement: expose the "source" of the language version

[kevmoo]

Right now, we have

  /// The major component of the language version for this library.
  int get languageVersionMajor;

  /// The minor component of the language version for this library.
  int get languageVersionMinor;

This is great, but we also need to know the source of the version – specifically, we need to know if the version is from a language version comment – // @dart=2.9.

This will allow us to generate parts that match any existing comment on a library. See also https://github.com/dart-lang/sdk/issues/42456

[bwilkerson]

Consider adding a single object (VersionInfo?) that can be returned and queried rather than having three (or more) separate getters on LibraryElement.

[kevmoo]

CC @stereotype441 @leafpetersen @natebosch @jakemac53

[kevmoo]

Consider adding a single object (VersionInfo?) that can be returned and queried rather than having three (or more) separate getters on LibraryElement.

Yes, please @bwilkerson !

I wonder if we want more than just a bool – we could get the language from the library definition, pubspec, or looking at the code there is some hard-wired version mapping to the latest known SDK version?

[bwilkerson]

While that's an interesting question, I'd recommend designing the smallest API that satisfies the known needs while allowing for clean future expansion. We might someday have a need to ask for the language version specified by a language version comment in the library (or null if there is none), the minimum and maximum versions supported by the containing package, and the maximum version supported by the current SDK, but I'd hate to add that much API speculatively. I believe that at the moment you just need the first of those.

[kevmoo]

Fine by me, @bwilkerson – points for getting this done soonish. It's pretty blocking for doing source generation correctly.

[bwilkerson]

@scheglov Interested in hearing your thoughts.

[scheglov]

I plan adding following data as LibraryElement.languageVersion:

class LibraryLanguageVersion {
  /// The version for the whole package that contains this library.
  final Version package;

  /// The version specified using `@dart` override, `null` if absent or invalid.
  final Version override;

  LibraryLanguageVersion({
    @required this.package,
    @required this.override,
  });

  /// The effective language version for the library.
  Version get effective {
    return override ?? package;
  }
}

@kevmoo will this work for you?

[kevmoo]

I need an explicit signal. Checking for override != null seems like it'll fit the ticket.

Where are you going to get Version from? pkg:pub_semver?

@natebosch @jakemac53 – thoughts??

[scheglov]

Yes, Version is from pkg:pub_semver.

[kevmoo]

Could I be picky and ask for String get overrideString that's either null or formatted perfectly as // @dart=X.Y – then folks don't have to crack open a version and do it correctly!

(This could simply be in addition to final field v/ Version)

[scheglov]

Hm... Why? This data is already available, clients could format it themselves.
Or write an extension to do this :-P

Is this API going to be used by many codegen implementations and we want it to provide a consistent helper?

[kevmoo]

Code gen is the ONLY reason I can think of to use this API. I can admit, I
will only use it in pkg:source_gen and most other places won't have to
think about it.

...so not a BIG deal. Just helpful.

On Thu, Jun 25, 2020 at 9:58 AM Konstantin Scheglov <
[email protected]> wrote:

Hm... Why? This data is already available, clients could format it
themselves.
Or write an extension to do this :-P

Is this API going to be used by many codegen implementations and we want
it to provide a consistent helper?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/dart-lang/sdk/issues/42474#issuecomment-649702154,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAEFCU4WC7QMZIXTRWTPRLRYN6ZTANCNFSM4OHFORWQ
.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/152601

[kevmoo]

Thanks, @scheglov !

Could we move to ship 0.39.11 soonish? (CC @stereotype441 )

[kevmoo]

Fixed and analyzer published!