Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

denwong47
Copy link
Contributor

@denwong47 denwong47 commented Jan 8, 2023

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
Contributor Author

Fixed CI errors. Apologies!


#[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
Member

Choose a reason for hiding this comment

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

😎

Copy link
Member

@tyt2y3 tyt2y3 left a comment

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
Member

tyt2y3 commented Jan 10, 2023

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
Contributor Author

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
Contributor Author

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
Member

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! :)

Copy link
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
72 checks passed
darkmmon added a commit to darkmmon/seaql.github.io that referenced this pull request Jul 12, 2023
billy1624 added a commit to SeaQL/seaql.github.io that referenced this pull request Jul 19, 2023
* 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 <ccw.billy.123@gmail.com>
@github-actions
Copy link

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
Labels
None yet
Projects
None yet
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