uniffi-rs icon indicating copy to clipboard operation
uniffi-rs copied to clipboard

uniffi-bindgen 0.14.0 generates invalid Kotlin code for some errors

Open AlexDBlack opened this issue 2 years ago • 5 comments

For a Rust error type with associated data called TestError, the Kotlin bindings generator generates a sealed class TestError, yet functions throwing that error try to call it via rustCallWithError(TestException). The TestException type does does exist in the generated code.

Either the generated class should be called TestException instead of TestError, or the functions should be called using rustCallWithError(TestError).

Interface definition:

namespace test {
    [Throws=TestError]
    string foo();
};

[Enum]
interface TestError {
    FirstError(string err);
    SecondError(u32 value);
};

Rust code:

#[derive(Debug, thiserror::Error)]
pub enum TestError {
    #[error("First error")]
    FirstError{err: String},
    #[error("Second error")]
    SecondError{ value: u32 }
}

pub fn foo() -> Result<String, TestError> {
    unimplemented!()
}

Cargo.toml:

[package]
name = "kotlin_uniffi"
version = "0.1.0"
edition = "2018"

[dependencies]
uniffi = "0.14.0"
thiserror = "1.0.22"

[build-dependencies]
uniffi_build = "0.14.0"

Relevant parts of the generated code using uniffi-bindgen generate src/test.udl --language kotlin:

fun foo(): String {
    val _retval = 
    rustCallWithError(TestException) { status ->
    _UniFFILib.INSTANCE.test_8c63_foo(status)
}
    return String.lift(_retval)
}
@ExperimentalUnsignedTypes
sealed class TestError  {
    
    data class FirstError(
        val err: String 
        ) : TestError()
    
    data class SecondError(
        val value: UInt 
        ) : TestError()
    

Note TestError vs. TestException.

Full generated Kotlin code: https://gist.github.com/AlexDBlack/f700cd882ec23bb431addce51fb65d36

┆Issue is synchronized with this Jira Task ┆Issue Number: UNIFFI-94

AlexDBlack avatar Sep 30 '21 02:09 AlexDBlack

Thanks for the report - I'm not hugely familiar with Kotlin and haven't yet read this report in detail, but is it possible it's a dupe of #1020? For additional context, see also #442.

mhammond avatar Sep 30 '21 03:09 mhammond

It's similar to #1020 (is so far as both involve Kotlin codegen for errors) but I think it's a different issue. There, it looks like the problem is a clash between a generated class called Exception (created from Rust Error enum) conflicting with the built-in Kotlin Exception class of the same name.

Here, the problem isn't a clash with a built-in Kotlin class, but rather that the Error -> Exception renaming is occurring in one place (in the function call) but not the other (in the class definition). We need the Error/Exception renaming to occur in either both places, or neither place. For consistency, the correct fix may be to generate a TestException class instead of a TestError class in my example above.

AlexDBlack avatar Sep 30 '21 04:09 AlexDBlack

Two problems identified here appear:

  • inconsistently applied renaming of *Error to *Exception.
  • in the case of an error called Error, clashing with Kotlin's (or Java's) own implementation of Exception.

This looks like it could do with a failing test, some careful checking of the macros.kt, and some judicial use of a fully qualified class names (i.e. java.lang.Exception) to avoid collisions— not entirely convinced this will work.

jhugman avatar Oct 11 '21 13:10 jhugman

@mhammond should I put this failing test in fixtures/regressions do you think?

jhugman avatar Oct 11 '21 13:10 jhugman

If you change [Enum] annotation to [Error] one the problem will disappear (at least on version 0.23+).

Voronar avatar Nov 02 '23 15:11 Voronar