googlearchive / pedantic

How to get the most value from Dart static analysis
https://pub.dev/packages/pedantic
BSD 3-Clause "New" or "Revised" License
324 stars 56 forks source link

Provide an options file which includes `package:lints/recommended.yaml` #86

Closed srawlins closed 3 years ago

srawlins commented 3 years ago

Now that a Dart team-recommended set of lint rules has been published, I think it makes sense to think of pedantic as the layer on top of those rules which we value highly internally.

(I know that that is not the actual situation; the pedantic rules are just completely separate, but in terms of analysis options files for users, I think it would be pragmatic to think of pedantic this way.)

Only one analysis options file can be included into another; you cannot include a list of analysis options files. Perhaps you can some day, but there is some hashing out w.r.t. merging that needs doing, and it might not be a high priority item. In the meantime, the only way to include multiple analysis options files into your own is by daisy chaining them. package:lints/recommended.yaml includes package:lints/core.yaml, so you get the lint rules from each.

It would be great for pedantic to provide a new analysis options file which includes package:lints/recommended.yaml. Something like package:pedantic/recommended.yaml. WDYT?

pq commented 3 years ago

/fyi @davidmorgan @mit-mit

mit-mit commented 3 years ago

SGTM, but this is really David's call

davidmorgan commented 3 years ago

Hmmmm looking at the delta between pedantic and recommended, I'm not sure it's worth it; it doesn't seem like a lot of value.

I'd actually go in the other direction: is there any value left in publishing/using the google3 lint set externally? It gives a little more consistency for google3 developers working on third_party packages, but there is anyway a delay in publishing so they're not usually fully in sync. I'm not sure external users care too much. And, it's potentially confusing.

WDYT?

srawlins commented 3 years ago

The delta is only 6 removed lints: https://github.com/dart-lang/lints/issues/37

I originally thought that all pub packages imported into Google internal had to be pedantic-compliant, but forgot we have a different rule set for third party, so I think you're right; I'll just be migrating my own packages to package:lints.

davidmorgan commented 3 years ago

That sounds right.

The last question is the old chestnut of what we do with unawaited :)

At this point I think that will mostly follow how/if Flutter ends up using unawaited_futures, so nothing to do right now from here.

@pq do you plan to update the linter docs to add tags/links into package:lints? Per the discussion here I think it makes sense to remove pedantic links at the same time. WDYT please?

Thanks.

bwilkerson commented 3 years ago

The last question is the old chestnut of what we do with unawaited.

I don't think we need to do anything with it. It was created for internal use because we didn't want internal users to use ignore comments, but Flutter doesn't have a policy against using ignore comments, so they already have a valid way of disabling the lint when necessary.

pq commented 3 years ago

@pq do you plan to update the linter docs to add tags/links into package:lints? Per the discussion here I think it makes sense to remove pedantic links at the same time. WDYT please?

Tags will be updated on next release.

Since pedantic is still used by googlers who might value these docs are you sure we want to remove the badge?

pq commented 3 years ago

I'd actually go in the other direction: is there any value left in publishing/using the google3 lint set externally?

Sorry, I was reading backwards...

If you do decide to remove the lints or stop updating/maintaining them, I'd suggest a deprecation period and some good messaging but you've probably already thought of that! :)

srawlins commented 3 years ago

Note that unawaited_futures specifically does call out using package:pedantic:

Future results in async function bodies must be awaited or marked unawaited using package:pedantic.

davidmorgan commented 3 years ago

I just sent @pq a CL explicitly setting the lint message in google3, then we can drop the reference to pedantic externally.

For pedantic, I suggest: I'll update the README to point to package:lints then stop publishing new versions.

mit-mit commented 3 years ago

Is it not better to wait a bit, and then see if these actually do continue to be used?

bwilkerson commented 3 years ago

It appears that a getter named unawaited is being added to the SDK (as an extension): https://dart-review.googlesource.com/c/sdk/+/200428. We'll need to update the lint to recognize it, then we can decide whether to deprecate the method in pedantic in favor of the getter.

davidmorgan commented 3 years ago

The lint should just work :) it doesn't do anything specific to the current unawaited method, either.

Then, I agree, pointing in the lint message to the SDK-based solution seems right.