Skip to content

Commit

Permalink
bug fix: ops that stay strictly at the EOF after assigns ops are igno…
Browse files Browse the repository at this point in the history 
…red (#4047)

* bug fix: ops strictly at the EOF after assigns

* propagate peek error

* use peek without skipping line terminator

* remove `Ok` from `lexer/operators.rs`
  • Loading branch information
@Nikita-str
Nikita-str authored Dec 9, 2024
1 parent 0c5f3b7 commit 67f6d56
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 75 deletions.
38 changes: 38 additions & 0 deletions core/engine/src/tests/operators.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,44 @@ fn delete_in_strict_function_returned() {
)]);
}

#[test]
fn ops_at_the_end() {
let msg = "abrupt end";

let mut actions = vec![TestAction::assert_eq("var a, b=3; a = b ++", 3)];

let abrupt_op_sources = [
// there was a bug with different behavior with and without space at the end;
// so few lines are almost the same except for ending space
"var a, b=3; a = b **",
"var a, b=3; a = b ** ",
"var a, b=3; a = b *",
"var a, b=3; a = b * ",
"var a, b=3; a /= b *",
"var a, b=3; a /= b * ",
"var a, b=3; a = b /",
"var a, b=3; a = b / ",
"var a, b=3; a = b +",
"var a, b=3; a = b -",
"var a, b=3; a = b ||",
"var a, b=3; a = b || ",
"var a, b=3; a = b ==",
"var a, b=3; a = b ===",
];

for source in abrupt_op_sources {
actions.push(TestAction::assert_native_error(
source,
JsNativeErrorKind::Syntax,
msg,
));
}

actions.push(TestAction::assert_eq("var a, b=3; a = b --", 3));

run_test_actions(actions);
}

#[test]
fn regex_slash_eq() {
run_test_actions([
Expand Down
6 changes: 3 additions & 3 deletions core/parser/src/lexer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,9 @@ impl<R> Lexer<R> {
}
}
} else {
Err(Error::syntax(
"Abrupt end: Expecting Token /,*,= or regex",
start,
Ok(Token::new(
Punctuator::Div.into(),
Span::new(start, self.cursor.pos()),
))
}
}
Expand Down
103 changes: 45 additions & 58 deletions core/parser/src/lexer/operator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ use boa_ast::{Position, Punctuator, Span};
use boa_interner::Interner;
use boa_profiler::Profiler;

const CHAR_ASSIGN: u32 = '=' as u32;

/// `vop` tests the next token to see if we're on an assign operation of just a plain binary operation.
///
/// If the next value is not an assignment operation it will pattern match the provided values and return the corresponding token.
macro_rules! vop {
($cursor:ident, $assign_op:expr, $op:expr) => ({
match $cursor.peek_char()? {
None => Err(Error::syntax("abrupt end - could not preview next value as part of the operator", $cursor.pos())),
Some(0x3D /* = */) => {
None => $op,
Some(CHAR_ASSIGN) => {
$cursor.next_char()?.expect("= token vanished");
$assign_op
}
Expand All @@ -22,34 +24,34 @@ macro_rules! vop {
});
($cursor:ident, $assign_op:expr, $op:expr, {$($case:pat => $block:expr), +}) => ({
match $cursor.peek_char()? {
None => Err(Error::syntax("abrupt end - could not preview next value as part of the operator", $cursor.pos())),
Some(0x3D /* = */) => {
None => $op,
Some(CHAR_ASSIGN) => {
$cursor.next_char()?.expect("= token vanished");
$assign_op
},
$($case => {
$cursor.next_char()?.expect("Token vanished");
$block
})+,
_ => $op,
Some(_) => $op,
}
});
}

/// The `op` macro handles binary operations or assignment operations and converts them into tokens.
macro_rules! op {
($cursor:ident, $start_pos:expr, $assign_op:expr, $op:expr) => ({
Ok(Token::new(
vop!($cursor, $assign_op, $op)?.into(),
Token::new(
vop!($cursor, $assign_op, $op).into(),
Span::new($start_pos, $cursor.pos()),
))
)
});
($cursor:ident, $start_pos:expr, $assign_op:expr, $op:expr, {$($case:pat => $block:expr),+}) => ({
let punc: Punctuator = vop!($cursor, $assign_op, $op, {$($case => $block),+})?;
Ok(Token::new(
let punc: Punctuator = vop!($cursor, $assign_op, $op, {$($case => $block),+});
Token::new(
punc.into(),
Span::new($start_pos, $cursor.pos()),
))
)
});
}

Expand Down Expand Up @@ -87,29 +89,22 @@ impl<R> Tokenizer<R> for Operator {
{
let _timer = Profiler::global().start_event("Operator", "Lexing");

match self.init {
b'*' => op!(cursor, start_pos, Ok(Punctuator::AssignMul), Ok(Punctuator::Mul), {
Some(0x2A /* * */) => vop!(cursor, Ok(Punctuator::AssignPow), Ok(Punctuator::Exp))
Ok(match self.init {
b'*' => op!(cursor, start_pos, Punctuator::AssignMul, Punctuator::Mul, {
Some(0x2A /* * */) => vop!(cursor, Punctuator::AssignPow, Punctuator::Exp)
}),
b'+' => op!(cursor, start_pos, Ok(Punctuator::AssignAdd), Ok(Punctuator::Add), {
Some(0x2B /* + */) => Ok(Punctuator::Inc)
b'+' => op!(cursor, start_pos, Punctuator::AssignAdd, Punctuator::Add, {
Some(0x2B /* + */) => Punctuator::Inc
}),
b'-' => op!(cursor, start_pos, Ok(Punctuator::AssignSub), Ok(Punctuator::Sub), {
Some(0x2D /* - */) => {
Ok(Punctuator::Dec)
}
b'-' => op!(cursor, start_pos, Punctuator::AssignSub, Punctuator::Sub, {
Some(0x2D /* - */) => Punctuator::Dec
}),
b'%' => op!(
cursor,
start_pos,
Ok(Punctuator::AssignMod),
Ok(Punctuator::Mod)
),
b'|' => op!(cursor, start_pos, Ok(Punctuator::AssignOr), Ok(Punctuator::Or), {
Some(0x7C /* | */) => vop!(cursor, Ok(Punctuator::AssignBoolOr), Ok(Punctuator::BoolOr))
b'%' => op!(cursor, start_pos, Punctuator::AssignMod, Punctuator::Mod),
b'|' => op!(cursor, start_pos, Punctuator::AssignOr, Punctuator::Or, {
Some(0x7C /* | */) => vop!(cursor, Punctuator::AssignBoolOr, Punctuator::BoolOr)
}),
b'&' => op!(cursor, start_pos, Ok(Punctuator::AssignAnd), Ok(Punctuator::And), {
Some(0x26 /* & */) => vop!(cursor, Ok(Punctuator::AssignBoolAnd), Ok(Punctuator::BoolAnd))
b'&' => op!(cursor, start_pos, Punctuator::AssignAnd, Punctuator::And, {
Some(0x26 /* & */) => vop!(cursor, Punctuator::AssignBoolAnd, Punctuator::BoolAnd)
}),
b'?' => {
let (first, second) = (cursor.peek_char()?, cursor.peek_n(2)?[1]);
Expand All @@ -119,62 +114,54 @@ impl<R> Tokenizer<R> for Operator {
op!(
cursor,
start_pos,
Ok(Punctuator::AssignCoalesce),
Ok(Punctuator::Coalesce)
Punctuator::AssignCoalesce,
Punctuator::Coalesce
)
}
Some(0x2E /* . */) if !matches!(second, Some(second) if (0x30..=0x39 /* 0..=9 */).contains(&second)) =>
{
cursor.next_char()?.expect(". vanished");
Ok(Token::new(
Token::new(
TokenKind::Punctuator(Punctuator::Optional),
Span::new(start_pos, cursor.pos()),
))
)
}
_ => Ok(Token::new(
_ => Token::new(
TokenKind::Punctuator(Punctuator::Question),
Span::new(start_pos, cursor.pos()),
)),
),
}
}
b'^' => op!(
cursor,
start_pos,
Ok(Punctuator::AssignXor),
Ok(Punctuator::Xor)
),
b'^' => op!(cursor, start_pos, Punctuator::AssignXor, Punctuator::Xor),
b'=' => op!(cursor, start_pos, if cursor.next_if(0x3D /* = */)? {
Ok(Punctuator::StrictEq)
Punctuator::StrictEq
} else {
Ok(Punctuator::Eq)
}, Ok(Punctuator::Assign), {
Punctuator::Eq
}, Punctuator::Assign, {
Some(0x3E /* > */) => {
Ok(Punctuator::Arrow)
Punctuator::Arrow
}
}),
b'<' => {
op!(cursor, start_pos, Ok(Punctuator::LessThanOrEq), Ok(Punctuator::LessThan), {
Some(0x3C /* < */) => vop!(cursor, Ok(Punctuator::AssignLeftSh), Ok(Punctuator::LeftSh))
op!(cursor, start_pos, Punctuator::LessThanOrEq, Punctuator::LessThan, {
Some(0x3C /* < */) => vop!(cursor, Punctuator::AssignLeftSh, Punctuator::LeftSh)
})
}
b'>' => {
op!(cursor, start_pos, Ok(Punctuator::GreaterThanOrEq), Ok(Punctuator::GreaterThan), {
Some(0x3E /* > */) => vop!(cursor, Ok(Punctuator::AssignRightSh), Ok(Punctuator::RightSh), {
Some(0x3E /* > */) => vop!(cursor, Ok(Punctuator::AssignURightSh), Ok(Punctuator::URightSh))
op!(cursor, start_pos, Punctuator::GreaterThanOrEq, Punctuator::GreaterThan, {
Some(0x3E /* > */) => vop!(cursor, Punctuator::AssignRightSh, Punctuator::RightSh, {
Some(0x3E /* > */) => vop!(cursor, Punctuator::AssignURightSh, Punctuator::URightSh)
})
})
}
b'!' => op!(
cursor,
start_pos,
vop!(cursor, Ok(Punctuator::StrictNotEq), Ok(Punctuator::NotEq)),
Ok(Punctuator::Not)
vop!(cursor, Punctuator::StrictNotEq, Punctuator::NotEq),
Punctuator::Not
),
b'~' => Ok(Token::new(
Punctuator::Neg.into(),
Span::new(start_pos, cursor.pos()),
)),
b'~' => Token::new(Punctuator::Neg.into(), Span::new(start_pos, cursor.pos())),
op => unimplemented!("operator {}", op),
}
})
}
}
80 changes: 80 additions & 0 deletions core/parser/src/lexer/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,86 @@ fn check_punctuators() {
expect_tokens(&mut lexer, &expected, interner);
}

#[test]
fn check_punctuator_at_the_end() {
//TODO: maybe just use `strum` (`EnumIter`)?
// * it is already in dependencies
let last_expected = [
Punctuator::Add,
Punctuator::And,
Punctuator::Arrow,
Punctuator::AssignAdd,
Punctuator::AssignAnd,
Punctuator::AssignBoolAnd,
Punctuator::AssignBoolOr,
Punctuator::AssignCoalesce,
// Punctuator::AssignDiv, : is unclosed regular expr
Punctuator::AssignLeftSh,
Punctuator::AssignMod,
Punctuator::AssignMul,
Punctuator::AssignOr,
Punctuator::AssignPow,
Punctuator::AssignRightSh,
Punctuator::AssignSub,
Punctuator::AssignURightSh,
Punctuator::AssignXor,
Punctuator::BoolAnd,
Punctuator::BoolOr,
Punctuator::Coalesce,
Punctuator::CloseBlock,
Punctuator::CloseBracket,
Punctuator::CloseParen,
Punctuator::Colon,
Punctuator::Comma,
Punctuator::Dec,
Punctuator::Div,
Punctuator::Dot,
Punctuator::Eq,
Punctuator::GreaterThan,
Punctuator::GreaterThanOrEq,
Punctuator::Inc,
Punctuator::LeftSh,
Punctuator::LessThan,
Punctuator::LessThanOrEq,
Punctuator::Mod,
Punctuator::Mul,
Punctuator::Neg,
Punctuator::Not,
Punctuator::NotEq,
Punctuator::OpenBlock,
Punctuator::OpenBracket,
Punctuator::OpenParen,
Punctuator::Optional,
Punctuator::Or,
Punctuator::Exp,
Punctuator::Question,
Punctuator::RightSh,
Punctuator::Semicolon,
Punctuator::Spread,
Punctuator::StrictEq,
Punctuator::StrictNotEq,
Punctuator::Sub,
Punctuator::URightSh,
Punctuator::Xor,
];

for last in last_expected {
let s = format!("var a = b {}", last.as_str());
let mut lexer = Lexer::from(s.as_bytes());
let interner = &mut Interner::default();

let expected = [
TokenKind::Keyword((Keyword::Var, false)),
TokenKind::identifier(interner.get_or_intern_static("a", utf16!("a"))),
TokenKind::Punctuator(Punctuator::Assign),
TokenKind::identifier(interner.get_or_intern_static("b", utf16!("b"))),
TokenKind::Punctuator(last),
];

expect_tokens(&mut lexer, &expected, interner);
}
}

#[test]
fn check_keywords() {
// https://tc39.es/ecma262/#sec-keywords
Expand Down
29 changes: 18 additions & 11 deletions core/parser/src/parser/cursor/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ where
}

/// Peeks a future token, without consuming it or advancing the cursor.
/// This peeking **skips** line terminators.
///
/// You can skip some tokens with the `skip_n` option.
pub(super) fn peek(
Expand All @@ -114,6 +115,18 @@ where
self.buffered_lexer.peek(skip_n, true, interner)
}

/// Peeks a future token, without consuming it or advancing the cursor.
/// This peeking **does not skips** line terminators.
///
/// You can skip some tokens with the `skip_n` option.
pub(super) fn peek_no_skip_line_term(
&mut self,
skip_n: usize,
interner: &mut Interner,
) -> ParseResult<Option<&Token>> {
self.buffered_lexer.peek(skip_n, false, interner)
}

/// Gets the current strict mode for the cursor.
pub(super) const fn strict(&self) -> bool {
self.buffered_lexer.strict()
Expand Down Expand Up @@ -195,14 +208,12 @@ where
&mut self,
interner: &mut Interner,
) -> ParseResult<SemicolonResult<'_>> {
self.buffered_lexer.peek(0, false, interner)?.map_or(
Ok(SemicolonResult::Found(None)),
|tk| match tk.kind() {
self.peek_no_skip_line_term(0, interner)?
.map_or(Ok(SemicolonResult::Found(None)), |tk| match tk.kind() {
TokenKind::Punctuator(Punctuator::Semicolon | Punctuator::CloseBlock)
| TokenKind::LineTerminator => Ok(SemicolonResult::Found(Some(tk))),
_ => Ok(SemicolonResult::NotFound(tk)),
},
)
})
}

/// Consumes the next token if it is a semicolon, or returns a `Errpr` if it's not.
Expand Down Expand Up @@ -245,10 +256,7 @@ where
context: &'static str,
interner: &mut Interner,
) -> ParseResult<&Token> {
let tok = self
.buffered_lexer
.peek(skip_n, false, interner)
.or_abrupt()?;
let tok = self.peek_no_skip_line_term(skip_n, interner).or_abrupt()?;

if tok.kind() == &TokenKind::LineTerminator {
Err(Error::unexpected(
Expand All @@ -267,8 +275,7 @@ where
skip_n: usize,
interner: &mut Interner,
) -> ParseResult<Option<bool>> {
self.buffered_lexer
.peek(skip_n, false, interner)?
self.peek_no_skip_line_term(skip_n, interner)?
.map_or(Ok(None), |t| {
Ok(Some(t.kind() == &TokenKind::LineTerminator))
})
Expand Down
Loading

0 comments on commit 67f6d56

Please sign in to comment.