apollographql/apollo-rs

Harden `peek()` / `peek_data()` call pairs in the parser

goto-bus-stop opened this issue · 0 comments

We have a few places where we call peek() to check an upcoming token's kind, and then peek_data().unwrap() to check its value. For example, to identify what type of definition is coming up: type, union, or something else. This is an actual self-contained snippet showing what I mean:

if let Some(TokenKind::Name) = p.peek() {
if p.peek_data().unwrap() == "implements" {
implements_interfaces(p);
} else {
p.err("unexpected Name");
}
}

The unwrap() call is valid in those cases, but a bit icky. For it to be valid, the current token must not change between those calls, and this is not statically verifiable. A mistake in a refactor could disconnect the peek() and peek_data() calls and then the unwrap could panic.

I'd like to see these refactored to use let Some(token) = peek_token() where possible, and then use token.kind / token.data directly, so there is no way that it could ever panic.