Skip to content

fix(rust): clarify error message for non-token reserved words#4323

Merged
WillLillis merged 1 commit intotree-sitter:masterfrom
jonshea:jonshea/clarify-NonTokenReservedWord-error-message
Apr 9, 2025
Merged

fix(rust): clarify error message for non-token reserved words#4323
WillLillis merged 1 commit intotree-sitter:masterfrom
jonshea:jonshea/clarify-NonTokenReservedWord-error-message

Conversation

@jonshea
Copy link
Contributor

@jonshea jonshea commented Mar 31, 2025

Improve the NonTokenReservedWord error message by including the specific reserved word that was not used as a token. The Updated message also clarifies that reserved words must be defined as tokens appearing in the grammar rules.

@jonshea
Copy link
Contributor Author

jonshea commented Mar 31, 2025

I’d also be happy to change this error to a warning. I can’t think of any reason it needs to be an error instead of a warning, but maybe I am missing something.

@clason clason added the ci:backport release-0.25 Backport label label Apr 6, 2025
Improve the `NonTokenReservedWord` error message by including the
specific reserved word that was not used as a token.
@jonshea jonshea force-pushed the jonshea/clarify-NonTokenReservedWord-error-message branch from e549b38 to d9c4709 Compare April 7, 2025 16:20
@clason clason requested a review from WillLillis April 8, 2025 10:06
@WillLillis
Copy link
Member

This is a great improvement to the existing error message. For a future improvement, it might be nice to cover the other rule types. I came up with this first pass, but it needs some more testing/cleanup and I don't want that to hold up this change :)

...
let rule = match &reserved_rule {
    Rule::String(s) | Rule::Pattern(s, _) | Rule::NamedSymbol(s) => {
        format!("'{s}'")
    }
    Rule::Blank => "`blank`".to_string(),
    Rule::Choice(c) => {
        if c.len() == 2 && matches!((&c[0], &c[1]), (Rule::Repeat(_), Rule::Blank))
        {
            "`repeat`".to_string()
        } else if c.len() == 2 && matches!(&c[1], Rule::Blank) {
            "`optional`".to_string()
        } else {
            "`choice`".to_string()
        }
    }
    Rule::Repeat(_) => "`repeat1`".to_string(),
    Rule::Seq(_) => "`seq`".to_string(),
    Rule::Metadata { params, .. } => {
        if params.alias.is_some() {
            "`alias`".to_string()
        } else if params.field_name.is_some() {
            "`field`".to_string()
        } else if params.associativity.is_some()
            || params.precedence != Precedence::None
            || params.dynamic_precedence != 0
        {
            "`prec`".to_string()
        } else if params.is_token {
            "`token`".to_string()
        } else {
            "<unexpected rule>".to_string() // unreachable?
        }
    }
    // unreachable as well?
    Rule::Symbol(_) | Rule::Reserved { .. } => "<unexpected rule>".to_string(),
};
Err(ExtractTokensError::NonTokenReservedWord(rule))?;
...

@WillLillis WillLillis merged commit 92c5d3b into tree-sitter:master Apr 9, 2025
18 checks passed
@tree-sitter-ci-bot
Copy link

Successfully created backport PR for release-0.25:

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants