Skip to content

explore ways to improve awareness around newly added rules #57885

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pq opened this issue Jan 24, 2019 · 10 comments
Open

explore ways to improve awareness around newly added rules #57885

pq opened this issue Jan 24, 2019 · 10 comments
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). devexp-linter Issues with the analyzer's support for the linter package P2 A bug or feature request we're likely to work on type-documentation A request to add or improve documentation type-enhancement A request for a change that isn't a bug

Comments

@pq
Copy link
Member

pq commented Jan 24, 2019

In #57873, @Hixie writes:

As a user (and not with my Flutter hat on), I'd love to have a way to turn on all possible lints, and require that I opt-out of the ones I don't want. This is because I frequently find that some super-useful lint has existed for months without my knowing.

Since this indicates I think a more general problem, I'm breaking it out for a separate discussion.

A few quick thoughts.

  • Dart SDK changelogs should include new lints; but I expect few people read them (or are even aware of what changes correspond to a given Flutter SDK (one thought is this could be a more prominent part of the Plugin Update experience (see also: Use the "What's new in Android Studio" panel for release notes flutter/flutter-intellij#2708)
  • IDEs could notice SDK changes and notify folks (e.g., via notifications/banners with relevant info)
  • Back to SDK updates, I wonder if flutter upgrade could include a link to relevant SDK changes in its output?

/cc @kwalrath @chalin @mit-mit for thoughts.

@pq pq added type-documentation A request to add or improve documentation area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). labels Jan 24, 2019
@a14n
Copy link
Contributor

a14n commented Jan 24, 2019

Like pedantic does with its analysis_options.yaml we could expose the linter all.yaml. Users could then use it inside their analysis_options.yaml:

include: package:linter/all.yaml

So every SDK bump will pull the full list of lints. User will be able to disable unwanted lints with ignore:

include: package:linter/all.yaml
analyzer:
  errors:
    implementation_imports: ignore

@chalin
Copy link
Contributor

chalin commented Jan 24, 2019

I'd love to have a way to turn on all possible lints, and require that I opt-out of the ones I don't want.

I've felt that need too. I like what Alexandre is proposing!

@srawlins
Copy link
Member

Also related, a tool to generate your analysis_options.yaml, #57730.

I can imagine a directive in analysis_options.yaml (even a comment):

# synced with linter 1.2.3

And then pub run bump-lint-rules says "Hmm what has been introduced between linter 1.2.3 and now? I'll ask the user if they want new_rule_a, new_rule_b, new_rule_c..."

@Hixie
Copy link
Contributor

Hixie commented Jan 24, 2019

  • Dart SDK changelogs

I'm sometimes involved in writing these and I don't read them. I definitely don't think most of our users read them.

  • IDEs could notice SDK changes and notify folks (e.g., via notifications/banners with relevant info)

This would help some people. Personally I don't use an IDE that has any knowledge of Dart or Flutter.

  • Back to SDK updates, I wonder if flutter upgrade could include a link to relevant SDK changes in its output?

I don't use flutter upgrade either (I just track the master branch with git). It also wouldn't catch cases where I switch branches (which I do a lot) or where I switch from one project to another (flutter upgrade would only know of the project I'm in when I upgrade).

I like @a14n's idea, though. That would work.

@bwilkerson
Copy link
Member

Like pedantic does with its analysis_options.yaml we could expose the linter all.yaml.

I'm not super thrilled with the prospect of having other packages depend on the linter package, primarily because the linter package depends on the analyzer package, which would pull in a lot of other unwanted dependencies. If we really want to make it possible to include all.yaml, then I propose we create a separate package for it.

But this did raise a question: Is there a way for users of a published lint set to know which version of the SDK they need in order to have all of those lints enabled?

The linter, for example, regularly contains lints in it's set that have not yet been shipped as part of a stable, or even dev, release of the SDK. Enabling those rules directly (in the package's own analysis options file) will display an error indicating that the rule isn't known; but enabling those rules indirectly (via an include) doesn't currently provide any feedback. Just saying that we might need some additional tooling to make this work better.

@pq
Copy link
Member Author

pq commented Jan 24, 2019

Is there a way for users of a published lint set to know which version of the SDK they need in order to have all of those lints enabled?

At the moment folks would have to look at the generated docs for each lint and keep a running tally of "bottom".

image

Not ideal and needless to say:

we might need some additional tooling to make this work better.

👍 Ideas welcome!

Question: if folks were to specify that they want "all the lints", would they mean all the lints for the current SDK, a specific explicitly listed SDK, for a constrained set of SDKs or something else?

@bwilkerson
Copy link
Member

Exactly. The current suggestion is "all the lints that will be supported in a future release", which might include rules that are not available in the current SDK and might be missing some that currently exist but are going to be removed (such as lints that won't apply to a future version of the language but might well apply to the version the user is using).

Perhaps a better solution would be to support an "all" option directly (rather that via including some hard-coded list). I don't know the best syntax, but something like

linter:
  rules:
    allExcept:
      - ...

Ideas welcome!

I was thinking of something like a warning of the form "The included analysis options enables the lint rule 'avoid_using_foo', which is not defined." Not great, because it isn't very actionable, but at least it makes the problem visible.

@a14n
Copy link
Contributor

a14n commented Jan 24, 2019

Like pedantic does with its analysis_options.yaml we could expose the linter all.yaml.

I'm not super thrilled with the prospect of having other packages depend on the linter package

In my mind this could have a specific handling directly in the SDK and will not require a dependency on linter. Actually linter is already special case because the SDK packages it.

It would be even more awesome if the version of linter in the SDK could be overriden by the version in pubspec.yaml. This would allow to use a more recent linter(with new lints/fixes...).

And the day linter becomes a plugin (unashamed plug 😄 ) no change should be required to analysis_options.yaml because the include uri will remain the same.

primarily because the linter package depends on the analyzer package, which would pull in a lot of other unwanted dependencies.

Thinking about it shouldn't analyzer plugins dependencies be declared in a new special section of pubspec.yaml (or inside analysis_options.yaml) to avoid dependency conflicts ?

@bwilkerson
Copy link
Member

Thinking about it shouldn't analyzer plugins dependencies be declared in a new special section of pubspec.yaml (or inside analysis_options.yaml) to avoid dependency conflicts?

There isn't any need. The plugin architecture was carefully designed so that the dependencies of the plugin don't effect the dependencies of the package associated with the plugin.

But the resolution of the package: URIs in the analysis options file does require that the target package is in the dependencies of the package containing the options file, so there's a very real danger of pulling in too many packages.

@Hixie
Copy link
Contributor

Hixie commented Jan 25, 2019

allExcept (or any kind of all that turns everything on with a separate (ideally existing) way to turn things off) is definitely the version that I think is most likely to reliably work, FWIW. No way for someone to forget to update the package, or for version numbers to get out of sync, in that case.

@pq pq added the type-enhancement A request for a change that isn't a bug label Apr 29, 2019
@srawlins srawlins added the P2 A bug or feature request we're likely to work on label Sep 23, 2022
@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 18, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 18, 2024
@bwilkerson bwilkerson added area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. and removed legacy-area-analyzer Use area-devexp instead. labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-devexp For issues related to the analysis server, IDE support, linter, `dart fix`, and diagnostic messages. area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). devexp-linter Issues with the analyzer's support for the linter package P2 A bug or feature request we're likely to work on type-documentation A request to add or improve documentation type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

7 participants