Analyzer should recognize features not accepted by a package's minimum required SDK version.

[lrhn]

Example: In Dart 2.1, the dart:core library re-exports Future and Stream from dart:async.

If you write a package and use Future or Stream without an import, the 2.1.0 analyzer will say that is perfectly OK. However, if your pubspec SDK version requirment allows a 2.0.0 SDK, then the code might not work there.

Similarly, Dart 2.1 introduced the ability to use integer literals in a double context, as a shorthand for the double litteral. Again, code using this with an SDK requirement below 2.1.0 is not correct.

Each Dart SDK released can introduce new non-breaking language features or library extensions. The analyzer should keep track of those changes and compare them to the minimum required SDK allowed by a package.

For language features, the versioning information will likely need to be hard-coded into the analyzer.
For library changes, we could introduce an annotation like @Since("2.1") on new parts of the API, which would apply to entire libraries, exports, single declarations, or even individual parameters.
(We can't mark API removals that way, but removals are breaking changes, so we will likely only do them on major version increments anyway. We can add version numbers to deprecations before we remove the feature, so something can be @Deprecated(since: "2.2"), but I'm not sure that's actually helpful. Or we can document removed fatures @Deprecated.andRemoved(namedArgument: #foo).)
It might be easier to just maintain a structured API changelog on the side.

[bwilkerson]

I actually argued for several years that analyzer would need to be aware of the differences between different versions of the SDK for exactly this reason. The answer I was given is that users are expected to use the version of the tools that were shipped with the version of the SDK that they're using. While I don't object to being proved right :-), I will point out that providing this level of support would probably be a non-trivial amount of work, so we'll need to schedule that work.

I'll also point out that unless the front end team is willing to take on this additional support (@kmillikin), adding this requirement for the analyzer would place additional difficulties in the path of eventually getting the analyzer ported over to use the front end. I'm not sure we want to make that goal harder to achieve.

[leafpetersen]

cc @a-siva @zanderso @vsmenon I think this is essentially the same issue we discussed a week or so ago. I think @zanderso was going to delve into it a bit more. It's not just an issue for the SDK: flutter has the same issue (if they add to their API, then code written against that new API is not compatible with older flutter SDKs).

[zanderso]

FYI @tvolkert

[natebosch]

Pub is another potential place to implement a check since the problem surfaces when the package is published. dart-lang/pub#1981

[lrhn]

I think the analyzer is a good place to do the checking. it can alert the user while they are writing, and it's (probably) a simple quick-fix to update the SDK requirement in the pubspec.yaml file.

Pub can do the same thing too, but that would just amount to analyzing the package source again.

My main question is probably which metadata is needed to actually do this.
I'm fine with adding @Since("2.1") annotations on the APIs, that would solve the problem for Future and for any other API changes.
It will not handle language changes. That will require the front-end to let the analyzer know which language features are used (no desugaring integer literals in double contexts into double constants before passing it to the analyzer). That's probably necessary anyway, otherwise the analyzer can't give good enough warnings.

I'm willing to start with @Since annotations. WDYT?

[bwilkerson]

... a simple quick-fix to update the SDK requirement ...

Yes, that would be easy to implement.

It will not handle language changes.

As I understand it, the plan, going forward, is for every new feature to initially be implemented behind an "experiment" flag. My initial assumption was that when the experiment was enabled by default that we would remove all of the references to that flag in the analyzer code base.

But one possibility for supporting multiple versions would be for analyzer to always keep that functionality behind the flag and to keep track of which experiments are enabled by default in each version. When a package is to be analyzed, analyzer could look to see which SDK is being used and enable the appropriate set of flags.

(It makes me kind of sad to think about the kind of code we'll need to have in the analyzer to deal with all of the different versions of the SDK. It won't be easy to maintain.)

There's another aspect of this: we need to be able to analyze a package against the same version of the SDK's APIs as the version of language features. If we're getting the APIs from the SDK and the set of language features from the version constraint in the pubspec, then we can easily have inconsistencies.

I think the better approach is for the information about which features are supported to be included directly in the SDK. When analyzer finds the SDK to be used (how it finds it can be a separate discussion if the current approach is not adequate), it can get everything it needs to know about both language features and the API (and the two pieces of information will always be consistent).

Analyzer can check to see whether the SDK matches the minimum bounds defined in the pubspec and let the user know if the two don't match.

I'll note that getting all of the information from the SDK also allows us to analyzer code that does not have a pubspec.

[lrhn]

We plan to have some sort of language-version opt-in/out, likely on a per library basis.
A library will uses the newest language version unless otherwise restricted. It can be restricted by having an explicit lower language version written in the library file, or by being in a package with a lower SDK version dependency.

It does mean that when a language feature is enabled by default, it can still be disabled by the library opting to be compiled using a lower language version. You can just be certain that if a feature is disabled, so are all features added in the same or later language versions, so the implementation of a feature can always depend on all features that are not newer than itself.

When we do get the language-version opt-in/out feature, we will say that experimental features only work in libraries that are compiled using the most recent language version. You can't go back and use 2.1 language semantics with a single 2.4 feature. You have to be on language version 2.3 to use experimental features planned for 2.4.

The default language version used by the libraries of a package is derived from its SDK version constraint. That would mean that experimental features are always disabled if you are not depending on the actual SDK that you are using. Individual libraries may opt for a lower language version, then they will not see experiments either.

So:

• Packages specify their SDK dependency in the pubspec.
• Language changes are handled as progressive versions. We enable compiling at older versions to allow gradual migration. Language experiments only work on top of the most recent language version, until they become the new language version. A package should not/cannot opt any library in to a version newer than its pubspec SDK version, which is also the default language version for its libraries. The analyzer (and all language tools) must be able to run without newer features. This should be easy if the features are developed below a flag to begin with.
• Library changes are non-breaking. We can mark changes with @Since markers. The analyzer can warn if a library uses SDK features newer than its pubspec SDK version.

The analyzer should not need to know about more than one set of SDK libraries. It does need to know about language features and which version they are enabled from. For incremental changes, it's fine to parse the feature and then report an error. For breaking language changes, the same syntax may be valid in both versions and mean something different.

[bwilkerson]

Ok. That does mean that our tools will need to have an SDK-independent way of figuring out which features were added in which versions.

Packages specify their SDK dependency in the pubspec.

Just one nit: not all packages have a pubspec. Bazel and Gn packages are notable examples. It would be nice if the eventual specification of this support recognized that. Also, we should have some agreed on semantic for code that isn't in any kind of package structure (such as defaulting to the SDK containing the VM being run).

[jakemac53]

Related #35158

[stereotype441]

Note that #35158 illustrates a specific instance of this bug that is proving particularly troublesome, even for google-owned packages: package owners are beginning to rely on the fact that as of SDK version 2.1.0-dev.5.0, dart:core exports Future, but they aren't updating their pubspecs to reflect that. As a result, users who are still on SDK version 2.0.0 are getting errors. We may want to write a special case lint to address this particular issue with Future ASAP, even if it takes us longer to address the general case.

Escalating to P1.

[jakemac53]

Can we get this assigned to the 2.1 SDK milestone? As soon as this is in a public release it will become a much bigger issue, and we won't be able to easily fix it for those users for a long time (until 2.2...)

[natebosch]

a specific instance of this bug that is proving particularly troublesome, even for google-owned packages

That is the same flag I raised when the feature was first introduced:

#26162 (comment)

[stereotype441]

I moved the high priority parts of this issue (referring specifically to the import of Future from dart:core) to #35162. Dropping the priority of this issue back to P2.

[jakemac53]

Friendly ping on this issue, as it seems to have gotten lost in the shuffle. We have several @Since annotations in the SDK now, but they aren't useful as they aren't being used by anything. This gives users a false sense of security, they probably assume that they will get a warning if they need to update their min SDK based on using anything annotated with @Since, but that is not the case.

[jakemac53]

I can take a quick look at this today since its close to my ❤️

[lrhn]

Just to add to your troubles: The goal is to give people a warning if they use a feature which is only available through a declaration with a @Since annotation with a version later than their min-SDK requirement.

The Future export from dart:core was a good example of that.
If you import dart:async as well, you have two imports which expose the same declaration (dart:async::Future), and only one has a @Since annotation (on the export itself), so there is no need to warn, even with a 2.0.0 min-SDK.

Similarly, the BytesBuilder was previosuly exposed only by dart:io, but later also by dart:typed_data. (And I now notice that export lacks the @Since, the other one has a @Deprecated though).

[jakemac53]

Ok, unassigning myself then 🤣 . It doesn't sound like something which I can reasonably fix in a short timeframe.

[jakemac53]

Should we consider removing all @Since annotations, and converting them to simple doc comments? At least until we have actual support?

Currently we are promising something that does not exist.

[bwilkerson]

If we remove the annotations then there won't be any motivation to add the support.

As for the implementation, we have other (non-annotation-based) checks for the use of language features that are not supported by the min SDK version, and the same pattern could be followed.

[jakemac53]

But there doesn't seem to be any plan to implement this, the request is over 3 years old and there is no active work scheduled.

If we aren't going to implement it, we should stop pretending we do, imo. Or, we should get it prioritized to a level that it will get on OKRs.

[bwilkerson]

I think there's an interesting assumption here that I'd like to make explicit in order to confirm: annotations that are not statically checked have no value.

If so, I'd like to challenge that assumption. I think that annotations are at least as good a way to express intent / contracts as documentation comments. I actually think they're often better because, for me at least, it's harder to miss the presence of an annotation than it is to miss a sentence somewhere in a doc comment.

Or, we should get it prioritized to a level that it will get on OKRs.

Personally, I think that this is too small an item to make it appropriate for OKRs. But ignoring that, it sounds like you think that it hasn't been prioritized correctly relative to the items that have made it onto the OKRs. Am I misunderstanding what you're saying?

[jakemac53]

I think that annotations are at least as good a way to express intent / contracts as documentation comments.

When I see an annotation, I assume it is there for some tool to read. I am not aware of any annotations that are not consumed by a lint or something else.

So when we started adding these annotations, I have assumed for a long time now that the analyzer was checking them, and I have been relying on that to know when I need to increase my min SDK constraint. We are often fairly good about running tests on stable SDKs in our own packages, and so we probably haven't actually made a mistake here, but I do feel like the annotation is making a promise that it isn't keeping.

So it isn't that I don't think the annotation does a good job of communicating things at a textual level, but more that there is some additional implicit support that I assume when I see an annotation, which I wouldn't assume from just a doc comment. So from my perspective it is better not to have the annotation, than to not have any tool reading it. But that is just my personal opinion, because I was myself tricked by it into thinking something was supported which was not.

Personally, I think that this is too small an item to make it appropriate for OKRs. But ignoring that, it sounds like you think that it hasn't been prioritized correctly relative to the items that have made it onto the OKRs. Am I misunderstanding what you're saying?

Whether it is on OKRs or not, I do think this should be highly prioritized. It is a very useful feature that prevents people from falling into a very easy trap (using an api that doesn't exist on an SDK they claim to support).

Yes, people should be running their tests on both their minimum and maximum SDK constraints. But they don't do that often in practice, and even on the Dart team there is not agreement that it is worth doing it, and it is likely not consistently done across all our packages.

[bwilkerson]

When I see an annotation, I assume it is there for some tool to read.

Thanks! I understand your perspective better now.

It is a very useful feature that prevents people from falling into a very easy trap ...

I agree that this would be a useful feature, so I'm going to increase the priority of the issue. Hopefully it can get addressed fairly soon.

[natebosch]

If we can get support for @Since on entire methods and arguments I think that would be worth doing, even if we don't get support for @Since on exports. I suspect that if we can implement support for deprecating exports (#48807) then the implementation could look similar for @Since

[vsmenon]

@bwilkerson @srawlins - can we either:

• keep this as a P1, assign it to someone, and aim for an upcoming release
• downgrade to P2

?

[srawlins]

Thanks for the ping, @vsmenon. I've added this to our agenda for tomorrow's meeting, and have downgraded it to P2 for now. We'll have a firmer plan tomorrow.

[srawlins]

OK we decided to leave this as a P2. It certainly was more like a P1 with the Dart 2.1 issues of exporting Future from core, but that is certainly affecting users less now. There was a consensus that this is an important feature, and it is added to our next planning doc.

It also sounds like the common case (e.g. referencing a class which is new in some version and doesn't exist in a previous release) should be easier to implement. The case of "this element is newly being exported" is trickier, and can be implemented separately, and needs some groundwork.

[stereotype441]

FYI, we encountered another instance recently where this feature would have been useful. In 63b49df, a new method was added to the SDK and used in the analyzer in the same commit; this doesn't work because the analyzer is published on pub (and hence has to work with older SDK versions). Fortunately I noticed the problem because I checked out the latest analyzer codebase with an SDK that was a few days old. However, as soon as I build the latest SDK there won't be any safeguard to stop me from accidentally starting to use that method again.