add additional lints to dart_flutter_team_lints

[devoncarew]

• add additional lints to dart_flutter_team_lints
• close general updates to package:dart_flutter_team_lints #110

This adds 7 of the lints that we've commonly enabled on dart-lang packages; see #110 for the full list and counts from our repos.

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[github-actions]

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:dart_flutter_team_lints	2.0.0	ready to publish	dart_flutter_team_lints-v2.0.0
package:firehose	0.3.27	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[github-actions]

PR Health

Package publish validation ✔️

Details

Package	Version	Status
package:dart_flutter_team_lints	2.0.0	ready to publish
package:firehose	0.3.27	already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

License Headers ✔️

Details

// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Changelog Entry ❗

Details

Package	Changed Files
package:firehose	pkgs/firehose/lib/firehose.dart pkgs/firehose/lib/src/github.dart pkgs/firehose/lib/src/health/health.dart pkgs/firehose/lib/src/repo.dart

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check

Coverage ⚠️

Details

File	Coverage
pkgs/firehose/lib/firehose.dart	💔 Not covered
pkgs/firehose/lib/src/github.dart	💚 10 %
pkgs/firehose/lib/src/health/health.dart	💔 Not covered
pkgs/firehose/lib/src/repo.dart	💚 87 %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check

[devoncarew]

Here are the lint results from this repo:

   info • pkgs/blast_repo/lib/src/top_level.dart:9:8 • Use relative imports for files in the 'lib' directory. Try converting the URI to a
          relative URI. • prefer_relative_imports
   info • pkgs/corpus/bin/deps.dart:60:50 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to
          the constructor invocation. • prefer_const_constructors
   info • pkgs/corpus/lib/api.dart:52:28 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to the
          constructor invocation. • prefer_const_constructors
   info • pkgs/corpus/lib/api.dart:74:9 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword to the
          constructor invocation. • prefer_const_constructors
   info • pkgs/corpus/lib/report.dart:7:8 • Use relative imports for files in the 'lib' directory. Try converting the URI to a relative
          URI. • prefer_relative_imports
   info • pkgs/corpus/test/api_test.dart:21:11 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword
          to the constructor invocation. • prefer_const_constructors
   info • pkgs/corpus/test/api_test.dart:24:13 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword
          to the constructor invocation. • prefer_const_constructors
   info • pkgs/corpus/test/api_test.dart:46:13 • Use 'const' with the constructor to improve performance. Try adding the 'const' keyword
          to the constructor invocation. • prefer_const_constructors
   info • pkgs/firehose/lib/firehose.dart:10:8 • Use relative imports for files in the 'lib' directory. Try converting the URI to a
          relative URI. • prefer_relative_imports
   info • pkgs/firehose/lib/src/github.dart:9:8 • Use relative imports for files in the 'lib' directory. Try converting the URI to a
          relative URI. • prefer_relative_imports
   info • pkgs/firehose/lib/src/health/health.dart:11:8 • Use relative imports for files in the 'lib' directory. Try converting the URI
          to a relative URI. • prefer_relative_imports
   info • pkgs/firehose/lib/src/repo.dart:7:8 • Use relative imports for files in the 'lib' directory. Try converting the URI to a
          relative URI. • prefer_relative_imports
   info • pkgs/repo_manage/lib/issue_transfer.dart:202:34 • Use 'const' with the constructor to improve performance. Try adding the
          'const' keyword to the constructor invocation. • prefer_const_constructors
   info • pkgs/repo_manage/lib/weekly.dart:62:47 • Use 'const' with the constructor to improve performance. Try adding the 'const'
          keyword to the constructor invocation. • prefer_const_constructors
   info • pkgs/repo_manage/lib/weekly.dart:65:48 • Use 'const' with the constructor to improve performance. Try adding the 'const'
          keyword to the constructor invocation. • prefer_const_constructors

So, 8 prefer_const_constructorss and 6 prefer_relative_importss. Both of those have dart fixes available.

[natebosch]

This one could lead to forced breaking API changes if a constructor parameter stops being used but is still passed by callers.

I don't know if it would ever occur that a constructor argument starts being ignored and it's not already a breaking behavior change, but I'm a little paranoid about lints that could suggest breaking API changes because an author applying a quick fix is less likely to consider impact on usages than an author manually changing arguments...

[devoncarew]

Yes, people will feel the need to address these issues, and will assume that the lint is telling them the right thing to do.

[natebosch]

@sigmundch - is there any risk of degrading IPL or any other negative impacts by making too many values const in widely used packages?

[devoncarew]

I don't have a strong opinion with this lint, but note that we do have it enabled in 22 of 29 sampled dart-lang packages, and flutter turns this lint on.

[sigmundch]

Indeed, it moves initialization of each of those values to be done eagerly during the IPL as you noted (as opposed to being lazy or on-demand at the time you need it.). The question comes down to how many constants we are talking about. If it's just a handful of them, it's fine, but if all the sudden this will increase the total number of constants by 2x, that's more concerning.

cc @rakudrama, although he is about to be on PTO for a while.

[rakudrama]

It will degrade IPL but a few small constants will not make much difference. It is really hard to give advice here since it depends a lot on the number and qualities of the constants. A few moderate sized class constants would be fine, unless one of their fields were initialized with, say, a gargantuan map.

I'd like to make dart2js be smarter about the impact of larger constants on IPL (e.g. dart-lang/sdk#32943) but I don't think it will ever be zero.

I would say go ahead since constant widgets, being identical each frame, help the flutter framework more efficiently detect parts of the widget tree that are invariant. It can translate to better frame rate.

[natebosch]

I don't think this lint is reliable enough - there are false positives.

[devoncarew]

sgtm - will remove

[devoncarew]

Updated to remove avoid_unused_constructor_parameters and cancel_subscriptions.

Note that I also added in conditional_uri_does_not_exist - not in the original PR.