fzyzcjy / flutter_rust_bridge

Flutter/Dart <-> Rust binding generator, feature-rich, but seamless and simple.
https://fzyzcjy.github.io/flutter_rust_bridge/
MIT License
3.61k stars 254 forks source link

`flutter_rust_bridge_codegen generate` generates incorrect code for enums in some rare/unique cases #1871

Closed NightBlaze closed 1 month ago

NightBlaze commented 1 month ago

Describe the bug

This issue is related to https://github.com/fzyzcjy/flutter_rust_bridge/issues/1870.

Because I need functions for enum and impl section is ignored by frb generator I decided to create a public function that accepts enum:

#[flutter_rust_bridge::frb(sync)]
pub fn enum_foo(p: &MyEnum) -> String {
    format!("{:?}", p)
}

Also the enum uses in a struct

#[derive(Debug)]
pub struct MyStruct {
    pub field: MyEnum
}

impl MyStruct {
    #[flutter_rust_bridge::frb(sync)]
    pub fn foo(&self) -> String {
        format!("{:?}", self)
    }
}

This combination leads to incorrect Dart code

// This file is automatically generated, so please do not edit it.
// Generated by `flutter_rust_bridge`@ 2.0.0-dev.30.

// ignore_for_file: invalid_use_of_internal_member, unused_import, unnecessary_import

import '../frb_generated.dart';
import 'package:flutter_rust_bridge/flutter_rust_bridge_for_generated.dart';

String greet({required String name, dynamic hint}) => RustLib.instance.api.greet(name: name, hint: hint);

String enumFoo({required MyEnum p, dynamic hint}) => RustLib.instance.api.enumFoo(p: p, hint: hint);

// Rust type: RustOpaqueMoi<flutter_rust_bridge::for_generated::rust_async::RwLock<MyEnum>>
@sealed
class MyEnum extends RustOpaque {
  MyEnum.dcoDecode(List<dynamic> wire) : super.dcoDecode(wire, _kStaticData);

  MyEnum.sseDecode(int ptr, int externalSizeOnNative) : super.sseDecode(ptr, externalSizeOnNative, _kStaticData);

  static final _kStaticData = RustArcStaticData(
    rustArcIncrementStrongCount: RustLib.instance.api.rust_arc_increment_strong_count_MyEnum,
    rustArcDecrementStrongCount: RustLib.instance.api.rust_arc_decrement_strong_count_MyEnum,
    rustArcDecrementStrongCountPtr: RustLib.instance.api.rust_arc_decrement_strong_count_MyEnumPtr,
  );
}

enum MyEnum {
  abc,
}

class MyStruct {
  final MyEnum field;

  const MyStruct({
    required this.field,
  });

  String foo({dynamic hint}) => RustLib.instance.api.myStructFoo(
        that: this,
        hint: hint,
      );

  @override
  int get hashCode => field.hashCode;

  @override
  bool operator ==(Object other) =>
      identical(this, other) || other is MyStruct && runtimeType == other.runtimeType && field == other.field;
}

The error is The name 'MyEnum' is already defined. because there are enum MyEnum and class MyEnum.

There are also errors in frb_generated.dart. All the errors in the file are related to encoding/decoding like The method 'dcoDecode' isn't defined for the type 'MyEnum'.

Steps to reproduce

  1. Create a fresh project flutter_rust_bridge_codegen create frb_enum
  2. Add the next snippet to simple.rs
    
    #[derive(Debug)]
    pub enum MyEnum {
    Abc
    }

[flutter_rust_bridge::frb(sync)]

pub fn enum_foo(p: &MyEnum) -> String { format!("{:?}", p) }

[derive(Debug)]

pub struct MyStruct { pub field: MyEnum }

impl MyStruct {

[flutter_rust_bridge::frb(sync)]

pub fn foo(&self) -> String {
    format!("{:?}", self)
}

}


3. Run `flutter_rust_bridge_codegen generate`

### Logs

```shell
-

Expected behavior

No response

Generated binding code

No response

OS

No response

Version of flutter_rust_bridge_codegen

2.0.0-dev.30

Flutter info

[✓] Flutter (Channel stable, 3.19.4, on macOS 14.4.1 23E224 darwin-x64, locale ru-RU)
    • Flutter version 3.19.4 on channel stable at /Users/user/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 68bfaea224 (3 weeks ago), 2024-03-20 15:36:31 -0700
    • Engine revision a5c24f538d
    • Dart version 3.3.2
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/user/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/user/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
    • Xcode at /Applications/Xcode.app/Contents/Developer
    • Build 15E204a
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • Flutter plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
      🔨 https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)

[✓] VS Code (version 1.88.0)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.86.0

[✓] Connected device (3 available)
    • iPhone (mobile)             • 00008101-001275A802DB001E • ios            • iOS 17.3.1 21D61
    • macOS (desktop)             • macos                     • darwin-x64     • macOS 14.4.1 23E224 darwin-x64
    • Chrome (web)                • chrome                    • web-javascript • Google Chrome 123.0.6312.107

[✓] Network resources
    • All expected network resources are available.

• No issues found!

Version of clang++

No response

Additional context

No response

fzyzcjy commented 1 month ago

Firstly, does pub fn enum_foo(p: MyEnum) -> String { instead of pub fn enum_foo(p: &MyEnum) -> String { work? I suspect it is because the &MyEnum automatically generates an opaque object.

NightBlaze commented 1 month ago

Firstly, does pub fn enum_foo(p: MyEnum) -> String { instead of pub fn enum_foo(p: &MyEnum) -> String { work? I suspect it is because the &MyEnum automatically generates an opaque object.

Yep, it works. But there is another issue. If MyStruct will have a func with self (not &self, &mut self) then it'll be an error on generation phase because MyEnum became non opaque.

-> % flutter_rust_bridge_codegen generate
[12.5s] Parse  
  └── [12.2s] Run cargo expand  
  └── [0.2s] Parse source graph
Error: function=Ident { sym: consume, span: bytes(1513..1520) }

Caused by:
    If you want to use `self`/`&mut self`, please make the struct opaque (by adding `#[frb(opaque)]` on the struct).

It will force to make MyStruct opaque too and it can break architecture.

The snippet that gives the error on generate phase

#[derive(Debug)]
pub enum MyEnum {
    Abc
}

#[flutter_rust_bridge::frb(sync)]
pub fn enum_foo(p: MyEnum) -> String {
    format!("{:?}", p)
}

#[derive(Debug)]
pub struct MyStruct {
    pub field: MyEnum
}

impl MyStruct {
    #[flutter_rust_bridge::frb(sync)]
    pub fn foo(&self) -> String {
        format!("{:?}", self)
    }

    #[flutter_rust_bridge::frb(sync)]
    pub fn consume(self) {
    }
}
fzyzcjy commented 1 month ago

If MyStruct will have a func with self (not &self, &mut self) then it'll be an error on generation phase because MyEnum became non opaque.

That's true, only &self is supported on non-opaque currently (though I do agree self may make sense!).

So what is the desired code that you want to write but fails? (For that snippet, I guess it may work if changing consume(self) into &self)

NightBlaze commented 1 month ago

So what is the desired code that you want to write but fails? (For that snippet, I guess it may work if changing consume(self) into &self)

After changing &MyEnum to MyEnum all my code works. But I can image a situation when a developer will need a struct with MyEnum field and a function with self on the struct.

So it's nice to have the issue to be fixed but it's not high priority because there are workarounds (using MyEnum, don't use self or explicitly set a struct as opaque) even if these workarounds will make a code not such clear and beautiful as it can be.

fzyzcjy commented 1 month ago

But I can image a situation when a developer will need a struct with MyEnum field and a function with self on the struct.

Totally agree. I am a bit hesitate about the design choice that, when should a struct be automatically judged as opaque, and when for non-opaque?

The drawbacks may include:

However, I guess the proposed change should be good, since we already have owned-type (the MyEnum) and ref-type (the &self) scenarios.

fzyzcjy commented 1 month ago

Firstly, https://github.com/fzyzcjy/flutter_rust_bridge/pull/1876 for better error hint

github-actions[bot] commented 2 weeks ago

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new issue.