Schema generates conflicting names for enum variants/across crates

[jshaw-jump]

Original issue (conflicting names of enum variants with outer structs)

The following code fails to compile because the schema name generation for enum variants can produce name clashes.

#[derive(BorshSchema)]
enum Foo {
    Bar(FooBar),
}

#[derive(BorshSchema)]
struct FooBar {}

error[E0275]: overflow evaluating the requirement `<Foo as borsh::BorshSchema>::add_definitions_recursively::FooBar: borsh::BorshSchema`
 --> code.rs:1:10
  |
2 | #[derive(BorshSchema)]
  |          ^^^^^^^^^^^
  |
  = help: see issue #48214
  = note: this error originates in the derive macro `borsh::BorshSchema` (in Nightly builds, run with -Z macro-backtrace for more info)

I believe the error is a result of using a simple concatenation to generate the name. https://github.com/near/borsh-rs/blob/7325a0aab74049237b9f978e1e2c0fcf7bb799c2/borsh-schema-derive-internal/src/enum_schema.rs#L28

---

Widened issue (conflicting names for types globally across multiple crates)

As per research by @dzmitry-lahoda, BorshSchema is problematic to use when multiple third-party crates are involved, which can introduce conflicting names and only be panicing with conflicts when tried to be used elsewhere.

use crate_a::A as AA;
use crate_b::A as AB;

#[derive(BorshSchema)]
struct B {
    x: AA,
    y: AB,
}

[frol]

Good catch! Thanks for reporting it, but I believe borsh schema issues won't be addressed at the moment as it is not used in production and was just an experiment without a clear specification

[austinabell]

cc @itegulov @miraclx we might want to consider this issue now that we are using schema for ABI

[dzmitry-lahoda]

what is possible outline of fix for this? I have clash for same named struct in different parts of my big account struct.

[dzmitry-lahoda]

I see next:

• BorshSchema is used with solana contracts, even some in spl
• BorshSchema is used in near codebase in several types
• NearSchema with borsh attribute generates BorshSchema impl. as per code and documentation NearSchema does BorshSchema as default.

So there are some users, may be a lot if search on GH.

cc @frol

[dzmitry-lahoda]

Did quick tests on schemars

TLDR

For same named items schemars generates second declaration/definition under +1 suffixed name.

Long:

See #/definitions/A2

thread 'engine::tests::borsh_diff::serde_name_conflict' panicked at engine/src/engine/tests/borsh_diff.rs:68:5: RootSchema { meta_schema: Some("http://json-schema.org/draft-07/schema#"), schema: SchemaObject { metadata: Some(Metadata { id: None, title: Some("D"), description: None, default: None, deprecated: false, read_only: false, write_only: false, examples: [] }), instance_type: Some(Single(Object)), format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: Some(ObjectValidation { max_properties: None, min_properties: None, required: {"b", "c"}, properties: {"b": Object(SchemaObject { metadata: None, instance_type: None, format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: None, reference: Some("#/definitions/B"), extensions: {} }), "c": Object(SchemaObject { metadata: None, instance_type: None, format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: None, reference: Some("#/definitions/C"), extensions: {} })}, pattern_properties: {}, additional_properties: None, property_names: None }), reference: None, extensions: {} }, definitions: {"A": Object(SchemaObject { metadata: None, instance_type: Some(Single(Object)), format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: Some(ObjectValidation { max_properties: None, min_properties: None, required: {"a"}, properties: {"a": Object(SchemaObject { metadata: None, instance_type: Some(Single(Integer)), format: Some("uint8"), enum_values: None, const_value: None, subschemas: None, number: Some(NumberValidation { multiple_of: None, maximum: None, exclusive_maximum: None, minimum: Some(0.0), exclusive_minimum: None }), string: None, array: None, object: None, reference: None, extensions: {} })}, pattern_properties: {}, additional_properties: None, property_names: None }), reference: None, extensions: {} }), "A2": Object(SchemaObject { metadata: None, instance_type: Some(Single(Object)), format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: Some(ObjectValidation { max_properties: None, min_properties: None, required: {"a"}, properties: {"a": Object(SchemaObject { metadata: None, instance_type: Some(Single(Integer)), format: Some("uint8"), enum_values: None, const_value: None, subschemas: None, number: Some(NumberValidation { multiple_of: None, maximum: None, exclusive_maximum: None, minimum: Some(0.0), exclusive_minimum: None }), string: None, array: None, object: None, reference: None, extensions: {} })}, pattern_properties: {}, additional_properties: None, property_names: None }), reference: None, extensions: {} }), "B": Object(SchemaObject { metadata: None, instance_type: Some(Single(Object)), format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: Some(ObjectValidation { max_properties: None, min_properties: None, required: {"a"}, properties: {"a": Object(SchemaObject { metadata: None, instance_type: None, format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: None, reference: Some("#/definitions/A"), extensions: {} })}, pattern_properties: {}, additional_properties: None, property_names: None }), reference: None, extensions: {} }), "C": Object(SchemaObject { metadata: None, instance_type: Some(Single(Object)), format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: Some(ObjectValidation { max_properties: None, min_properties: None, required: {"a"}, properties: {"a": Object(SchemaObject { metadata: None, instance_type: None, format: None, enum_values: None, const_value: None, subschemas: None, number: None, string: None, array: None, object: None, reference: Some("#/definitions/A2"), extensions: {} })}, pattern_properties: {}, additional_properties: None, property_names: None }), reference: None, extensions: {} })} }

mod a {
    use borsh::{de, BorshDeserialize, BorshSchema, BorshSerialize};

    #[derive(Default, Debug, BorshSerialize, BorshSchema, arbitrary::Arbitrary, schemars::JsonSchema, serde::Serialize)]
    pub  struct A {
        a: u8,
    }
    
    #[derive(Default, Debug, BorshSerialize, BorshSchema, arbitrary::Arbitrary, schemars::JsonSchema, serde::Serialize)]
    pub struct  B {
        a: A,
    }
}

mod b {
    use borsh::{de, BorshDeserialize, BorshSchema, BorshSerialize};

    #[derive(Default, Debug, BorshSerialize, BorshSchema, arbitrary::Arbitrary, schemars::JsonSchema, serde::Serialize)]
    pub  struct A {
        a: u8,
    }
    
    #[derive(Default, Debug, BorshSerialize, BorshSchema, arbitrary::Arbitrary, schemars::JsonSchema, serde::Serialize)]
    pub struct  C {
        a: A,
    }
}

use a::B;
use b::C;
#[derive(Default, Debug, BorshSerialize, BorshSchema, arbitrary::Arbitrary, schemars::JsonSchema, serde::Serialize)]
struct D {
    b: B,
    c: C,
}

#[test]
fn serde_name_conflict() {
    let schema = schemars::gen::SchemaGenerator::default()
    .into_root_schema_for::<D>();
    panic!("{:?}", schema);
}

[dzmitry-lahoda]

@frol @itegulov @miraclx @austinabell

would PR which does exact same bump of number suffix in declaration accepted as fix as in schemars?

solution

So for type A and A in definitions, second one will become A2

possible improvement

• improvement can be if definitions are 100% same, do not bump, but use existing one (not conflict panic)
• store in schema name as it was given as is, and reference name with bumped id

alternative

Prefix definition with parent struct, so if A and A in B and C, name of second one will be B_A, which will require need to propagate parent info (hopefully it is already).

In case A and A in same struct one will have alias or mod prefix, so use alias or mod prefix.

Alternative seems complicated.

Not sure if one can get full Rust compile time info about modules in proc macro.