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

Commit

Permalink
feat: pass error kind via parameter (#1788)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Nov 16, 2021
1 parent 30b8e43 commit 08ba1bc
Show file tree
Hide file tree
Showing 12 changed files with 220 additions and 110 deletions.
64 changes: 53 additions & 11 deletions .github/workflows/test262.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [windows-latest, macos-latest, ubuntu-latest]
os: [windows-latest, macos-latest, ubuntu-latest]
steps:
- name: Checkout repository
uses: actions/checkout@v2
Expand All @@ -33,31 +33,28 @@ jobs:
uses: Swatinem/rust-cache@v1
- name: Run Test262 suite
continue-on-error: true
run: cargo xtask coverage --json
- name: Rename the emitted file
run: |
mv base_results.json new_results.json
run: cargo xtask coverage --json > new_results.json
- name: Save test results
uses: actions/upload-artifact@v2
with:
name: new_results
path: new_results.json

compare:
# This job compares the coverage results from this PR and the coverage results from master
coverage-comparison:
needs: coverage
name: Compare coverage
runs-on: ${{ matrix.os }}
strategy:
fail-fast: false
matrix:
os: [windows-latest, macos-latest, ubuntu-latest]
os: [windows-latest, macos-latest, ubuntu-latest]
steps:
- name: Checkout repository
uses: actions/checkout@v2
with:
submodules: recursive
# To restore once the main branch has the new command
# ref: main
ref: main
- name: Install toolchain
uses: actions-rs/toolchain@v1
with:
Expand All @@ -67,10 +64,55 @@ jobs:
uses: Swatinem/rust-cache@v1
- name: Run Test262 suite on main branch
continue-on-error: true
run: cargo xtask coverage --json
run: cargo xtask coverage --json > base_results.json
- name: Download PRs test results
uses: actions/download-artifact@v2
with:
name: new_results
- name: Compare results on ${{ matrix.os }}
run: cargo xtask compare ./base_results.json ./new_results.json --markdown
if: github.event_name == 'pull_request'
id: comparison
shell: bash
run: |
comment="$(cargo xtask compare ./base_results.json ./new_results.json --markdown)"
comment="${comment//'%'/'%25'}"
comment="${comment//$'\n'/'%0A'}"
comment="${comment//$'\r'/'%0D'}"
echo "::set-output name=comment::$comment"
- name: Get the PR number
if: github.event_name == 'pull_request'
id: pr-number
uses: kkak10/[email protected]

- name: Find Previous Comment
if: github.event_name == 'pull_request'
uses: peter-evans/[email protected]
id: previous-comment
with:
issue-number: ${{ steps.pr-number.outputs.pr }}
body-includes: Test262 conformance changes

- name: Update comment
if: github.event_name == 'pull_request' && steps.previous-comment.outputs.comment-id
uses: peter-evans/[email protected]
continue-on-error: true
with:
comment-id: ${{ steps.previous-comment.outputs.comment-id }}
body: |
### Test262 comparison coverage results on ${{ matrix.os }}
${{ steps.comparison.outputs.comment }}
edit-mode: replace

- name: Write a new comment
if: github.event_name == 'pull_request' && !steps.previous-comment.outputs.comment-id
uses: peter-evans/[email protected]
continue-on-error: true
with:
issue-number: ${{ steps.pr-number.outputs.pr }}
body: |
### Test262 comparison coverage results on ${{ matrix.os }}
${{ steps.comparison.outputs.comment }}
50 changes: 40 additions & 10 deletions crates/rslint_parser/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,20 @@ impl<'t> Parser<'t> {
}

/// Recover from an error with a recovery set or by using a `{` or `}`.
///
/// # Arguments
///
/// * `error` - the [Diagnostic] to emit
/// * `recovery` - it recovers the parser position is inside a set [tokens](TokenSet)
/// * `include_braces` - it recovers the parser if the current token is a curly brace
/// * `unknown_node` - 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 `}`.
pub fn err_recover(
&mut self,
error: impl Into<ParserError>,
recovery: TokenSet,
include_braces: bool,
unknown_node: SyntaxKind,
) -> Option<()> {
if self.state.no_recovery {
return None;
Expand All @@ -206,12 +215,24 @@ impl<'t> Parser<'t> {
let m = self.start();
self.error(error);
self.bump_any();
m.complete(self, SyntaxKind::ERROR);
m.complete(self, unknown_node);
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) {
///
/// # Arguments
///
/// * `recovery` - it recovers the parser position is inside a set [tokens](TokenSet)
/// * `include_braces` - it recovers the parser if the current token is a curly brace
/// * `unknown_node` - 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 `}`.
pub fn err_recover_no_err(
&mut self,
recovery: TokenSet,
include_braces: bool,
unknown_node: SyntaxKind,
) {
match self.cur() {
T!['{'] | T!['}'] if include_braces => {
return;
Expand All @@ -225,7 +246,7 @@ impl<'t> Parser<'t> {

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

/// Starts a new node in the syntax tree. All nodes and tokens
Expand Down Expand Up @@ -423,19 +444,24 @@ impl<'t> Parser<'t> {
}

/// Bump and add an error event
pub fn err_and_bump(&mut self, err: impl Into<ParserError>) {
pub fn err_and_bump(&mut self, err: impl Into<ParserError>, unknown_syntax_kind: SyntaxKind) {
let m = self.start();
self.bump_any();
m.complete(self, SyntaxKind::ERROR);
m.complete(self, unknown_syntax_kind);
self.error(err);
}

pub fn err_if_not_ts(&mut self, mut marker: impl BorrowMut<CompletedMarker>, err: &str) {
pub fn err_if_not_ts(
&mut self,
mut marker: impl BorrowMut<CompletedMarker>,
err: &str,
unknown_syntax_kind: SyntaxKind,
) {
if self.typescript() {
return;
}
let borrow = marker.borrow_mut();
borrow.change_kind(self, SyntaxKind::ERROR);
borrow.change_kind(self, unknown_syntax_kind);
let err = self.err_builder(err).primary(borrow.range(self), "");

self.error(err);
Expand Down Expand Up @@ -489,7 +515,11 @@ impl<'t> Parser<'t> {
start..end
}

pub fn expr_with_semi_recovery(&mut self, assign: bool) -> Option<CompletedMarker> {
pub fn expr_with_semi_recovery(
&mut self,
assign: bool,
unknown_syntax_kind: SyntaxKind,
) -> Option<CompletedMarker> {
let func = if assign {
syntax::expr::assign_expr
} else {
Expand All @@ -504,7 +534,7 @@ impl<'t> Parser<'t> {

self.error(err);
self.bump_any();
m.complete(self, SyntaxKind::ERROR);
m.complete(self, unknown_syntax_kind);
return None;
}

Expand Down Expand Up @@ -715,7 +745,7 @@ impl CompletedMarker {
}

pub fn err_if_not_ts(&mut self, p: &mut Parser, err: &str) {
p.err_if_not_ts(self, err);
p.err_if_not_ts(self, err, SyntaxKind::ERROR);
}
}

Expand Down
3 changes: 3 additions & 0 deletions crates/rslint_parser/src/syntax/decl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ fn parameters_common(p: &mut Parser, constructor_params: bool) -> CompletedMarke
T![')'],
],
true,
ERROR,
);
None
}
Expand Down Expand Up @@ -963,6 +964,7 @@ fn class_member_no_semi(p: &mut Parser) -> Option<CompletedMarker> {
err,
token_set![T![;], T![ident], T![async], T![yield], T!['}'], T![#]],
false,
ERROR,
);
None
}
Expand Down Expand Up @@ -1061,6 +1063,7 @@ pub fn method(
err,
recovery_set.into().unwrap_or(BASE_METHOD_RECOVERY_SET),
false,
ERROR,
);
return None;
}
Expand Down
10 changes: 5 additions & 5 deletions crates/rslint_parser/src/syntax/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ 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);
temp.err_recover(err, EXPR_RECOVERY_SET, false, ERROR);
}
}
break;
Expand Down Expand Up @@ -1012,7 +1012,7 @@ pub fn primary_expr(p: &mut Parser) -> Option<CompletedMarker> {
))
.primary(p.cur_tok().range, "");

p.err_and_bump(err);
p.err_and_bump(err, ERROR);
m.complete(p, ERROR)
} else {
let err = p
Expand Down Expand Up @@ -1047,7 +1047,7 @@ 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);
p.err_recover(err, p.state.expr_recovery_set, true, ERROR);
return None;
}
};
Expand All @@ -1067,7 +1067,7 @@ pub fn identifier_reference(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);
p.err_recover(err, p.state.expr_recovery_set, true, ERROR);
None
}
}
Expand Down Expand Up @@ -1263,7 +1263,7 @@ pub fn object_property(p: &mut Parser) -> Option<CompletedMarker> {
// let a = { /: 6, /: /foo/ }
// let a = {{}}
if prop.is_none() {
p.err_recover_no_err(token_set![T![:], T![,]], false);
p.err_recover_no_err(token_set![T![:], T![,]], false, ERROR);
}
// test_err object_expr_non_ident_literal_prop
// let b = {5}
Expand Down
4 changes: 3 additions & 1 deletion crates/rslint_parser/src/syntax/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ pub fn pattern(p: &mut Parser, parameters: bool, assignment: bool) -> Option<Com
if p.state.allow_object_expr {
ts = ts.union(token_set![T!['{']]);
}
p.err_recover(err, ts, false);
p.err_recover(err, ts, false, ERROR);
return None;
}
})
Expand Down Expand Up @@ -171,6 +171,7 @@ pub fn array_binding_pattern(
p.err_recover_no_err(
token_set![T![await], T![ident], T![yield], T![:], T![=], T![']']],
false,
ERROR,
);
}
if !p.at(T![']']) {
Expand Down Expand Up @@ -239,6 +240,7 @@ fn object_binding_prop(p: &mut Parser, parameters: bool) -> Option<CompletedMark
p.err_recover_no_err(
token_set![T![await], T![ident], T![yield], T![:], T![=], T!['}']],
false,
ERROR,
);
return None;
};
Expand Down
21 changes: 13 additions & 8 deletions crates/rslint_parser/src/syntax/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,16 @@ pub fn stmt(p: &mut Parser, recovery_set: impl Into<Option<TokenSet>>) -> Option

// We must explicitly handle this case or else infinite recursion can happen
if p.at_ts(token_set![T!['}'], T![import], T![export]]) {
p.err_and_bump(err);
p.err_and_bump(err, ERROR);
return None;
}

p.err_recover(err, recovery_set.into().unwrap_or(STMT_RECOVERY_SET), false);
p.err_recover(
err,
recovery_set.into().unwrap_or(STMT_RECOVERY_SET),
false,
ERROR,
);
return None;
}
};
Expand Down Expand Up @@ -171,7 +176,7 @@ fn expr_stmt(p: &mut Parser) -> Option<CompletedMarker> {
m.complete(p, ERROR);
}

let mut expr = p.expr_with_semi_recovery(false)?;
let mut expr = p.expr_with_semi_recovery(false, ERROR)?;
// Labelled stmt
if expr.kind() == JS_REFERENCE_IDENTIFIER_EXPRESSION && p.at(T![:]) {
expr.change_kind(p, NAME);
Expand Down Expand Up @@ -255,7 +260,7 @@ pub fn throw_stmt(p: &mut Parser) -> CompletedMarker {

p.error(err);
} else {
p.expr_with_semi_recovery(false);
p.expr_with_semi_recovery(false, ERROR);
}
semi(p, start..p.cur_tok().range.end);
m.complete(p, JS_THROW_STATEMENT)
Expand Down Expand Up @@ -347,7 +352,7 @@ pub fn return_stmt(p: &mut Parser) -> CompletedMarker {
let start = p.cur_tok().range.start;
p.expect(T![return]);
if !p.has_linebreak_before_n(0) && p.at_ts(STARTS_EXPR) {
p.expr_with_semi_recovery(false);
p.expr_with_semi_recovery(false, ERROR);
}
semi(p, start..p.cur_tok().range.end);
let complete = m.complete(p, JS_RETURN_STATEMENT);
Expand Down Expand Up @@ -779,13 +784,13 @@ fn variable_initializer(p: &mut Parser) {
let m = p.start();

p.expect(T![=]);
p.expr_with_semi_recovery(true);
p.expr_with_semi_recovery(true, ERROR);

m.complete(p, SyntaxKind::JS_EQUAL_VALUE_CLAUSE);
}

// A do.. while statement, such as `do {} while (true)`
// test do_while_stmt
// test do_while_stmtR
// do { } while (true)
// do throw Error("foo") while (true)
pub fn do_stmt(p: &mut Parser) -> CompletedMarker {
Expand Down Expand Up @@ -969,7 +974,7 @@ fn switch_clause(p: &mut Parser) -> Option<Range<usize>> {
"Expected the start to a case or default clause here",
);

p.err_recover(err, STMT_RECOVERY_SET, true);
p.err_recover(err, STMT_RECOVERY_SET, true, ERROR);
}
}
None
Expand Down
Loading

0 comments on commit 08ba1bc

Please sign in to comment.