Skip to content

Conversation

@tertsdiepraam
Copy link
Contributor

@tertsdiepraam tertsdiepraam commented May 30, 2025

Closes #153.

That example will now fail with this message:

thread 'main' panicked at examples/foo.rs:12:5:
called `Result::unwrap()` on an `Err` value: "Name \"accept\" is a keyword in Roto and therefore not a valid identifier"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The parser will also report more useful errors when a keyword is used when an identifier is expected, the difference is the note at the end of the error:

Error: Parse error: expected an identifier but got 'accept'
   ╭─[examples/simple.roto:2:10]
   │
 2 │ function accept(x: IpAddr) -> bool {
   │          ───┬──  
   │             ╰──── expected an identifier
   │ 
   │ Note: `accept` is a keyword and cannot be used as an identifier.
───╯

and the other case where this happens:

Error: Parse error: expected an identifier, `super`, `pkg` or `dep` but got 'accept'
   ╭─[examples/simple.roto:1:8]
   │
 1 │ import accept;
   │        ───┬──  
   │           ╰──── expected an identifier, `super`, `pkg` or `dep`
   │ 
   │ Note: `accept` is a keyword and cannot be used as an identifier.
───╯

The superfluous ` mark has also been removed.

@tertsdiepraam tertsdiepraam force-pushed the dont-register-invalid-name branch from 913a2e5 to 6d4855d Compare May 30, 2025 09:23
@tertsdiepraam tertsdiepraam changed the title Don't allow registering items with invalid identifiers as names Improve errors when keywords are used as identifiers May 30, 2025
Copy link

@bal-e bal-e left a comment

Choose a reason for hiding this comment

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

Looks good! I'm a bit surprised just how much stuff this relatively simple refactor involved.

Comment on lines +434 to +436
"true" => return ControlFlow::Break((Token::Bool(true), span)),
"false" => return ControlFlow::Break((Token::Bool(false), span)),
x => return ControlFlow::Break((Token::Ident(x), span)),
Copy link

Choose a reason for hiding this comment

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

Neat!

Comment on lines 479 to 481
Token::Keyword(k) => {
return k.fmt(f);
}
Copy link

Choose a reason for hiding this comment

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

Nit: maybe have fn Keyword::code(&self) -> &'static str and use that here? It might come in handy in a few other situations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah probably, although I don't like code. Maybe as_str?

@tertsdiepraam
Copy link
Contributor Author

Looks good! I'm a bit surprised just how much stuff this relatively simple refactor involved.

Yeah well it's a couple things I suppose:

  • Splitting keywords from other tokens
  • Move the runtime tests
  • Error on doing this wrong in a Roto script
  • Error on doing this wrong while registering a function

So all in all it's not trivial.

@tertsdiepraam tertsdiepraam merged commit aa248c8 into main Jun 6, 2025
13 checks passed
@tertsdiepraam tertsdiepraam deleted the dont-register-invalid-name branch July 14, 2025 20:00
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.

Can't create a method named "accept" or "reject"

2 participants