Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat: mark errors with "guessed" unknown nodes instead of errors #1789

Merged
merged 12 commits into from
Nov 25, 2021
2 changes: 1 addition & 1 deletion .github/workflows/test262.yml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ jobs:
issue-number: ${{ steps.pr-number.outputs.pr }}
body-includes: Test262 comparison coverage results on ${{ matrix.os }}

- name: Update comment
- name: Update existing comment
if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id
uses: peter-evans/create-or-update-comment@v1.4.5
continue-on-error: true
Expand Down
6 changes: 4 additions & 2 deletions crates/rome_formatter/src/ts/statements/statement.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{FormatElement, FormatResult, Formatter, ToFormatElement};
use rslint_parser::ast::JsAnyStatement;
use rslint_parser::{ast::JsAnyStatement, AstNode};

impl ToFormatElement for JsAnyStatement {
fn to_format_element(&self, formatter: &Formatter) -> FormatResult<FormatElement> {
Expand Down Expand Up @@ -54,7 +54,9 @@ impl ToFormatElement for JsAnyStatement {
JsAnyStatement::JsVariableDeclarationStatement(decl) => {
decl.to_format_element(formatter)
}
JsAnyStatement::JsUnknownStatement(_) => todo!(),
JsAnyStatement::JsUnknownStatement(unknown_statement) => {
Ok(formatter.format_raw(unknown_statement.syntax()))
}
JsAnyStatement::ImportDecl(_) => todo!(),
JsAnyStatement::ExportNamed(_) => todo!(),
JsAnyStatement::ExportDefaultDecl(_) => todo!(),
Expand Down
1 change: 1 addition & 0 deletions crates/rslint_parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ mod lossless_tree_sink;
mod lossy_tree_sink;
mod numbers;
mod parse;
pub(crate) mod parse_recovery;
mod state;
mod syntax_node;
mod token_source;
Expand Down
98 changes: 98 additions & 0 deletions crates/rslint_parser/src/parse_recovery.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
use crate::{Parser, ParserError, TokenSet};
use rslint_errors::Diagnostic;
use rslint_lexer::{SyntaxKind, T};

/// This struct contains the information needed to the parser to recover from a certain error
///
/// By default it doesn't check curly braces, use [with_braces_included] to turn opt-in the check
#[derive(Debug)]
pub struct ParseRecovery {
/// The [Diagnostic] to emit
error: Option<ParserError>,
/// It tells the parser to recover if the position is inside a set of [tokens](TokenSet)
recovery: TokenSet,
/// It tells the parser to recover if the current token is a curly brace
include_braces: bool,
/// The kind of the unknown node the parser inserts if it isn't able to recover because
/// the current token is neither in the recovery set nor any of `{` or `}`.
unknown_node_kind: SyntaxKind,
}

impl ParseRecovery {
pub fn new(recovery: TokenSet, unknown_node_kind: SyntaxKind) -> Self {
Self {
error: None,
recovery,
include_braces: false,
unknown_node_kind,
}
}

pub fn with_error<Err: Into<ParserError>>(
recovery: TokenSet,
unknown_node_kind: SyntaxKind,
error: Err,
) -> Self {
Self {
error: Some(error.into()),
recovery,
include_braces: false,
unknown_node_kind,
}
}

/// Enable check of curly braces as recovery tokens
pub fn enabled_braces_check(mut self) -> Self {
self.include_braces = true;
self
}

/// The main function that tells to the parser how to recover itself.
///
/// Recover from an error with a [recovery set](TokenSet) or by using a `{` or `}`.
///
/// If [ParserRecoverer] has an error, it gets tracked in the events.
pub fn recover(&self, p: &mut Parser) {
let error = self.get_error();
if let Some(error) = error {
p.error(error);
} else {
// the check on state should be done only when there's no error
if p.state.no_recovery {
return;
}
}
if !self.parsing_is_recoverable(p) {
let m = p.start();
p.bump_any();
m.complete(p, self.get_unknown_node_kind());
}
}

/// Checks if the parsing phase is recoverable by checking curly braces and [tokens set](TokenSet)
fn parsing_is_recoverable(&self, parser: &Parser) -> bool {
self.is_at_token_set(parser) || self.is_at_braces(parser) || self.is_at_eof(parser)
}

/// It returns the diagnostic
fn get_error(&self) -> Option<Diagnostic> {
self.error.to_owned()
}

/// It returns the unknown node kind that will be used to complete the parsing
fn get_unknown_node_kind(&self) -> SyntaxKind {
self.unknown_node_kind
}

fn is_at_braces(&self, parser: &Parser) -> bool {
matches!(parser.cur(), T!['{'] | T!['}'] if self.include_braces)
}

fn is_at_token_set(&self, parser: &Parser) -> bool {
parser.at_ts(self.recovery)
}

fn is_at_eof(&self, parser: &Parser) -> bool {
parser.cur() == SyntaxKind::EOF
}
}
62 changes: 3 additions & 59 deletions crates/rslint_parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,58 +190,6 @@ impl<'t> Parser<'t> {
}
}

/// Recover from an error with a recovery set or by using a `{` or `}`.
pub fn err_recover(
&mut self,
error: impl Into<ParserError>,
recovery: TokenSet,
include_braces: bool,
) -> Option<()> {
if self.state.no_recovery {
return None;
}

match self.cur() {
T!['{'] | T!['}'] if include_braces => {
self.error(error);
return Some(());
}
_ => (),
}

if self.at_ts(recovery) || self.cur() == EOF {
self.error(error);
return Some(());
}

let m = self.start();
self.error(error);
self.bump_any();
m.complete(self, SyntaxKind::ERROR);
Some(())
}

/// Recover from an error but don't add an error to the events
pub fn err_recover_no_err(&mut self, recovery: TokenSet, include_braces: bool) {
match self.cur() {
T!['{'] | T!['}'] if include_braces => {
return;
}
EOF => {
return;
}
_ => (),
}

if self.at_ts(recovery) {
return;
}

let m = self.start();
self.bump_any();
m.complete(self, SyntaxKind::ERROR);
}

/// Starts a new node in the syntax tree. All nodes and tokens
/// consumed between the `start` and the corresponding `Marker::complete`
/// belong to the same node.
Expand Down Expand Up @@ -507,11 +455,7 @@ impl<'t> Parser<'t> {
start..end
}

pub fn expr_with_semi_recovery(
&mut self,
assign: bool,
unknown_syntax_kind: SyntaxKind,
) -> Option<CompletedMarker> {
pub fn expr_with_semi_recovery(&mut self, assign: bool) -> Option<CompletedMarker> {
let func = if assign {
syntax::expr::assign_expr
} else {
Expand All @@ -523,10 +467,10 @@ impl<'t> Parser<'t> {
let err = self
.err_builder("expected an expression, but found `;` instead")
.primary(self.cur_tok().range, "");

self.error(err);
self.bump_any();
m.complete(self, unknown_syntax_kind);
m.complete(self, SyntaxKind::JS_UNKNOWN_EXPRESSION);

return None;
}

Expand Down
44 changes: 40 additions & 4 deletions crates/rslint_parser/src/syntax/class.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::parse_recovery::ParseRecovery;
use crate::syntax::decl::{formal_param_pat, parameter_list, parameters_list};
use crate::syntax::expr::assign_expr;
use crate::syntax::function::{function_body, ts_parameter_types, ts_return_type};
Expand Down Expand Up @@ -29,6 +30,8 @@ pub(super) fn class_expression(p: &mut Parser) -> CompletedMarker {
// class extends {}
// class
// class foo { set {} }
// class A extends bar extends foo {}
// class A extends bar, foo {}
/// Parses a class declaration
pub(super) fn class_declaration(p: &mut Parser) -> CompletedMarker {
class(p, ClassKind::Declaration)
Expand Down Expand Up @@ -482,6 +485,38 @@ fn class_member(p: &mut Parser) -> CompletedMarker {
if matches!(member_name, "get" | "set") && !is_at_line_break_or_generator {
let is_getter = member_name == "get";

// test getter_class_member
// class Getters {
// get foo() {}
// get static() {}
// static get bar() {}
// get "baz"() {}
// get ["a" + "b"]() {}
// get 5() {}
// get #private() {}
// }
// class NotGetters {
// get() {}
// async get() {}
// static get() {}
// }

// test setter_class_number
// class Setters {
// set foo(a) {}
// set static(a) {}
// static set bar(a) {}
// set "baz"(a) {}
// set ["a" + "b"](a) {}
// set 5(a) {}
// set #private(a) {}
// }
// class NotSetters {
// set(a) {}
// async set(a) {}
// static set(a) {}
// }

// The tree currently holds a STATIC_MEMBER_NAME node that wraps a ident token but we now found
// out that the 'get' or 'set' isn't a member name in this context but instead are the
// 'get'/'set' keywords for getters/setters. That's why we need to undo the member name node,
Expand Down Expand Up @@ -528,11 +563,12 @@ fn class_member(p: &mut Parser) -> CompletedMarker {
let err = p
.err_builder("expected `;`, a property, or a method for a class body, but found none")
.primary(p.cur_tok().range, "");
p.err_recover(
err,
ParseRecovery::with_error(
token_set![T![;], T![ident], T![async], T![yield], T!['}'], T![#]],
false,
);
JS_UNKNOWN_MEMBER,
err,
)
.recover(p);

member_marker.complete(p, JS_UNKNOWN_MEMBER)
}
Expand Down
11 changes: 8 additions & 3 deletions crates/rslint_parser/src/syntax/decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use super::expr::assign_expr;
use super::pat::pattern;
use super::typescript::*;
use crate::parse_recovery::ParseRecovery;
use crate::syntax::function::function_body;
use crate::{SyntaxKind::*, *};

Expand Down Expand Up @@ -198,7 +199,9 @@ pub(super) fn parameters_list(
}
Some(res)
} else {
p.err_recover_no_err(
// test_err formal_params_invalid
// function (a++, c) {}
ParseRecovery::new(
token_set![
T![ident],
T![await],
Expand All @@ -208,8 +211,10 @@ pub(super) fn parameters_list(
T![...],
T![')'],
],
true,
);
JS_UNKNOWN_PATTERN,
)
.enabled_braces_check()
.recover(p);
None
}
};
Expand Down
14 changes: 11 additions & 3 deletions crates/rslint_parser/src/syntax/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use super::decl::{arrow_body, parameter_list};
use super::pat::pattern;
use super::typescript::*;
use super::util::*;
use crate::parse_recovery::ParseRecovery;
use crate::syntax::class::class_expression;
use crate::syntax::function::function_expression;
use crate::syntax::object::object_expr;
Expand Down Expand Up @@ -244,6 +245,7 @@ fn assign_expr_recursive(
// function *foo() {
// yield foo;
// yield* foo;
// yield;
// }
pub fn yield_expr(p: &mut Parser) -> CompletedMarker {
let m = p.start();
Expand Down Expand Up @@ -705,6 +707,7 @@ pub fn args(p: &mut Parser) -> CompletedMarker {

// test_err paren_or_arrow_expr_invalid_params
// (5 + 5) => {}
// (a, ,b) => {}
pub fn paren_or_arrow_expr(p: &mut Parser, can_be_arrow: bool) -> CompletedMarker {
let m = p.start();
let checkpoint = p.checkpoint();
Expand Down Expand Up @@ -749,7 +752,8 @@ pub fn paren_or_arrow_expr(p: &mut Parser, can_be_arrow: bool) -> CompletedMarke
let err = temp.err_builder(&format!("expect a closing parenthesis after a spread element, but instead found `{}`", temp.cur_src()))
.primary(temp.cur_tok().range, "");

temp.err_recover(err, EXPR_RECOVERY_SET, false);
ParseRecovery::with_error(EXPR_RECOVERY_SET, JS_UNKNOWN_PATTERN, err)
.recover(&mut temp);
}
}
break;
Expand Down Expand Up @@ -1085,7 +1089,9 @@ pub fn primary_expr(p: &mut Parser) -> Option<CompletedMarker> {
let err = p
.err_builder("Expected an expression, but found none")
.primary(p.cur_tok().range, "Expected an expression here");
p.err_recover(err, p.state.expr_recovery_set, true);
ParseRecovery::with_error(p.state.expr_recovery_set, JS_UNKNOWN_EXPRESSION, err)
.enabled_braces_check()
.recover(p);
return None;
}
};
Expand All @@ -1105,7 +1111,9 @@ pub fn reference_identifier_expression(p: &mut Parser) -> Option<CompletedMarker
.err_builder("Expected an identifier, but found none")
.primary(p.cur_tok().range, "");

p.err_recover(err, p.state.expr_recovery_set, true);
ParseRecovery::with_error(p.state.expr_recovery_set, JS_UNKNOWN_EXPRESSION, err)
.enabled_braces_check()
.recover(p);
None
}
}
Expand Down