☂️ Update lint set for Flutter applications

[goderbauer]

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_functions from core.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_types lint (Update: Removed from recommend.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 create to generate an analysis_options.yaml file 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.yaml with a message (during flutter analyze) to upgrade to a custom analysis_options.yaml file 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_futures lint 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 Navigator and AnimationController methods, maybe others)
• prefer_mixins lint 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 ChangeNotifier and WidgetsBindingObserver, 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.

[devoncarew]

One comment:

core.yaml, recommend.yaml, and potentially a Flutter-specific lint file are published in a package

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]

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]

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]

/fyi also @davidmorgan @bwilkerson

[davidmorgan]

Thanks everyone!

Commented on the doc re: versioning. package_pedantic solves 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]

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]

Sorry, my mistake--I checked: the analyzer has no problems resolving different yaml versions from different packages. Please ignore :)

[lrhn]

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: ignore to analysis_options.yaml to override the import, but that requires more work for the user to figure out which precise lints have changed between versions).

[goderbauer]

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]

We know from pedantic that 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]

Can we put something in a package's pubspec.yaml that would make it impossible to include as a regular (non-dev) dependency?

[pq]

Can we put something in a package's pubspec.yaml that would make it impossible to include as a regular (non-dev) dependency?

@jonasfj ?

[jonasfj]

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:

1. Changes dart pub add <package> to add it as dev_dependency
2. Produces a warning in dartanalyzer.

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]

Maybe it's just as simple as having is_dev_dependency: true, we can always deprecate such a feature without side-effects.

FWIW, I really like this proposal. 👍

[davidmorgan]

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_dependency package can use other is_dev_dependency packages as a full dependency.

So e.g. package:test can be marked is_dev_dependency, and my package:more_awesome_test can be marked is_dev_dependency and depend on package:test as 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]

Updated the top comment with the current status.

[mit-mit]

@goderbauer I created tracking issues for the punted lints in the package:lints repo:
https://github.com/dart-lang/lints/issues?q=is%3Aissue+is%3Aopen+label%3Atype-lint

[goderbauer]

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]

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 -v and a minimal reproduction of the issue.