Skip to content

Added escaping to ActiveEnum to allow for non-UAX#31 chars, as well as camel_case related conflicts.#1374

Merged
billy1624 merged 9 commits intoSeaQL:masterfrom
denwong47:allow_symbols_in_string_value_of_active_enum
Mar 23, 2023
Merged

Added escaping to ActiveEnum to allow for non-UAX#31 chars, as well as camel_case related conflicts.#1374
billy1624 merged 9 commits intoSeaQL:masterfrom
denwong47:allow_symbols_in_string_value_of_active_enum

Conversation

@denwong47
Copy link
Copy Markdown
Contributor

@denwong47 denwong47 commented Jan 8, 2023 •
edited
Loading

PR Info

Bug Fixes

  • Fixes DeriveActiveEnum throwing
    • "" is not a valid identifier panic if string_value consists only of non-UAX#31 compliant characters
    • the name SomeName is defined multiple times if two enum values have string_value of someName and some_name respectively
    • "_0 123" is not a valid identifier if string_value starts with any digit followed by at least one space in the string.

Details

This adds a function camel_case_with_escaped_non_xid to util.rs of sea-orm-macros, which is a wrapper for camel_case to conduct further escaping specifically suitable for Rust Identifiers.

It does so by:

  • iterating through each character to check
    • if it is not _ or whitespace, and
    • if its within XID_Start character set for first character, and
    • if its within XID_Continue character set for the rest of the string
  • if not,
    • it decode the Unicode character to u32, then replace original character with its {:X} hex representation: e.g. whitespace becomes 0x20.
  • the string is then reconstructed
  • if the string is at this point empty,
    • return special string "__Empty".
  • otherwise apply camel_case.
  • the first character is then checked for numeric; if so, prepend an underscore.

For example,

#[derive(Clone, Debug, PartialEq, EnumIter, DeriveActiveEnum)]
#[sea_orm(
    rs_type = "String",
    db_type = "String(None)",
    enum_name = "conflicting_string_values"
)]
pub enum ConflictingStringValues {
    #[sea_orm(string_value = "")]
    Member1,
    #[sea_orm(string_value = "$")]
    Member2,
    #[sea_orm(string_value = "$$")]
    Member3,
    #[sea_orm(string_value = "AB")]
    Member4,
    #[sea_orm(string_value = "A_B")]
    Member5,
    #[sea_orm(string_value = "A$B")]
    Member6,
    #[sea_orm(string_value = "0 123")]
    Member7,
}

will now produce the following Variant Enum:

        pub enum ConflictingStringValuesVariant {
            __Empty,
            _0x24,
            _0x240x24,
            Ab,
            A0x5Fb,
            A0x24B,
            _0x300x20123,
        }

See Also

Valid Rust Identifier talking about XID_Start and XID_Continue

@denwong47 denwong47 changed the title Added escaping to ActiveEnum to allow for non-Unicode SA#31 chars, as well as camel_case related conflicts. Added escaping to ActiveEnum to allow for non-UAX#31 chars, as well as camel_case related conflicts. Jan 8, 2023
@denwong47
Copy link
Copy Markdown
Contributor Author

denwong47 commented Jan 9, 2023

Fixed CI errors. Apologies!

tyt2y3
tyt2y3 reviewed Jan 10, 2023
View reviewed changes
tests/common/features/sea_orm_active_enums.rs Outdated

#[derive(Debug, Clone, PartialEq, Eq, EnumIter, DeriveActiveEnum, Copy)]
#[sea_orm(rs_type = "String", db_type = "Enum", enum_name = "pop_os_names_typos")]
pub enum PopOSTypos {
Copy link
Copy Markdown
Member

@tyt2y3 tyt2y3 Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😎

tyt2y3
tyt2y3 approved these changes Jan 10, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@tyt2y3 tyt2y3 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution!

The implementation looks clean to me, and I appreciate the documentation.
Still require Billy's review as he is the primary author of sea-orm-macros

@tyt2y3 tyt2y3 requested a review from billy1624 January 10, 2023 08:09
@tyt2y3
Copy link
Copy Markdown
Member

tyt2y3 commented Jan 10, 2023 •
edited
Loading

For example,

#[derive(Clone, Debug, PartialEq, EnumIter, DeriveActiveEnum)]
#[sea_orm(
    rs_type = "String",
    db_type = "String(None)",
    enum_name = "conflicting_string_values"
)]
pub enum ConflictingStringValues {
    #[sea_orm(string_value = "")]
    Member1,
    #[sea_orm(string_value = "$")]
    Member2,
    #[sea_orm(string_value = "$$")]
    Member3,
    #[sea_orm(string_value = "AB")]
    Member4,
    #[sea_orm(string_value = "A_B")]
    Member5,
    #[sea_orm(string_value = "A$B")]
    Member6,
    #[sea_orm(string_value = "0 123")]
    Member7,
}

will now produce the following Variant Enum:

        pub enum ConflictingStringValuesVariant {
            __Empty,
            _0x24,
            _0x240x24,
            Ab,
            A0x5Fb,
            A0x24B,
            _0x300x20123,
        }

It will be great if we can also include this example as a doc test!

@denwong47
Copy link
Copy Markdown
Contributor Author

denwong47 commented Jan 10, 2023

Thank you, I'll have a look at doc testing tomorrow!

@denwong47 denwong47 force-pushed the allow_symbols_in_string_value_of_active_enum branch from 14390cb to 30d385f Compare January 10, 2023 22:49
@denwong47
Copy link
Copy Markdown
Contributor Author

denwong47 commented Jan 10, 2023

Changes post-review:

  • rebased onto latest SeaQL:master
  • added unit test for camel_case_with_escaped_non_uax31 in sea_orm_macros::util::test submodule.
    • @tyt2y3 Unfortunately I wasn't able to do these test as a doc test as suggested - since util is private to the sub-crate, doc test can't import it as far as I know.
  • renamed the main function of camel_case_with_escaped_non_xid to camel_case_with_escaped_non_uax31 - which is a better description of the standard we are trying to check against.
  • fixed Cargo.toml in sea-orm-macros not specifying feature tests-cfg, which seems to be required for an existing unit test.

@billy1624
Copy link
Copy Markdown
Member

billy1624 commented Mar 23, 2023

Hey @denwong47, I just fetch from latest master. Now, I'm going to add a few more test cases here and there before merging. Thanks for the efforts!! Great contribution! :)

billy1624
billy1624 approved these changes Mar 23, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@billy1624 billy1624 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for the PR! More PRs are welcomed! See you around on GitHub :P

@billy1624 billy1624 merged commit 5ef273d into SeaQL:master Mar 23, 2023
darkmmon added a commit to darkmmon/seaql.github.io that referenced this pull request Jul 12, 2023
@darkmmon
active enum fail safe SeaQL/sea-orm#1374
5791b83
billy1624 added a commit to SeaQL/seaql.github.io that referenced this pull request Jul 19, 2023
@darkmmon @billy1624
SeaORM 0.12.x Docs - 1 (#99)
f9ea2e2 
* Optional Field SeaQL/sea-orm#1513

* .gitignore SeaQL/sea-orm#1334

* migration table custom name SeaQL/sea-orm#1511

* OR condition relation SeaQL/sea-orm#1433

* DerivePartialModel SeaQL/sea-orm#1597

* space for migration file naming SeaQL/sea-orm#1570

* composite key up to 12 SeaQL/sea-orm#1508

* seaography integration SeaQL/sea-orm#1599

* QuerySelect SimpleExpr SeaQL/sea-orm#1702

* sqlErr SeaQL/sea-orm#1707

* migration check SeaQL/sea-orm#1519

* postgres array SeaQL/sea-orm#1565

* param intoString SeaQL/sea-orm#1439

* **skipped** re-export SeaQL/sea-orm#1661

* ping SeaQL/sea-orm#1627

* on empty do nothing SeaQL/sea-orm#1708

* on conflict do nothing SeaQL/sea-orm#1712

* **skipped** upgrade versions

* active enum fail safe SeaQL/sea-orm#1374

* relation generation check SeaQL/sea-orm#1435

* entity generation bug SeaQL/sea-schema#105

* **skipped** bug fix that does not require edits

* EnumIter change SeaQL/sea-orm#1535

* completed and fixed a previous todo SeaQL/sea-orm#1570

* amended wordings and structures

* Edit

* Remove temp file

---------

Co-authored-by: Billy Chan <[email protected]>
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 2, 2023

🎉 Released In 0.12.1 🎉

Thank you everyone for the contribution!
This feature is now available in the latest release. Now is a good time to upgrade!
Your participation is what makes us unique; your adoption is what drives us forward.
You can support SeaQL 🌊 by starring our repos, sharing our libraries and becoming a sponsor ⭐.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tyt2y3 tyt2y3 tyt2y3 approved these changes

@billy1624 billy1624 billy1624 approved these changes

Assignees

@denwong47 denwong47

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

string_value of DeriveActiveEnum errors on underscores, spaces and Non-Unicode UAX#31 chars

3 participants

@denwong47 @tyt2y3 @billy1624