Error type named `Error` emits invalid Swift code

[arg0d]

This might be a duplicate issue, but I didn't manage to find another issue like this.

For generated Swift code, Error is ambiguous with "system" Error type.

Version: v0.27.1 (I'm guessing older versions too

Error is a very natural type name when starting a new library, IMHO the generators should be fixed to support Error as error type name. We've already had 2 different teams working on separate projects step onto this same issue while onboarding their projects to uniffi :sweat_smile:

There is a very simillar issue in C# bindings generators, https://github.com/NordSecurity/uniffi-bindgen-cs/issues/76.

diff --git a/fixtures/coverall/src/lib.rs b/fixtures/coverall/src/lib.rs
index f22ab5bc4..9d557ac72 100644
--- a/fixtures/coverall/src/lib.rs
+++ b/fixtures/coverall/src/lib.rs
@@ -23,6 +23,12 @@ pub enum CoverallError {
     TooManyHoles,
 }

+#[derive(Debug, thiserror::Error, uniffi::Error)]
+pub enum Error {
+    #[error("out of storage")]
+    DiskProblem {},
+}
+
 #[derive(Debug, thiserror::Error)]
 pub enum CoverallFlatError {
     #[error("Too many variants: {num}")]

/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:2778:1: error: inheritance from non-protocol type 'Error'
extension ComplexError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:2844:1: error: inheritance from non-protocol type 'Error'
extension ComplexMacroError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:2892:1: error: inheritance from non-protocol type 'Error'
extension CoverallError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:2940:1: error: inheritance from non-protocol type 'Error'
extension CoverallFlatError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:2988:1: error: inheritance from non-protocol type 'Error'
extension CoverallFlatMacroError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:3036:1: error: inheritance from non-protocol type 'Error'
extension CoverallMacroError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:3080:1: error: inheritance from non-protocol type 'Error'
extension CoverallRichErrorNoVariantData: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:3370:1: error: inheritance from non-protocol type 'Error'
extension RootError: Error { }
^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:286:27: error: thrown expression type 'Error' does not conform to 'Error'
                throw try errorHandler(callStatus.errorBuf)
                      ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:760:100: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterTypeCoveralls.lift(try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                                   ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:805:102: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
open func falliblePanic(message: String)throws  {try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                                     ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:884:91: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterBool.lift(try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                          ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:892:90: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeComplexError.SwiftType' (aka '(RustBuffer) throws -> ComplexError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterBool.lift(try rustCallWithError(FfiConverterTypeComplexError.lift) {
                                                                                         ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:904:91: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterBool.lift(try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                          ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:961:91: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
open func takeOtherFallible()throws  {try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                          ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:1059:61: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
        try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                            ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:1076:104: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterTypeFalliblePatch.lift(try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                                       ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:1214:98: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeComplexError.SwiftType' (aka '(RustBuffer) throws -> ComplexError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterOptionString.lift(try rustCallWithError(FfiConverterTypeComplexError.lift) {
                                                                                                 ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:1223:93: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
    return try  FfiConverterString.lift(try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                            ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4074:95: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallError.SwiftType' (aka '(RustBuffer) throws -> CoverallError') to expected argument type '(RustBuffer) throws -> Error'
public func println(text: String)throws  {try rustCallWithError(FfiConverterTypeCoverallError.lift) {
                                                                                              ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4099:94: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallFlatError.SwiftType' (aka '(RustBuffer) throws -> CoverallFlatError') to expected argument type '(RustBuffer) throws -> Error'
public func throwFlatError()throws  {try rustCallWithError(FfiConverterTypeCoverallFlatError.lift) {
                                                                                             ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4104:120: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallRichErrorNoVariantData.SwiftType' (aka '(RustBuffer) throws -> CoverallRichErrorNoVariantData') to expected argument type '(RustBuffer) throws -> Error'
public func throwRichErrorNoVariantData()throws  {try rustCallWithError(FfiConverterTypeCoverallRichErrorNoVariantData.lift) {
                                                                                                                       ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4142:102: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeComplexMacroError.SwiftType' (aka '(RustBuffer) throws -> ComplexMacroError') to expected argument type '(RustBuffer) throws -> Error'
public func throwComplexMacroError()throws  {try rustCallWithError(FfiConverterTypeComplexMacroError.lift) {
                                                                                                     ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4147:104: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallFlatMacroError.SwiftType' (aka '(RustBuffer) throws -> CoverallFlatMacroError') to expected argument type '(RustBuffer) throws -> Error'
public func throwFlatMacroError()throws  {try rustCallWithError(FfiConverterTypeCoverallFlatMacroError.lift) {
                                                                                                       ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4152:96: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeCoverallMacroError.SwiftType' (aka '(RustBuffer) throws -> CoverallMacroError') to expected argument type '(RustBuffer) throws -> Error'
public func throwMacroError()throws  {try rustCallWithError(FfiConverterTypeCoverallMacroError.lift) {
                                                                                               ^
/mounted_workdir/target/tmp/uniffi-fixture-coverall-af68e6307d32e36f/coverall.swift:4157:86: error: cannot convert value of type '(RustBuffer) throws -> FfiConverterTypeRootError.SwiftType' (aka '(RustBuffer) throws -> RootError') to expected argument type '(RustBuffer) throws -> Error'
public func throwRootError()throws  {try rustCallWithError(FfiConverterTypeRootError.lift) {

[arg0d]

The worst part about this issue is that you get feedback after shipping the generated bindings to consumers :sweat_smile: Or after trying to compile them yourself.

[Sajjon]

This can be trivially fixed by the bindgen generator adding a Swift. prefix before Error.

This is valid Swift Code:

enum Error: Swift.Error { ... }

[Sajjon]

@arg0d you can fix a PR for this your self, just append Swift. here: https://github.com/mozilla/uniffi-rs/blob/main/uniffi_bindgen/src/bindings/swift/templates/ErrorTemplate.swift#L86

(probably a good idea to include a "fixture" too)

[mhammond]

(probably a good idea to include a "fixture" too)

To be clear, a new fixture may not be necessary - an addition to an existing one is generally fine - whatever works for you

[Sajjon]

@arg0d Can you try https://github.com/mozilla/uniffi-rs/commit/3d93e26ce323f19ad752fb592dc206756153a406 ? it should solve this issue :)

[arg0d]

Looks good, thanks!

[arg0d]

I'm trying to fix this for C#, and I'm wondering if this was a good way to fix this issue.

Shadowing "system error" type is not a good solution, it creates some really confusing code. Given the following code, it's quite natural to assume Exception refers to System.Exception. Reading this code, nobody would expect that Exception might actually refer to a user defined type Exception, rather than System.Exception. This might introduce a really sneaky bug, where the programmer naturally expects that all exceptions will be caught, but actually only the user-defined exception would be caught, leading to uncaught exception bugs.

try {
    DoSomething();
} catch (Exception e) {
    ..
}

[mhammond]

I'm not sure what you mean - IIUC, that PR tries to always use the fully-qualified name when it is referring to a non-uniffi error type. I agree that someone choosing to shadow this name might be shooting themselves in the foot, but I'm not sure Uniffi needs to avoid that when the alternative is for them to just not do that?

[arg0d]

I did some testing, and turns out I was just imagining a problem. I assumed Exception is an implicit type in C#, but it's actually part of using System; namespace. So if you try to using System; using uniffi.example; where uniffi.example contains type Exception, C# compiler generates error: ambiguous references to 'Exception'. I assume there is an equivalent error for Swift/Kotlin.