identify place to host canonical lints

@munificent

Let the bike-shedding begin!

To start, here are a few proposals.

Proposal 1 : (new) package:canonical

Name

package:canonical (https://pub.dev/packages?q=canonical 👍 )

Structure

lib/
  analysis_options.yaml
  canonical_1.0.0.yaml
  recommend/
    canonical_1.0.0.yaml
    analysis_options.yaml

A few details:

analysis_options.yaml just include:s latest the same way pedantic currently does
lib/recommend/canonical_1.0.0.yaml has an include: of the root (scoring) lints and adds the recommended ones to it

Sample Uses

To get the latest scoring lints:

include: package:canonical/analysis_options.yaml

or to pin to a version:

include: package:canonical/analysis_options.1.0.0.yaml

Turning the dial up to the recommended lints looks like:

include: package:canonical/recommend/analysis_options.yaml
include: package:canonical/recommend/analysis_options.1.0.0.yaml

Proposal 2: package:pedantic

Alternatively, we could use the existing package:pedantic. (Cheers @munificent for this one.)

Structure

lib/
  analysis_options.yaml
  pedantic_1.0.0.yaml
  pub/
    pedantic_1.0.0.yaml
    analysis_options.yaml
  recommend/
    pedantic_1.0.0.yaml
    analysis_options.yaml

In this case the "root" one would probably have to remain the current google style.

Sample Uses

To get google style:

include: package:pedantic/analysis_options.yaml
include: package:pedantic/analysis_options.1.0.0.yaml

Pub scoring:

include: package:pedantic/pub/analysis_options.yaml
include: package:pedantic/pub/analysis_options.1.0.0.yaml

Recommended:

include: package:pedantic/recommend/analysis_options.yaml
include: package:pedantic/recommend/analysis_options.1.0.0.yaml

/fyi @bwilkerson @munificent @natebosch @jakemac53 @davidmorgan @mit-mit @lrhn for input

I think this should not be a pub package, this should be something shipped in the SDK. I don't see any way that we can enable scoring lints by default (even if we don't do it to start with) if they are defined in a package.

I think this should not be a pub package, this should be something shipped in the SDK. I don't see any way that we can enable scoring lints by default (even if we don't do it to start with) if they are defined in a package.

We could make it so that pub lish warns if you don't add the scoring lints? Regular apps and non-published packages though would be harder to point at these lints to be sure.

Putting it in the SDK is not without downsides either - it is harder to evolve, adds more friction to updating your sdk, and makes it so users on old sdks don't see the updated scoring lits. Users on the pre-release sdks will also see a different set than pub.dev sees (assuming it uses stable).

As far as the original proposal - I would be somewhat inclined to move the top level analysis_options.yaml under pub_score directory? It isn't clear to me that those are the pub scoring lints with the current structure.

I would also probably go for recommended instead of recommend.

If we put it in the SDK, that means the set of lints can spontaneously change on users when they upgrade SDKs. If we put it in a package, it gets pinned in their lockfile and upgraded when they choose.

it gets pinned in their lockfile and upgraded when they choose.

If this is a goal it excludes showing lints to folks without config. I think that work we do now which encourages more config today it's a step in the wrong direction - early opt-in is one thing but additional config like a package dep that is temporary feels like noise and debt to clean up later.

I think it's important that we can provide help and tips to users even if they didn't start their project with stagehand or know to add some magic config to their analysis_options.yaml and pubspec.yaml.

set of lints can spontaneously change on users when they upgrade SDKs

We could choose which lints to show based on language version. That's a mechanism that would be very likely to be useful down the road anyway.

If we put it in the SDK, that means the set of lints can spontaneously change on users when they upgrade SDKs. If we put it in a package, it gets pinned in their lockfile and upgraded when they choose.

Fwiw, I think this is actually a negative for the package option. At least when it comes to scoring related lints I think we would always want all users to see exactly what pub will be scoring on right now, and not have to do anything special to update. It is possibly more likely that shipping it with the sdk would achieve that more consistently.

We also don't have a built in way of updating a single package, so upgrading lints becomes tied in with updating all dependencies. Granted the sdk option has a similar problem of it being tied to the sdk update.

What are canonical lints? Which rules are included? Could they be added to effective_dart?

If we put it in the SDK, that means the set of lints can spontaneously change on users when they upgrade SDKs. If we put it in a package, it gets pinned in their lockfile and upgraded when they choose.

For scoring lints, I think we have to move towards a breaking change like notification process anyway, given how impactful they are to package authors.

FWIW I suggest package:analysis_options if we end up with a new package.

Re: pinning, having versioned yaml files within the package allows people to choose between pinning and not pinning, even if it auto updates with the SDK.

If we distribute the package with the SDK, this will be the first package that comes with the SDK. We'll need new resolution logic in the Pub tool (it's a package which is not fetched from pub), and we'll need to decide whether to move it into the pub cache, or whether to link directly to it in the SDK repo (I'd probably go for the former).

And if someone wants to use a newer the package with an older SDK, they're completely out of luck.

I think I'd prefer to just publish it as a normal package. I don't want to mix it with package:pedantic.

We can make Pub recommend a pub get when it sees that your analysis options package is not up-to-date (older than what the Pub tool itself uses).

I'm probably fine with having the analyzer use a default set of lints (which is the newest recommended set) if there is no lint configuration at all, or only lint-x :ignore configurations. If there is any positive lint configured, then drop the defaults and use the configuration.

Pub supports SDK constraints (Flutter uses them extensively), so it's not very much work if we wanted to.

dependencies:
  canonical:
    sdk: dart

Regarding versioning, we could still allow referencing older versions explicitly, e.g. something like this for selecting the 2.12 scoring lints?

analysis_options.yaml:

include: package:canonical/scoring/2.12/analysis_options.yaml

If this ends up being a package: There was some discussion over on flutter/flutter#78432 (comment) whether we need double-versioning (i.e. the package itself has a version and then each version includes a set of versioned analyzer_options.yaml files as suggested in the very first message of this thread). The conclusion there was that one version - the version of the package - would be enough to simplify things.

Also: In flutter the idea is that flutter create would set this all up. It would generate whatever is needed to enable the canonical lints.

@devoncarew

From recent discussion between @devoncarew and myself, we'd like to not store the lints in a published package. Doing so has a number of user experience issues:

No linting available for bare Dart files (e.g. mkdir myapp; cd myapp; nano hello.dart; dart analyze)
No way of upgrading all projects to a new set of lints when upgrading to a new major SDK (similar to what Flutter does today)
No way of ensuring consistency between the version being run with locally and the version pub will run scoring with

Thus, we'd like to suggest we store the lints in the Dart SDK, for example in dart-lang/sdk/pkg/lints and that we do not publish this package to pub.

With that, I'd like to still support manually selecting an older lint set, so I suggest we store the files inside /pkg/lints with a new file for every major SDK version, similar to pedantic, e.g. for the Dart 2.13 release:

/pkg/lints/lib/analysis_options.2.10.yaml
/pkg/lints/lib/analysis_options.2.12.yaml
/pkg/lints/lib/analysis_options.2.13.yaml
/pkg/lints/lib/analysis_options.yaml: just contains an import of /pkg/lints/lib/analysis_options.2.13.yaml

... and that we do not publish this package to pub.

How would a user import from one of these analysis options files if the package isn't published?

No way of upgrading all projects to a new set of lints when upgrading to a new major SDK (similar to what Flutter does today)

It wouldn't be force updated, but a pub upgrade would update it. That has some advantages in that you can update it more frequently than SDK rolls as well.

No way of ensuring consistency between the version being run with locally and the version pub will run scoring with

Imo the proposed solution doesn't solve this, it actually makes it worse. If this is in a package it allows users of older sdks to use the newer lints (what pub is scoring with). If shipped with the SDK then users on older sdks have no way of getting the new lints. The pub lish command could warn if it sees you have an outdated version as well.

For some clarity, what we'd discussed (off-line) was something like:

include: dart:core/lints/core.yaml

And the implementation file(s) in the sdk would live in <dart-sdk>/lib/core/lints/core.yaml.

In other words, we'd use dart: scheme URIs to refer to the sdk vended analysis options files.

IMO the recommended lints should be on by default and the way to back out of them should be something like

analyzer:
  use_recommended_lints: false

and we can allow opting out of scoring lints which folks might want to do if they aren't publishing.

analyzer:
  use_recommended_lints: false
  use_pub_score_lints: false

No way of upgrading all projects to a new set of lints when upgrading to a new major SDK (similar to what Flutter does today)

I am actually seeing this as a downside (and I think it has been one of the reasons we haven't updated the set of enforced lints in Flutter for more than a year). If the set of enforced lints is determined by my (Flutter or Dart) SDK version I am going to experience more breaking changes in my project if I am upgrading to a new SDK that introduces additional lints (assuming that I have CI running to ensure that my project is analyzer clean). If the lints live in a separately versioned package, upgrading to a new SDK is less breaking and I can migrate to the latest set of lints at my leisure by bumping the lint package version in my pubspec.

Managing the lints in the Dart SDK will presumably also make rolling new Dart SDK versions into Flutter more painful because newly introduced lints may break some of our customer tests (contributed via https://github.com/flutter/tests). Decoupling means that new SDK versions can roll in independently from customers upgrading to the latest lint sets.

If I understand the proposal right, you can still pin lints to a major version as there will be options files corresponding to each major version. Under such a scheme, when bumping to SDK 2.14 (say) you could pin yourself to the 2.13 lints by including a file like dart:core/lints/core_2_13.yaml or similar (rather than dart:core/lints/core.yaml which is always the latest) .

Correct: My goal is to have the optimal defaults, but still support easy customization should you not want these defaults. One such customization is to pin to a specific / older version.

Resolved. See: https://github.com/dart-lang/lints

Navigation Menu

Repository navigation

identify place to host canonical lints #58302

Proposal 1 : (new) package:canonical

Proposal 2: package:pedantic

11 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

identify place to host canonical lints #58302

Description

Proposal 1 : (new) package:canonical

Proposal 2: package:pedantic

Activity

natebosch commented on Jan 8, 2021

jakemac53 commented on Jan 8, 2021

jakemac53 commented on Jan 8, 2021

munificent commented on Jan 8, 2021

natebosch commented on Jan 8, 2021

natebosch commented on Jan 8, 2021

jakemac53 commented on Jan 8, 2021

NatoBoram commented on Jan 10, 2021

mit-mit commented on Jan 11, 2021

davidmorgan commented on Jan 11, 2021

lrhn commented on Jan 11, 2021

mit-mit commented on Jan 11, 2021

11 remaining items

goderbauer commented on Apr 6, 2021

goderbauer commented on Apr 6, 2021

mit-mit commented on Apr 6, 2021

bwilkerson commented on Apr 6, 2021

jakemac53 commented on Apr 6, 2021

devoncarew commented on Apr 6, 2021

natebosch commented on Apr 6, 2021

goderbauer commented on Apr 7, 2021

pq commented on Apr 7, 2021

mit-mit commented on Apr 8, 2021

pq commented on May 6, 2021

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions