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

[Feature request] fix by issue type #47219

Closed
sm2017 opened this issue Sep 15, 2021 · 15 comments
Closed

[Feature request] fix by issue type #47219

sm2017 opened this issue Sep 15, 2021 · 15 comments
Assignees
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-fix P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug

Comments

@sm2017
Copy link

sm2017 commented Sep 15, 2021

Unfortunately dart fix as only 2 commands as of now

dart fix --dry-run //To preview changes

and

dart fix --apply //To apply the changes

I need a way to tell dart to fix only a specific issue? i.e only fix all Prefer const with constant constructors.

@lrhn lrhn added the area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. label Sep 15, 2021
@bkonyi
Copy link
Contributor

bkonyi commented Sep 15, 2021

cc @pq

@pq pq added P2 A bug or feature request we're likely to work on type-enhancement A request for a change that isn't a bug labels Sep 15, 2021
@srawlins
Copy link
Member

Here's my use case:

Let's say I contribute to a medium or big package is using lints 1.0.0, or even pedantic, and I want to upgrade to lints 2.0.0. After changing the include directive to use lints 2.0.0, I see 2000 new diagnostics in my IDE. Yikes! I'd like to fix those, but I don't want to mail out a change to my team with 2000 fixes, each of which fixes one of eight-ish new lint rules.

It would be much easier for my team to review a 500-fix change that only fixes one thing, then a 200-fix change that only fixes one other thing. Single-purpose CLs.

What is my workflow to produces these changes? Today, its pretty manual: find the name of one lint rule I wish to fix, comment out the include directive in analysis options, then add the lint rule at the bottom. Then run dart fix. Mail change (not including analysis_options.yaml!), remove the one lint rule, un-comment the include directive, re-analyze. Repeat.

It would be very nice if instead, I could specify a diagnostic to fix. Something like --rule=foo. It doesn't even have to be a list, or support inversions (maybe someday we'll get to dart fix --rules=include:lints/recommended.yaml,-this_rule,-that_rule,+oh_but_add_this_one, but I'm not asking for that 😁 ).

@stereotype441
Copy link
Member

This would also be helpful for me when preparing internal large-scale bulk changes - part of the review process involves specifying the process you used to produce the bulk change. If the reviewer sees --rule=foo in the command line, it'll give them a lot more confidence that no unintentional changes snuck in.

@bwilkerson
Copy link
Member

... maybe someday we'll get to dart fix --rules=include:lints/recommended.yaml,-this_rule,-that_rule,+oh_but_add_this_one

I really hope not.

Something like --rule=foo.

My concern / question is how usable it is to have an interface that requires users to type the names of diagnostic codes. If we're only concerned with lints it might not be too bad because they've likely already typed the name in the analysis options file. Unfortunately, I can't think of anything other than the names that makes any sense.

Perhaps the output from --dry-run could be redone a bit to at least make it easier to copy / paste the names.

Minor nit, but I'd prefer something less lint-specific than rule. diagnostic is probably too long, but maybe code? That's not great either.

The only alternative I can think of is to allow the user to specify the analysis options file on the command-line and have the user create copies. That's probably worse than what we have today.

@pq
Copy link
Member

pq commented Mar 18, 2022

Minor nit, but I'd prefer something less lint-specific than rule. diagnostic is probably too long, but maybe code?

+1.

Perhaps the output from --dry-run could be redone a bit to at least make it easier to copy / paste the names.

Agreed, 💯 .

@srawlins
Copy link
Member

I like the idea for --dry-run, maybe a new stats output like

foo_rule: 10 fixes
bar_rule: 12 fixes

(--dry-run by itself prints individual file, rule, fix information, right?)

@bwilkerson
Copy link
Member

Both the --apply and --dry-run modes print the same information after a slightly different header:

file/path/one.dart:
  diagnostic_code1 * count1
  diagnostic_code2 * count2
file/path/two.dart:
  ...

It's easy to see which files would be changed and what would be done in each, but hard to see all of the diagnostics that might potentially want to be fixed.

If we really want to make it easy to fix a single diagnostic, we might even consider something like

To fix an individual diagnostic, run one of the following commands:
  path/to/dart fix --apply --code=diagnostic_code1 path
  path/to/dart fix --apply --code=diagnostic_code2 path

@pq
Copy link
Member

pq commented Mar 18, 2022

If we really want to make it easy to fix a single diagnostic, ...

I was thinking something similar. 👍

@ipcjs
Copy link

ipcjs commented May 17, 2022

@sm2017
I wrote a command line tool that might be useful to you: dart-fix-rules

@pq
Copy link
Member

pq commented May 17, 2022

/fyi @bwilkerson

@pq pq added P1 A high priority bug; for example, a single project is unusable or has many test failures and removed P2 A bug or feature request we're likely to work on labels Aug 15, 2022
@mit-mit
Copy link
Member

mit-mit commented Aug 16, 2022

Can we make is so that dart fix --apply --code=diagnostic_code1 path works regardless of whether there is an analysis config or not; and if there is, whether that contains diagnostic_code1 or not?

@srawlins
Copy link
Member

That's probably possible.

copybara-service bot pushed a commit that referenced this issue Aug 18, 2022
See: #47219

I'll follow up with tests for error conditions once diagnostic codes are validated by the protocol (see: https://dart-review.googlesource.com/c/sdk/+/255541).

Change-Id: I3318364e952522522608e86bdfdd231839685689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/255544
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
copybara-service bot pushed a commit that referenced this issue Aug 25, 2022
Adds details to make applying individual fixes easier.

Example output:

```
3 proposed fixes in 1 file.

foo/baz/main.dart
  annotate_overrides • 1 fix
  prefer_single_quotes • 2 fixes

To fix an individual diagnostic, run one of the following commands:
  dart fix --apply --code=no_duplicate_case_values .
  dart fix --apply --code=unused_imports .

To fix all diagnostics, run:
  dart fix --apply .
```

See: #47219


Change-Id: Ibc89388acabe868fe3253d802e03481d853646ac
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/256367
Commit-Queue: Phil Quitslund <pquitslund@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
@pq
Copy link
Member

pq commented Aug 25, 2022

With 1519292, marking the core of this feature done. Let's follow-up with new issues for further enhancements (such as #49815).

Thanks for the input all. Your continued feedback is welcome and appreciated!

@pq pq closed this as completed Aug 25, 2022
@FMorschel
Copy link
Contributor

FMorschel commented Aug 30, 2022

How can I stay updated about when this releases on stable?

P.S.: Sorry about the level of this question, I'm just beginning to be part of the community actively.

@srawlins
Copy link
Member

This is definitely in Dart 2.19.0. And I think it should be in the recent Dart 2.19.0 dev releases (2.19.0-146.0.dev ish).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. dart-cli-fix P1 A high priority bug; for example, a single project is unusable or has many test failures type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

10 participants