Skip to content
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

Current experiment logic prevents us from being able to expire or remove experiment flags sanely #1245

Closed
jakemac53 opened this issue Jul 26, 2023 · 2 comments · Fixed by #1256

Comments

@jakemac53
Copy link
Contributor

I was recently trying to retire the dart 3.0 experiment flags (https://dart-review.googlesource.com/c/sdk/+/316460) but this ended up breaking most of our internal codebase.

The current way we deal with language features is to just enable the experiment as soon as formatting for it is supported, and we always set the sdkLanguageVersion to 2.19.0. This kind of works, until we go to retire or delete an experiment flag.

As an example my first fix for the current situation was to change the logic to parse with the current language version and no experiments, and then after that try to parse as 2.19.0. That works except that we have old versions of the formatter out there, and they will no longer work for any Dart 3.0 code.

There is no analyzer api for parsing with the current version plus some experiments that I can find (cc @scheglov I could be missing something) - if we had that we could add a third fallback that enables inline classes with the current language version (that feature has no experimental release version yet, but maybe we should require it to before it gets formatter support?).

We can somewhat alleviate the situation with breaking changes in analyzer any time an experiment is either expired or deleted, but that is very arduous.

Longer term, simply forcing users to explicitly enable features is more maintainable I think. I understand that places a burden on things that use the formatter as a library, but not many things actually do that.

@leafpetersen
Copy link
Member

Publishing code that relies on experiment flags is really something we should avoid. Or else we should really re-think our experiment flag story.

@munificent
Copy link
Member

Longer term, simply forcing users to explicitly enable features is more maintainable I think. I understand that places a burden on things that use the formatter as a library, but not many things actually do that.

SGTM.

I will have to pass those flags in the formatter's tests in order to land support for formatting a new language feature before the experiment ships. And then removing those flags will break the tests.

But that won't break any users. And, in particular, it means that new versions of analyzer that don't allow the flag won't break people using dart_style as a library.

munificent added a commit that referenced this issue Sep 1, 2023
…d. (#1256)

* Stop passing experiment flags to parser for features that have shipped.

Fix #1245.

* Allow passing experiment flags.

* Update lib/src/dart_formatter.dart

Co-authored-by: Jacob MacDonald <jakemac@google.com>

* Don't use FeatureSet.languageLanguageVersion().

---------

Co-authored-by: Jacob MacDonald <jakemac@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants