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

feat: pass error kind via parameter #1788

Merged
merged 3 commits into from
Nov 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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/pr-number-action@v1.3

- name: Find Previous Comment
if: github.event_name == 'pull_request'
uses: peter-evans/find-comment@v1.3.0
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/create-or-update-comment@v1.4.5
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/create-or-update-comment@v1.4.5
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