Skip to content

Commit b34c6f6

Browse files
BoshenBoshen
committed
perf(parser,semantic): improve handling of diagnostics (#11641)
* Move some checks from semantic to parser * Remove redundant indirections
1 parent 78f1336 commit b34c6f6

File tree

19 files changed

+312
-481
lines changed

19 files changed

+312
-481
lines changed

‎crates/oxc_linter/src/rules/eslint/no_unused_vars/tests/typescript_eslint.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ fn test() {
3737
import { AccessorDecorator } from 'decorators';
3838
export class Foo {
3939
@AccessorDecorator
40-
set bar() {}
40+
set bar(_foo) {}
4141
}
4242
",
4343
None,

‎crates/oxc_linter/src/rules/typescript/explicit_function_return_type.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ fn test() {
766766
get prop(): number {
767767
return 1;
768768
}
769-
set prop() {}
769+
set prop(_foo) {}
770770
method(): void {
771771
return;
772772
}
@@ -1312,7 +1312,7 @@ fn test() {
13121312
get prop() {
13131313
return 1;
13141314
}
1315-
set prop() {}
1315+
set prop(_foo) {}
13161316
method() {
13171317
return;
13181318
}
@@ -1604,7 +1604,7 @@ fn test() {
16041604
get prop() {
16051605
return 1;
16061606
}
1607-
set prop() {}
1607+
set prop(_foo) {}
16081608
method() {
16091609
return;
16101610
}

‎crates/oxc_linter/src/rules/unicorn/no_accessor_recursion.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ fn test() {
403403
let fail = vec![
404404
r"
405405
const foo = {
406-
get bar(value) {
406+
get bar() {
407407
this.bar
408408
}
409409
};

‎crates/oxc_linter/src/snapshots/typescript_explicit_function_return_type.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ source: crates/oxc_linter/src/tester.rs
4848

4949
⚠ typescript-eslint(explicit-function-return-type): Missing return type on function.
5050
╭─[explicit_function_return_type.tsx:8:12]
51-
7 │ set prop() {}
51+
7 │ set prop(_foo) {}
5252
8 │ method() {
5353
· ──────
5454
9 │ return;

‎crates/oxc_linter/src/snapshots/unicorn_no_accessor_recursion.snap‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ source: crates/oxc_linter/src/tester.rs
33
---
44
⚠ eslint-plugin-unicorn(no-accessor-recursion): Disallow recursive access to `this` within getters.
55
╭─[no_accessor_recursion.tsx:4:21]
6-
3 │ get bar(value) {
6+
3 │ get bar() {
77
4 │ this.bar
88
· ────────
99
5 │ }

‎crates/oxc_parser/src/diagnostics.rs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -709,3 +709,49 @@ pub fn interface_extend(span: Span) -> OxcDiagnostic {
709709
)
710710
.with_label(span)
711711
}
712+
713+
#[cold]
714+
pub fn reg_exp_flag_u_and_v(span: Span) -> OxcDiagnostic {
715+
OxcDiagnostic::error(
716+
"The 'u' and 'v' regular expression flags cannot be enabled at the same time",
717+
)
718+
.with_label(span)
719+
}
720+
721+
#[cold]
722+
pub fn setter_with_parameters(span: Span) -> OxcDiagnostic {
723+
OxcDiagnostic::error("A 'set' accessor must have exactly one parameter.").with_label(span)
724+
}
725+
726+
#[cold]
727+
pub fn setter_with_rest_parameter(span: Span) -> OxcDiagnostic {
728+
OxcDiagnostic::error("A 'set' accessor cannot have rest parameter.").with_label(span)
729+
}
730+
731+
#[cold]
732+
pub fn getter_parameters(span: Span) -> OxcDiagnostic {
733+
OxcDiagnostic::error("A 'get' accessor must not have any formal parameters.").with_label(span)
734+
}
735+
736+
#[cold]
737+
pub fn variable_declarator_definite(span: Span) -> OxcDiagnostic {
738+
ts_error(
739+
"1263",
740+
"Declarations with initializers cannot also have definite assignment assertions.",
741+
)
742+
.with_label(span)
743+
}
744+
745+
#[cold]
746+
pub fn variable_declarator_definite_type_assertion(span: Span) -> OxcDiagnostic {
747+
ts_error(
748+
"1264",
749+
"Declarations with definite assignment assertions must also have type annotations.",
750+
)
751+
.with_label(span)
752+
}
753+
754+
#[cold]
755+
pub fn invalid_rest_assignment_target(span: Span) -> OxcDiagnostic {
756+
OxcDiagnostic::error("Invalid rest operator's argument.").with_label(span)
757+
}

‎crates/oxc_parser/src/js/class.rs‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,27 @@ impl<'a> ParserImpl<'a> {
591591
)
592592
}
593593

594+
pub(crate) fn check_getter(&mut self, function: &Function<'a>) {
595+
if !function.params.items.is_empty() {
596+
self.error(diagnostics::getter_parameters(function.params.span));
597+
}
598+
}
599+
600+
pub(crate) fn check_setter(&mut self, function: &Function<'a>) {
601+
if let Some(rest) = &function.params.rest {
602+
self.error(diagnostics::setter_with_rest_parameter(rest.span));
603+
} else if function.params.parameters_count() != 1 {
604+
self.error(diagnostics::setter_with_parameters(function.params.span));
605+
}
606+
}
607+
594608
fn check_method_definition(&mut self, method: &MethodDefinition<'a>) {
609+
let function = &method.value;
610+
match method.kind {
611+
MethodDefinitionKind::Get => self.check_getter(function),
612+
MethodDefinitionKind::Set => self.check_setter(function),
613+
_ => {}
614+
}
595615
if !method.computed {
596616
if let Some((name, span)) = method.key.prop_name() {
597617
if method.r#static && name == "prototype" && !self.ctx.has_ambient() {

‎crates/oxc_parser/src/js/declaration.rs‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,27 +119,44 @@ impl<'a> ParserImpl<'a> {
119119
let (id, definite) = if self.is_ts {
120120
// const x!: number = 1
121121
// ^ definite
122-
let mut definite = false;
123-
if binding_kind.is_binding_identifier()
122+
let definite = if binding_kind.is_binding_identifier()
124123
&& !self.cur_token().is_on_new_line()
125-
&& self.eat(Kind::Bang)
124+
&& self.at(Kind::Bang)
126125
{
127-
definite = true;
128-
}
129-
let optional = self.eat(Kind::Question); // not allowed, but checked in checker/typescript.rs
126+
let span = self.cur_token().span();
127+
self.bump_any();
128+
Some(span)
129+
} else {
130+
None
131+
};
132+
let optional = if self.at(Kind::Question) {
133+
self.error(diagnostics::unexpected_token(self.cur_token().span()));
134+
self.bump_any();
135+
true
136+
} else {
137+
false
138+
};
130139
let type_annotation = self.parse_ts_type_annotation();
131140
if let Some(type_annotation) = &type_annotation {
132141
Self::extend_binding_pattern_span_end(type_annotation.span.end, &mut binding_kind);
133142
}
134143
(self.ast.binding_pattern(binding_kind, type_annotation, optional), definite)
135144
} else {
136-
(self.ast.binding_pattern(binding_kind, NONE, false), false)
145+
(self.ast.binding_pattern(binding_kind, NONE, false), None)
137146
};
138147
let init = self.eat(Kind::Eq).then(|| self.parse_assignment_expression_or_higher());
139-
let decl = self.ast.variable_declarator(self.end_span(span), kind, id, init, definite);
148+
let decl =
149+
self.ast.variable_declarator(self.end_span(span), kind, id, init, definite.is_some());
140150
if decl_parent == VariableDeclarationParent::Statement {
141151
self.check_missing_initializer(&decl);
142152
}
153+
if let Some(span) = definite {
154+
if decl.init.is_some() {
155+
self.error(diagnostics::variable_declarator_definite(span));
156+
} else if decl.id.type_annotation.is_none() {
157+
self.error(diagnostics::variable_declarator_definite_type_assertion(span));
158+
}
159+
}
143160
decl
144161
}
145162

‎crates/oxc_parser/src/js/expression.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,10 @@ impl<'a> ParserImpl<'a> {
372372

373373
let pattern = RegExpPattern { text: Atom::from(pattern_text), pattern };
374374

375+
if flags.contains(RegExpFlags::U | RegExpFlags::V) {
376+
self.error(diagnostics::reg_exp_flag_u_and_v(span));
377+
}
378+
375379
self.ast.reg_exp_literal(span, RegExp { pattern, flags }, Some(Atom::from(raw)))
376380
}
377381

‎crates/oxc_parser/src/js/grammar.rs‎

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,21 @@ impl<'a> CoverGrammar<'a, ArrayExpression<'a>> for ArrayAssignmentTarget<'a> {
7575
}
7676
ArrayExpressionElement::SpreadElement(elem) => {
7777
if i == len - 1 {
78-
rest = Some(p.ast.assignment_target_rest(
79-
elem.span,
80-
AssignmentTarget::cover(elem.unbox().argument, p),
81-
));
78+
let span = elem.span;
79+
let argument = elem.unbox().argument;
80+
if !matches!(
81+
argument.get_inner_expression(),
82+
Expression::Identifier(_)
83+
| Expression::ArrayExpression(_)
84+
| Expression::ObjectExpression(_)
85+
| Expression::StaticMemberExpression(_)
86+
| Expression::ComputedMemberExpression(_)
87+
| Expression::PrivateFieldExpression(_)
88+
) {
89+
p.error(diagnostics::invalid_rest_assignment_target(argument.span()));
90+
}
91+
let target = AssignmentTarget::cover(argument, p);
92+
rest = Some(p.ast.assignment_target_rest(span, target));
8293
if let Some(span) = p.state.trailing_commas.get(&expr.span.start) {
8394
p.error(diagnostics::rest_element_trailing_comma(*span));
8495
}
@@ -130,10 +141,19 @@ impl<'a> CoverGrammar<'a, ObjectExpression<'a>> for ObjectAssignmentTarget<'a> {
130141
}
131142
ObjectPropertyKind::SpreadProperty(spread) => {
132143
if i == len - 1 {
133-
rest = Some(p.ast.assignment_target_rest(
134-
spread.span,
135-
AssignmentTarget::cover(spread.unbox().argument, p),
136-
));
144+
let span = spread.span;
145+
let argument = spread.unbox().argument;
146+
if !matches!(
147+
argument.get_inner_expression(),
148+
Expression::Identifier(_)
149+
| Expression::StaticMemberExpression(_)
150+
| Expression::ComputedMemberExpression(_)
151+
| Expression::PrivateFieldExpression(_)
152+
) {
153+
p.error(diagnostics::invalid_rest_assignment_target(argument.span()));
154+
}
155+
let target = AssignmentTarget::cover(argument, p);
156+
rest = Some(p.ast.assignment_target_rest(span, target));
137157
} else {
138158
return p.fatal_error(diagnostics::spread_last_element(spread.span));
139159
}

0 commit comments

Comments
 (0)