dart-lang / linter

Linter for Dart.
https://dart.dev/tools/linter-rules
BSD 3-Clause "New" or "Revised" License
636 stars 170 forks source link

Tools to generate your analysis_options text #1051

Closed srawlins closed 2 years ago

srawlins commented 6 years ago

I think it would be crazy slick, and not super hard, to write an interactive web tool, or a cmdline tool, that walks a user through each available lint, showing the BAD and the GOOD examples, and asking, "Is this something you want to enforce?", and giving you a linter: section for your analysis_options.yaml.

I partly came to this idea because I was wondering how a team/person is supposed to update their list of lint warnings, once they've used the tool for a few months, and 15 more lint rules have been added to the functionality. The tool could start with "what rules do you have enabled today?" to present you with the ones you're missing.

pq commented 6 years ago

I love this idea.

your pubspec.yaml.

I think you mean analysis_options.yaml?

In practice, I think this would be greatly improved by some default sets a la #288. Answering 100+ questions would be a drag!

zoechi commented 6 years ago

I'm thinking about a config file for a mono-repo where I define one or more sets of rules I want to use

Some exceptions because of

And then have an update check that checks if new rules became available since the last check This check would allow me to interactively accept or exclude with a reason (project, project style guide) and then update an analysis_options.linter.yaml in each project and also the config file that remembers which rule I excluded for what reason.

I would split it out in analysis_options.linter.yaml and then import this file in analysis_options.yaml for easier handling and avoiding issues with updating manually edited files.

As far as I remember the analyzer looks upwards for analysis_options.yaml files. If this is the case then for a mono-repo there could be a general analysis_options.linter.yaml with rules used for all sub-projects in the repo root and in each project only the rules that are not shared with the other sub-projects.

bwilkerson commented 6 years ago

Not to discourage the original definition, but there are several of enhancements I can see people quickly asking for. (I was typing this when @zoechi's comment came in, just to prove my point :-) A couple that come to mind are:

Eventually, I think this turns into the equivalent of a settings dialog, or even more.

But @pq is right. We know from past experience that that as the number of lint rules increases managing them becomes harder and harder. Settings dialogs don't really help users manage the complexity. I don't know whether rule sets really help that much (they're equivalent to "categories" or "groups" in a dialog, which didn't help as much as we'd hoped), but we need to find a better way to support configuration.

srawlins commented 6 years ago

Also keep track of the rules that were explicitly rejected so we don't ask about them again.

Yeah I definitely want this too; I've been scratching my head over how it would work and not be a PITB for the user. Would many people hate checked-in analysis options like this:

linter:
  - foo
  - bar

  # Rejected; do not add without team discussion
  # - baz # does not fit our style
  # - quux # linter#123 bug
bwilkerson commented 6 years ago

I wouldn't, but I don't know about other people.

But I'll add another ask: please sort the rule names; it makes it much easier to visually search for rules.

a14n commented 6 years ago

See Flutter analysis_options.yaml it has a lot of comments:

linter:
  rules:
    # these rules are documented on and in the same order as
    # the Dart Lint rules page to make maintenance easier
    # https://github.com/dart-lang/linter/blob/master/example/all.yaml
    - always_declare_return_types
    - always_put_control_body_on_new_line
    # - always_put_required_named_parameters_first # we prefer having parameters in the same order as fields https://github.com/flutter/flutter/issues/10219
    - always_require_non_null_named_parameters
    - always_specify_types
    - annotate_overrides
    # - avoid_annotating_with_dynamic # conflicts with always_specify_types
    - avoid_as
    # - avoid_bool_literals_in_conditional_expressions # not yet tested
    # - avoid_catches_without_on_clauses # we do this commonly
    # - avoid_catching_errors # we do this commonly
    - avoid_classes_with_only_static_members

An other point is that some lints conflict each other.

srawlins commented 5 years ago

I personally think such a tool would be blocked on clear justifications for each lint, so that the user can make an informed decision from within such a tool. #1391

robbecker-wf commented 5 years ago

FWIW I wrote a tool that does exactly this for use inside Workiva. If there is interest I could see about open sourcing it.

pq commented 5 years ago

@robbecker-wf: that sounds really interesting. I'd love to know more!

robbecker-wf commented 5 years ago

@pq Open Sourcing in progress .. feel free to email me if you want to discuss sooner.

pq commented 5 years ago

Awesome! I look forward to seeing it. If I get impatient, I'll mail you.

Thanks!

robbecker-wf commented 5 years ago

@pq Recently opened sourced. https://github.com/Workiva/abide It's still specific to our needs at this point. I'd like to have it be more configurable. Give it a look at see if we can shape it to fill this need in the Dart community.

pq commented 5 years ago

This looks amazing. Thank you @robbecker-wf!

fyi @srawlins and @a14n

robbecker-wf commented 5 years ago

Thanks! I filed some initial issues to generalize. Feel free to file issues or put up PRs to adjust it as needed. We actually run this on projects and report it to a dashboard. I think it would be pretty sweet to include the abide score and data on the pub page for each package.

pq commented 5 years ago

I think it would be pretty sweet to include the abide score and data on the pub page for each package.

Food for thought: @isoos @jonasfj

pq commented 5 years ago

FYI @danrubel re: using in dartfix.

pq commented 3 years ago

/fyi @csells

srawlins commented 2 years ago

I'm going to close out this old request as we have package:lints and package:flutter_lints and I think there's not much need for this tool any longer. Someone can re-open if they feel strongly we still need this.