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

Allow use of rust keywords as pest rules #750

Merged
merged 5 commits into from Dec 14, 2022
Merged

Allow use of rust keywords as pest rules #750

merged 5 commits into from Dec 14, 2022

Conversation

DvvCz
Copy link
Contributor

@DvvCz DvvCz commented Dec 13, 2022

This prefixes all non-builtin rules with r# to allow for use of rust keywords as pest rules. This is my first pull request so please let me know if I should make any changes.

This refactors code to use format_ident! rather than Ident::new (with format! internally in the case of _PEST_GRAMMAR_{}), as it works the same internally, with the Span falling back to Span::call_site() when not given an Ident, so there shouldn't be any issues.

Updated test cases that use identifiers as expressions to test escaping them

Fixes #747

This prefixes all non-builtin rules with r# to allow for use of rust keywords as pest rules.

This refactors code to use format_ident! rather than Ident::new (with format! internally in some cases), as it does the same thing internally. Span is defined to fallback to Span::call_site() in case of being given a non-ident, so there shouldn't be any issues.

Updated generate_complete test case and removed the rust keyword restriction from the validator.
* Updated tests that use identifiers to escape them with `r#`

* Changed the previously added `generate_complete` `r#if` test to ensure using an identifier as an expression is properly emitted

* Removed some redundant Ident::new() inside of `format_ident!` calls
@DvvCz DvvCz requested a review from a team as a code owner December 13, 2022 20:45
@DvvCz DvvCz requested review from NoahTheDuke and removed request for a team December 13, 2022 20:45
@NoahTheDuke
Copy link
Member

This is super interesting. What compatibility breaks could there be?

Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

great stuff! there's at least one semver breaking change, so one possibly suggestion may be to keep the old unused public function and mark it deprecated.
Also, would it be possible to add or extend derive's tests to have a grammar that uses some Rust keywords?

meta/src/validator.rs Show resolved Hide resolved
Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

  • cargo fmt
  • bring back validate_rust_keywords and mark it deprecated:
Checking pest_meta
    Checking <unknown> v2.5.0 -> v2.5.1 (patch change)
   Completed [   0.116s] 22 checks; 21 passed, 1 failed, 0 unnecessary

--- failure function_missing: pub fn removed or renamed ---

Description:
A publicly-visible function cannot be imported by its prior path. A `pub use` may have been removed, or the function itself may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-check/tree/v0.14.0/src/queries/function_missing.ron

Failed in:
  function pest_meta::validator::validate_rust_keywords, previously in file meta/src/validator.rs:120
       Final [   0.117s] semver requires new major version: 1 major and 0 minor checks failed

@DvvCz
Copy link
Contributor Author

DvvCz commented Dec 13, 2022

This is super interesting. What compatibility breaks could there be?

There shouldn't be any compatibility breaks, as r#Ident maps to Ident, as noted in the issue. The only new import this uses is format_ident! which is from quote 1.0

`generate_rule` was checking the name after formatting for whether it was `WHITESPACE` or `COMMENT`.

Re-imported proc_macro2::Span for tests since they still use it.
@tomtau tomtau merged commit 2c47201 into pest-parser:master Dec 14, 2022
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.

Request: Lift restriction against Rust keyword (by applying r# prefix)
3 participants