Skip to content

Dart format --fix requires two runs to reach fixpoint. #971

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

Closed
domesticmouse opened this issue Nov 3, 2020 · 9 comments
Closed

Dart format --fix requires two runs to reach fixpoint. #971

domesticmouse opened this issue Nov 3, 2020 · 9 comments

Comments

@domesticmouse
Copy link
Member

Hey Dart Style team,

On the dart-services project we generate code from protobuf definitions, then we need to run dartfmt --fix -w twice to get code stability. This changed between Dart 2.10.2 and 2.10.3. This behaviour also reproduces in Dart 2.11.0-213.4.beta and 2.12.0-0.0.dev.

To reproduce you need to install a couple of tools

Reproduction steps:

$ gh repo clone dart-lang/dart-services
Cloning into 'dart-services'...
remote: Enumerating objects: 17, done.
remote: Counting objects: 100% (17/17), done.
remote: Compressing objects: 100% (16/16), done.
remote: Total 6432 (delta 2), reused 1 (delta 1), pack-reused 6415
Receiving objects: 100% (6432/6432), 1.60 MiB | 1.49 MiB/s, done.
Resolving deltas: 100% (4479/4479), done.
$ cd dart-services
$ protoc --dart_out=lib/src protos/dart_services.proto
$ dartfmt --fix -w lib/src/protos
Formatting directory lib/src/protos:
Formatted dart_services.pb.dart
Formatted dart_services.pbenum.dart
Formatted dart_services.pbjson.dart
Formatted dart_services.pbserver.dart
$ dartfmt --fix -w lib/src/protos
Formatting directory lib/src/protos:
Formatted dart_services.pb.dart
Unchanged dart_services.pbenum.dart
Unchanged dart_services.pbjson.dart
Unchanged dart_services.pbserver.dart
@munificent
Copy link
Member

I tried to repro but, I'm getting:

protoc-gen-dart: program not found or is not executable
Please specify a program using absolute path or make sure the program is available in your PATH system variable
--dart_out: protoc-gen-dart: Plugin failed with status code 1.

Would you mind just attaching the unformatted dart_services.pb.dart file and I'll go from that?

@domesticmouse
Copy link
Member Author

Hey Bob,

Have a look at step_1.dart in https://gist.github.com/domesticmouse/5d195b849ba37c8f2135c90a1ec3ab40

Steps 2 and 3 are the dartfmt runs.

@natebosch natebosch changed the title Dart format requires two runs to reach fixpoint. Dart format --fix requires two runs to reach fixpoint. Mar 24, 2021
@natebosch
Copy link
Member

This only happens with --fix.

I feel like this may have come up in the past and is a known issue that --fix may not produce final result. @munificent do you recall that?

@munificent
Copy link
Member

This right here is the known issue. :) So, yes, it's known but undesirable. The architecture of how --fix is implemented right now is kind of hacky, which is what leads to this. For most common use cases the output code does end up with the correct formatting, so it's not a high priority. But I'd like to either make it always produce correctly formatted code, or just remove the feature entirely now that we have dart fix.

@domesticmouse
Copy link
Member Author

So I should replace dartfmt --fix with dart fix?

If you are going to drop, please add a warning now pushing people to dart fix.

@munificent
Copy link
Member

So I should replace dartfmt --fix with dart fix?

Those two commands currently do different things. The former can do a few entirely syntactic fix-ups. The latter is built on top of analyzer and can to more complex changes. I have a vague long-term goal to remove dartfmt --fix once dart fix can do all the fixes that dartfmt --fix can do, but no real schedule or anything. It just seems odd to have two very similar tools.

@domesticmouse
Copy link
Member Author

Thanks for the clarification @munificent

@munificent
Copy link
Member

I should say that in general dartfmt --fix is never intended to be run automatically like on CI or submit queues. That's why you have to pass the --fix argument explicitly. It is careful to only make safe changes, but the expectation is that a human will vet them. For things like automated formatting on CI, just use dartfmt.

@munificent
Copy link
Member

Closing this since we're removing --fix entirely: #1153.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants