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

v2.4.0 runs dart fix thus modifies codes not generated by frb #2316

Open
lbhfuture opened this issue Sep 26, 2024 · 11 comments
Open

v2.4.0 runs dart fix thus modifies codes not generated by frb #2316

lbhfuture opened this issue Sep 26, 2024 · 11 comments
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working

Comments

@lbhfuture
Copy link

Describe the bug

I run flutter_rust_bridge_codegen integrate to add frb to my existed project, and it modified my codes which is not generated by frb.

The console shows:

[2024-09-26T06:14:49.361Z INFO .../flutter_rust_bridge_codegen-2.4.0/src/library/integration/integrator.rs:79] Apply Dart fixes
[2024-09-26T06:15:00.092Z INFO .../flutter_rust_bridge_codegen-2.4.0/src/library/integration/integrator.rs:82] Format Dart code

The formatter above should only modify dart code in lib/src/rust, but it seems not works well, and my flutter_rust_bridge.yaml is below:

rust_input: crate::api
rust_root: rust/
dart_output: lib/src/rust
enable_lifetime: true

frb version is 2.4.0

Steps to reproduce

  1. Run flutter_rust_bridge_codegen integrate to add frb to a existed project.

Logs

[2024-09-26T06:14:49.361Z INFO .../flutter_rust_bridge_codegen-2.4.0/src/library/integration/integrator.rs:79] Apply Dart fixes
[2024-09-26T06:15:00.092Z INFO .../flutter_rust_bridge_codegen-2.4.0/src/library/integration/integrator.rs:82] Format Dart code

Expected behavior

No response

Generated binding code

No response

OS

MacOS

Version of flutter_rust_bridge_codegen

2.4.0

Flutter info

No response

Version of clang++

No response

Additional context

No response

@lbhfuture lbhfuture added the bug Something isn't working label Sep 26, 2024
Copy link

welcome bot commented Sep 26, 2024

Hi! Thanks for opening your first issue here! 😄

@fzyzcjy fzyzcjy changed the title Seems a major damage bug, it modified codes which is not generated by frb Seems a major damage bug, it modified codes which is not generated by frb, because it runs dart fix Sep 26, 2024
@fzyzcjy fzyzcjy added the awaiting Waiting for responses, PR, further discussions, upstream release, etc label Sep 26, 2024
@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 26, 2024

  1. Firstly, you can go back to older 2.3.0 version without the "auto run dart fix" feature, before this is handled
  2. No worries, it will not harm your code by modifying arbitrarily; instead, it only runs dart fix
  3. cc @AlexV525 who introduced the "auto run dart fix" feature (which is quite useful in some scenarios) - maybe we should at least make a bool flag for this feature?

@fzyzcjy fzyzcjy changed the title Seems a major damage bug, it modified codes which is not generated by frb, because it runs dart fix v2.4.0 runs dart fix thus modifies codes not generated by frb Sep 26, 2024
@AlexV525
Copy link
Contributor

The tricky part is that dart fix does not support for fixing a single file. We can support opt-out of the feature first, then consider further support with handling a specific batch of files.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 26, 2024

Totally agree!

@AlexV525
Copy link
Contributor

Oh, I just ran dart fix --apply ./dir_name and it works.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 26, 2024

I tried it a bit:

dart fix --apply ./lib/main.dart ./lib/src/rust/frb_generated.dart 
Only one file or directory is expected.

Usage: dart fix [arguments]
-h, --help                      Print this usage information.
-n, --dry-run                   Preview the proposed changes but make no changes.
    --apply                     Apply the proposed changes.
    --code=<code1,code2,...>    Apply fixes for one (or more) diagnostic codes.

Run "dart help" to see global options

But we should not run dart fix one time for each file/folder, otherwise it may be slow... (not tested though; if running multiple ones does not have overhead, then this looks pretty reasonable)

Maybe we can also create/find an issue at Dart repo asking for supporting multiple file/folders

@AlexV525
Copy link
Contributor

We can only apply to dart_output, don't we?

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 26, 2024

Yes and no. Yes because usually that folder should only be touched by frb and not but normal users; no because theoretically we did not forbid users from adding any extra files there...

So I guess a workaround is to only apply to that folder, and if there rare latter case happens, we can tell users to move their files or to accept the fix.

@AlexV525
Copy link
Contributor

no because theoretically we did not forbid users from adding any extra files there...

Fair enough. We can improve our guides and warnings to tell users not to put extra files into the output path as the best practice.

@fzyzcjy
Copy link
Owner

fzyzcjy commented Sep 28, 2024

Related: #2328

(manually add this comment since github does not auto backlink between discussion and issues)

@Ferry-200

This comment was marked as duplicate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting Waiting for responses, PR, further discussions, upstream release, etc bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants