Lint request: A single "set" of "effective Dart" lints

[matanlurey]

Forked from #32596 (comment).

AngularDart (and internally at Google) requires a better process for identifying, testing, and using lints than the one currently used right now (i.e. many varied contributions without requirements they conform with the style guide - often they are inventing style guide requirements that do not exist).

i.e. I'd like to be able to write (or similar):

# analysis_options.yaml
linter: google # Use the Google style guide set of lints.

Internally, we might need a transitional set of lints that are not quite all of them:

# analysis_options.yaml
linter: google-relaxed

Here are our requirements:

1. The internal style team determines what lints match both google and google-relaxed. It's likely most of these lints already exist, but we should double check the implementations. We'd be linting 10s of millions of lines of code, so we need the quality bar to the extremely high (internally lints with a false-positive rating of >=0.5% are automatically disabled).
2. Automated performance testing avoids issues around IDE crashes or slowdowns.
3. As lints are added to this list, we have adequate time to vet them on large code bases.
4. Lints that are optional (i.e. prefer) need to be excluded on existing code - i.e. we don't want to force teams to have to write hundreds of // ignore: x_x_x because they didn't follow an optional rule.

Nice to have externally (we do this internally already):

1. Lints can be enabled "just-in-time" - i.e. for presubmit/code review (instead of always on). This is probably a requirement for 4. above, anyway, because otherwise we wouldn't have context on whether code is "new" or not.

/cc @JekCharlsonYu., @davidmorgan (who is working on internal linting).

[Hixie]

FWIW, Flutter has a pair of "recommended" analysis options files, one for people who want an easy-go-lucky style (the default for flutter analyze), and one for people who want a super-strict style (the default for the Flutter repository itself). I could totally see the Effective Dart style guide having a recommended set of lints in the same way.

[matanlurey]

Definitely!

[zoechi]

related #57153

[avilladsen]

We've recently been trying to take analysis options more seriously at my workplace, and have run into the same kinds of issues. Turning on a new lint requires a lot of vetting to make sure that it works with our code base, and we've had to give up on several lints because of false positives or the sheer number of times we violate the lint in existing code. Sharing the lints we've vetted is also problematic, as pretty much all we can do is post a set of "standard" lints and tell people to check back for updates occasionally. In that light, I'm all for this request!

However, there's a couple things that worry me about google or google-relaxed, if they are (as I understand this request) being treated as special cases defined in the linter:

1. A single disagreement between Google and us on what the right lint rules are could render this convenience unusable for us, depending on how severe the disagreement is. We don't intend to develop a competing standard to effective Dart, but there will likely be some lints (probably prefer rules) that we decide we don't want to enforce. And there will definitely be lints that we want to adopt, but at our own pace rather than Google's.
2. How would the versions of google and google-relaxed be defined? If they are special-cased into the linter, I'd assume that they are tied into the current version of Dart. That makes dealing with new lint rules a potential barrier to upgrading Dart versions. Maybe more of a nuisance than a barrier, but it's still another thing to deal with in what you'd want to be as streamlined a process as possible.

Rather than adding a google special case, could the linter/analyzer allow "importing" configuration? E.g. something like ESLint's shareable configs. That would let us still benefit from sharing analysis options, without tying us in a potentially fragile way to a particular lint set that we have no control over. It could also be used for flutter or angular lint sets that build on top of the core google set, if that's a road you all want to go down.

[bwilkerson]

You can include one analysis options file in another by having a top level key of includes: whose value is the URI of the analysis_options.yaml file to be included.

I suspect that facility satisfies this issue, so I'm closing it.

[avilladsen]

Yes, that satisfies my use case, and works great in a quick experiment (though it's include:, not includes:). I don't know that it addresses the broader intent of the original request, however, as that was about not just how to implement such a set, but the methodology of maintaining it. I can't speak for @matanlurey, though.

Also, is the include: flag documented anywhere?

Edit: Specifically, I'd expect include: to be documented on the Customize Static Analysis page.

[bwilkerson]

I didn't find any documentation for it, no.

[natebosch]

I suspect that facility satisfies this issue, so I'm closing it.

Is the expectation that we should start recommending people write include: https://www.dartlang.org/tools/google_analysis_options.yaml in their own files and host our "canonical" set of lints online? Does the analyzer CLI and server work well with https URIs in includes?

[bwilkerson]

Is the expectation that we should start recommending people write ...

No. Neither the command-line analyzer nor the analysis server support https: URIs.

The expectation is that the Angular package could contain one or more standard options files just like the Flutter package does and that users can use a package: URI to reference them.

[natebosch]

makes sense, I don't think we need changes in the analyzer to support it in that case, but it's probably worth keeping this issue open as a request to ship such a file in the SDK. Angular Dart wouldn't own the "effective dart" lint set.

I'd also be fine moving this issue to dart-lang/linter as a request to ship the file there. It would not be a big issue, I think, to add a dependency on that package to pick up the canonical lint sets. Versioning would be easier if it were in the SDK though since we could validate that the lints are compatible with the version of the linter package in use, not the one in the pub solve.

[bwilkerson]

Ok, I've re-opened it.

... it's probably worth keeping this issue open as a request to ship such a file in the SDK.

Yeah, that would be harder to do. The only URIs for referencing things in the SDK are the dart: URI's, and they're special in a way that would make this kind of use hard.

I'd also be fine moving this issue to dart-lang/linter as a request to ship the file there.

I like that idea better than special casing some dart:lintSet/effectiveDart pattern. Users could add a dev dependency on linter in order to allow it to be resolved (although that could increase the risk of conflicting constraints). We could also publish a new "lint sets" package designed just for this purpose that wouldn't have any dependencies on anything else. It could still be a dev dependency, but wouldn't have the same danger of adding potentially conflicting constraints.

[bwilkerson]

There is now a "google.yaml" file defining the lints used at Google. I believe that should satisfy this request. Please re-open if there's something I'm missing.