nervosnetwork / molecule

Another serialization system: minimalist and canonicalization.
MIT License
36 stars 23 forks source link

[Feature] Add custom union ID syntax #65

Closed eval-exec closed 1 year ago

eval-exec commented 1 year ago

This PR want to close #59

working in progress.

eval-exec commented 1 year ago

These schema are equal:

union Foo {
    a,
    b,
    c,
}

union Foo {
    a : 0,
    b : 1,
    c : 2,
}

union Foo {
    a,
    b : 1,
    c,
}

These are equal too:

union Foo {
    a : 1,
    b,
    c : 10,
        d,
}
union Foo {
    c : 10,
        d,
    a : 1,
    b,
}
union Foo {
    a : 1,
    b : 2,
    c : 10,
        d : 11,
}

These schema are bad syntax:

union Foo {
    a : 9,
    b : 9,
}

union Foo {
    a : 10,
    b,
    c,
        d : 9,
        e,
        f,
}

Implementation Detail

For compatibility with older versions, I added a pest code block for customize union_item_decl:

custom_union_item_decl       =  {
                                    identifier ~ (brk)* ~ ":" ~ (brk)* ~
                                    number_greater_or_equal_than_zero     ~ (brk)* ~
                                    field_end
                                }

And added a new struct for ast::raw::UnionDecl:

#[derive(Debug, Property)]
pub(crate) struct CustomUnionItemDecl {
    typ: String,
    id: usize,
}
#[derive(Debug, Property)]
pub(crate) struct UnionDecl {
    name: String,
    items: Vec<CustomUnionItemDecl>,
    imported_depth: usize,
}

Then, parser can recognize molecule schema like these:

union Foo {
    a,
        b,
}

union Foo {
    a : 0,
    b : 1,
}
union Foo {
    a,
    b : 10,
}
eval-exec commented 1 year ago

Compatibility test and integration test haven't been completed, but the main logic and unit test of "custom union ID feature" are ready for review. @yangby-cryptape @driftluo @zhangsoledad

quake commented 1 year ago

can we follow other struct field definition convention by using : instead of = ?

union Foo {
    a: 0,
    b: 1,
    c: 2,
}
eval-exec commented 1 year ago

@yangby-cryptape The CI jobs are using v1.58.1, Do you think it's necessary to upgrade molecule's rust toolchain to v1.67.1 ?

https://github.com/nervosnetwork/molecule/blob/master/.github/workflows/ci.yaml#L24

quake commented 1 year ago

@yangby-cryptape The CI jobs are using v1.58.1, Do you think it's necessary to upgrade molecule's rust toolchain to v1.67.1 ?

https://github.com/nervosnetwork/molecule/blob/master/.github/workflows/ci.yaml#L24

Let's do it with a new pr

eval-exec commented 1 year ago

@quake I didn't change any code after your review, but I rebased the code to make commits message more neat and more convenient for review. Please re-trigger the github CI action.