Repository navigation
☂️ Update lint set for Flutter applications #78432
Description
The lints currently applied by flutter analyze to flutter applications created with flutter create is outdated: analysis_options_user.yaml received its last meaningful update in October 2019. This umbrella issue tracks the work necessary to update those lints and align the set of lints recommended for Flutter application with the Dart universe.
Details of the approach are discussed in http://flutter.dev/go/flutter-lints.
Work Items
- Remove avoid_private_typedef_functions from recommend.yaml dart-archive/linter#2517, Remove
avoid_private_typedef_functionsfromcore.yaml - Lints to be considered for score.yaml or recommend.yaml from Flutter dart-lang/sdk#58349, Add Flutter-specific lints to core.yaml/recommend.yaml or new Flutter-specific lint set on top of recommend.yaml (WIP: Add lints for Flutter apps and packages dart-archive/lints#9)
- Reach an agreement on how to handle the
omit_local_variable_typeslint (Update: Removed fromrecommend.yaml) - identify place to host canonical lints dart-lang/sdk#58302, publish a 1.0.0 version dart-lang/core#842,
core.yaml,recommend.yaml, and potentially a Flutter-specific lint file are published in a package - Change
flutter createto generate ananalysis_options.yamlfile importing the new recommended set of lints from that package (RFC: Integrate package:flutter_lints into templates #81417) - Publish migration guide to upgrade existing apps to the new lints (Done: Migration guide for package:flutter_lints website#5749_)
- Deprecate analysis_options_user.yaml #82948 Deprecate
analysis_options_user.yamlwith a message (duringflutter analyze) to upgrade to a customanalysis_options.yamlfile importing the new recommended set of lints - Use pkg:lints and pkg:flutter_lints when analyzing code dart-lang/dart-pad#1881, Update DartPad to use the new lint sets
Punted
We've decided to remove unawaited_futures and prefer_mixins from the lint set for now so the following issues are no longer blocking and can be addressed later.
-
unawaited_futureslint is ready for Flutter code:- method-side opt-out for unawaited_futures dart-lang/sdk#58348, Method-side opt-out for unawaited_futures
- Move unawaited to package:meta? dart-lang/sdk#58351, Move unawaited to package:meta
- Enable the lint in the flutter codebase for dogfooding (opting out some
NavigatorandAnimationControllermethods, maybe others)
-
prefer_mixinslint is ready for Flutter code:- opt-out for old-school mixin-like classes from prefer_mixin dart-lang/sdk#45343, opt-out for old-school mixin-like classes from prefer_mixin
- Opt-out
ChangeNotifierandWidgetsBindingObserver, enable lint in flutter codebase for dogfooding
/cc @pq @Hixie @mit-mit @munificent @devoncarew I think these are the work items that came out of our meeting the other day.
Activity
devoncarew commentedon Mar 17, 2021
One comment:
If we do decide that we need a Flutter specific lint file (if we're not able to in-line everything into core and recommended), the flutter specific lints could just be shipped as part of the flutter package (e.g., packages/flutter/lib/flutter_recommended.yaml).
goderbauer commentedon Mar 17, 2021
I believe that would cause versioning issues: Whenever you upgrade your version of flutter you'd be forced to also upgrade your code base to the latest lints in
packages/flutter/lib/flutter_recommended.yaml. If it ships as part of a separate (versioned) package, updating flutter and updating the set of recommended lints can happen independently.devoncarew commentedon Mar 17, 2021
It would definitely be a decision in terms of how we want these to roll out. We could version them with the sdk and ship changes as part of new stable versions.
pq commentedon Mar 17, 2021
/fyi also @davidmorgan @bwilkerson
davidmorgan commentedon Mar 18, 2021
Thanks everyone!
Commented on the doc re: versioning.
package_pedanticsolves this by having versioned files within the package, so everyone can always be on the latest version and still choose which version they want. Additionally, there is one file that always includes the latest lints, in case you want to auto update rather than pin.That model seems to work :)
goderbauer commentedon Mar 18, 2021
It is not clear to me why you'd need the double versioning (pub package version + yaml version). If the package doesn't contain any code, wouldn't it be enough to just use the package version? Whenever a new lint is added, it's treated as a breaking change and the MAJOR version number is bumped. If you're ready to upgrade to the latest set of recommended lints (and fix the warnings produced by it), you bump the package version in you package's pubspec.
davidmorgan commentedon Mar 19, 2021
Sorry, my mistake--I checked: the analyzer has no problems resolving different yaml versions from different packages. Please ignore :)
lrhn commentedon Apr 7, 2021
The reason for double-versioning is that it allows you to not use the most recent version of the lints.
That's not important if the lints package is only a dev-dependency for all packages. If any package you depend on uses the lints package as a non-dev-dependency, you'll need to use a version accepted by that other package.
If we make a change to the lints file, by adding a lint, your code might no longer be lint-warning-free. Some build systems might consider that an error. That means that you need to migrate your code to the new lints, and until then, you might want to keep your lints at the previous version.
Again, if everybody uses the lints package as a dev-dependency only, you can just lock yourself to an older version of the lints package. If not, you might need to depend on a newer version of the lints package, and the versioned yaml files allows you to still use the older lints file.
(Arguably, you can also just add
the_new_lint: ignoretoanalysis_options.yamlto override the import, but that requires more work for the user to figure out which precise lints have changed between versions).goderbauer commentedon Apr 7, 2021
The above was assuming that the package is added as a dev-dependency, which I think is the right thing to do for this package. If you don't do that correctly, it will cause problems, yes. But that's also true for other dev-dependencies that were accidentally added as a regular dependency, no? E.g. if I accidentally add package:test as a regular dependency I am also forcing the hands of other packages that depend on me...
davidmorgan commentedon Apr 8, 2021
We know from
pedanticthat making it a full dependency is a bad idea ;) we don't have an example for publishing yaml in a dev dependency always working well, but as far as I can tell it should :)Hixie commentedon Apr 8, 2021
Can we put something in a package's
pubspec.yamlthat would make it impossible to include as a regular (non-dev) dependency?pq commentedon Apr 8, 2021
@jonasfj ?
jonasfj commentedon Apr 8, 2021
No such thing exists, but we could introduce it.
I think it's a bit strong to say "never". Perhaps it's sufficient to introduce something that:
dart pub add <package>to add it asdev_dependencydartanalyzer.Maybe it's just as simple as having
is_dev_dependency: true, we can always deprecate such a feature without side-effects.The downside of completely forbidding consumption as non-dev dependency is that any package intended to be used as a dev-dependency could certainly be used by another tool that is itself a dev-dependency.
pq commentedon Apr 8, 2021
FWIW, I really like this proposal. 👍
davidmorgan commentedon Apr 9, 2021
SGTM.
Prior art for is_dev_dependency: bazel's testonly attribute (no direct link, sorry, please search that table). Note that would suggest that an
is_dev_dependencypackage can use otheris_dev_dependencypackages as a full dependency.So e.g.
package:testcan be markedis_dev_dependency, and mypackage:more_awesome_testcan be markedis_dev_dependencyand depend onpackage:testas a full dependency.This is a big step beyond the simple non-transitive version we need for lints yaml, but maybe worth a thought :) could be added to the simple version later, so doesn't need to block anything.
goderbauer commentedon Apr 28, 2021
Updated the top comment with the current status.
mit-mit commentedon May 12, 2021
@goderbauer I created tracking issues for the punted lints in the
package:lintsrepo:https://github.com/dart-lang/lints/issues?q=is%3Aissue+is%3Aopen+label%3Atype-lint
goderbauer commentedon May 19, 2021
The main goal of this issue has been achieved, a migration guide for existing projects is published at https://flutter.dev/docs/release/breaking-changes/flutter-lints-package.
What's left to do is to update DartPad (tracked in dart-lang/dart-pad#1881) and actively nudging existing/established projects to upgrade to
pkg:flutter_lints(this is tracked in #82948).I am going to close this umbrella issue since the main work is done and the remaining auxiliary work is tracked separately.
github-actions commentedon Aug 1, 2021
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.