Skip to content
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

Tools to generate your analysis_options text #1051

Closed
srawlins opened this issue Jun 29, 2018 · 18 comments
Closed

Tools to generate your analysis_options text #1051

srawlins opened this issue Jun 29, 2018 · 18 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@srawlins
Copy link
Member

srawlins commented Jun 29, 2018

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.

@srawlins srawlins added the type-enhancement A request for a change that isn't a bug label Jun 29, 2018
@pq
Copy link
Member

pq commented Jun 29, 2018

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
Copy link

zoechi commented Jun 29, 2018

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

  • unfixed bugs in linter rules or Dart
  • not matching my project style guide
  • not matching a specific project type (Angular, Server, Flutter, ...)

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
Copy link
Member

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:

  • Also keep track of the rules that were explicitly rejected so we don't ask about them again.
  • Provide a way to review and change previous decisions.

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 srawlins changed the title Tools to generate your pubspec text Tools to generate your analysis_options text Jun 29, 2018
@srawlins
Copy link
Member Author

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
Copy link
Member

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
Copy link
Contributor

a14n commented Jun 29, 2018

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
Copy link
Member Author

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
Copy link

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
Copy link
Member

pq commented Jan 31, 2019

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

@robbecker-wf
Copy link

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

@pq
Copy link
Member

pq commented Jan 31, 2019

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

Thanks!

@robbecker-wf
Copy link

@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
Copy link
Member

pq commented Feb 3, 2019

This looks amazing. Thank you @robbecker-wf!

fyi @srawlins and @a14n

@robbecker-wf
Copy link

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
Copy link
Member

pq commented Feb 4, 2019

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
Copy link
Member

pq commented Feb 5, 2019

FYI @danrubel re: using in dartfix.

@pq
Copy link
Member

pq commented Dec 15, 2020

/fyi @csells

@srawlins
Copy link
Member Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants