Skip to content

Commit

Permalink
feat(js_parser): better diagnostic for infer type #243
Browse files Browse the repository at this point in the history
  • Loading branch information
denbezrukov committed Sep 17, 2023
1 parent f021629 commit fac7d7a
Show file tree
Hide file tree
Showing 20 changed files with 5,374 additions and 217 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/parser_conformance.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
- 'crates/biome_js_syntax/**'
- 'crates/biome_js_factory/**'
- 'crates/biome_js_semantic/**'
- 'crates/rome_js_parser/**'
- 'crates/biome_js_parser/**'
- 'crates/biome_rowan/**'
- 'xtask/**'

Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
- Fix [#294](https://github.com/biomejs/biome/issues/294). [noConfusingVoidType](https://biomejs.dev/linter/rules/no-confusing-void-type/) no longer reports false positives for return types. Contributed by @b4s36t4

### Parser

- Enhance diagnostic for infer type handling in the parser. The 'infer' keyword can only be utilized within the 'extends' clause of a conditional type. Using it outside of this context will result in an error. Ensure that any type declarations using 'infer' are correctly placed within the conditional type structure to avoid parsing issues. Contributed by @denbezrukov

### VSCode

## 1.2.2 (2023-09-16)
Expand Down
31 changes: 24 additions & 7 deletions crates/biome_js_formatter/src/ts/types/infer_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,31 @@ mod tests {

#[test]
fn needs_parentheses() {
assert_needs_parentheses!("let s: (infer string)[] = symbol();", TsInferType);
assert_needs_parentheses!(
"type A = T extends (infer string)[] ? string : never",
TsInferType
);
assert_needs_parentheses!(
"type A = T extends unique (infer string) ? string : never",
TsInferType
);

assert_needs_parentheses!("let s: unique (infer string);", TsInferType);
assert_not_needs_parentheses!(
"type A = T extends [number, ...infer string] ? string : never",
TsInferType
);
assert_needs_parentheses!(
"type A = T extends [(infer string)?] ? string : never",
TsInferType
);

assert_not_needs_parentheses!("let s: [number, ...infer string]", TsInferType);
assert_needs_parentheses!("let s: [(infer string)?]", TsInferType);

assert_needs_parentheses!("let s: (infer string)[a]", TsInferType);
assert_not_needs_parentheses!("let s: a[(infer string)]", TsInferType);
assert_needs_parentheses!(
"type A = T extends (infer string)[a] ? string : never",
TsInferType
);
assert_not_needs_parentheses!(
"type A = T extends a[(infer string)] ? string : never",
TsInferType
);
}
}
45 changes: 30 additions & 15 deletions crates/biome_js_parser/src/syntax/class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,9 +402,11 @@ fn parse_extends_clause(p: &mut JsParser) -> ParsedSyntax {
p.error(p.err_builder("'extends' list cannot be empty.", extends_end..extends_end))
} else {
TypeScript
.parse_exclusive_syntax(p, parse_ts_type_arguments, |p, arguments| {
ts_only_syntax_error(p, "type arguments", arguments.range(p))
})
.parse_exclusive_syntax(
p,
|p| parse_ts_type_arguments(p, TypeContext::default()),
|p, arguments| ts_only_syntax_error(p, "type arguments", arguments.range(p)),
)
.ok();
}

Expand All @@ -419,7 +421,7 @@ fn parse_extends_clause(p: &mut JsParser) -> ParsedSyntax {
break;
}

parse_ts_type_arguments(p).ok();
parse_ts_type_arguments(p, TypeContext::default()).ok();

let extra_class = extra.complete(p, JS_BOGUS);

Expand Down Expand Up @@ -605,6 +607,7 @@ fn parse_index_signature_class_member(p: &mut JsParser, member_marker: Marker) -
p,
member_marker,
MemberParent::Class,
TypeContext::default(),
))
},
|p, member| ts_only_syntax_error(p, "Index signatures", member.range(p)),
Expand Down Expand Up @@ -762,6 +765,7 @@ fn parse_class_member_impl(
decorator_list,
ParameterContext::ClassSetter,
ExpressionContext::default().and_object_expression_allowed(has_l_paren),
TypeContext::default(),
)
})
.or_add_diagnostic(p, js_parse_error::expected_parameter);
Expand All @@ -771,7 +775,9 @@ fn parse_class_member_impl(
// class Test {
// set a(value: string): void {}
// }
if let Present(return_type_annotation) = parse_ts_return_type_annotation(p) {
if let Present(return_type_annotation) =
parse_ts_return_type_annotation(p, TypeContext::default())
{
p.error(ts_set_accessor_return_type_error(
p,
&return_type_annotation,
Expand Down Expand Up @@ -1124,17 +1130,17 @@ fn parse_ts_property_annotation(

let mut annotation = match (optional_range, definite_range) {
(Some(_), None) => {
parse_ts_type_annotation(p).ok();
parse_ts_type_annotation(p, TypeContext::default()).ok();
m.complete(p, TS_OPTIONAL_PROPERTY_ANNOTATION)
}
(None, Some(_)) => {
parse_ts_type_annotation(p).or_add_diagnostic(p, |p, range| {
parse_ts_type_annotation(p, TypeContext::default()).or_add_diagnostic(p, |p, range| {
p.err_builder("Properties with definite assignment assertions must also have type annotations.",range, )
});
m.complete(p, TS_DEFINITE_PROPERTY_ANNOTATION)
}
(Some(optional_range), Some(definite_range)) => {
parse_ts_type_annotation(p).ok();
parse_ts_type_annotation(p, TypeContext::default()).ok();
let error = p
.err_builder(
"class properties cannot be both optional and definite",
Expand Down Expand Up @@ -1246,13 +1252,15 @@ fn parse_method_class_member_rest(
ParameterContext::ClassImplementation
};

parse_parameter_list(p, parameter_context, flags)
parse_parameter_list(p, parameter_context, TypeContext::default(), flags)
.or_add_diagnostic(p, js_parse_error::expected_class_parameters);

TypeScript
.parse_exclusive_syntax(p, parse_ts_return_type_annotation, |p, annotation| {
ts_only_syntax_error(p, "return type annotation", annotation.range(p))
})
.parse_exclusive_syntax(
p,
|p| parse_ts_return_type_annotation(p, TypeContext::default()),
|p, annotation| ts_only_syntax_error(p, "return type annotation", annotation.range(p)),
)
.ok();

let member_kind = expect_method_body(p, &m, modifiers, ClassMethodMemberKind::Method(flags));
Expand Down Expand Up @@ -1523,7 +1531,7 @@ fn parse_constructor_class_member_body(
parse_constructor_parameter_list(p)
.or_add_diagnostic(p, js_parse_error::expected_constructor_parameters);

if let Present(marker) = parse_ts_type_annotation(p) {
if let Present(marker) = parse_ts_type_annotation(p, TypeContext::default()) {
let err = p.err_builder("constructors cannot have type annotations", marker.range(p));

p.error(err);
Expand Down Expand Up @@ -1612,8 +1620,14 @@ fn parse_constructor_parameter(p: &mut JsParser, context: ExpressionContext) ->
let modifiers = parse_class_member_modifiers(p, true);

// we pass decorator list as Absent because TsPropertyParameter has its own decorator list
parse_formal_parameter(p, Absent, ParameterContext::ParameterProperty, context)
.or_add_diagnostic(p, expected_binding);
parse_formal_parameter(
p,
Absent,
ParameterContext::ParameterProperty,
context,
TypeContext::default(),
)
.or_add_diagnostic(p, expected_binding);

let kind = if modifiers.validate_and_complete(p, TS_PROPERTY_PARAMETER) {
TS_PROPERTY_PARAMETER
Expand All @@ -1628,6 +1642,7 @@ fn parse_constructor_parameter(p: &mut JsParser, context: ExpressionContext) ->
decorator_list,
ParameterContext::ClassImplementation,
context,
TypeContext::default(),
)
.map(|mut parameter| {
// test_err ts ts_constructor_this_parameter
Expand Down
97 changes: 66 additions & 31 deletions crates/biome_js_parser/src/syntax/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,16 +255,20 @@ fn parse_function(p: &mut JsParser, m: Marker, kind: FunctionKind) -> CompletedM
ParameterContext::Implementation
};

parse_parameter_list(p, parameter_context, flags)
parse_parameter_list(p, parameter_context, TypeContext::default(), flags)
.or_add_diagnostic(p, js_parse_error::expected_parameters);

TypeScript
.parse_exclusive_syntax(p, parse_ts_return_type_annotation, |p, marker| {
p.err_builder(
"return types can only be used in TypeScript files",
marker.range(p),
)
})
.parse_exclusive_syntax(
p,
|p| parse_ts_return_type_annotation(p, TypeContext::default()),
|p, marker| {
p.err_builder(
"return types can only be used in TypeScript files",
marker.range(p),
)
},
)
.ok();

let body = parse_function_body(p, flags);
Expand Down Expand Up @@ -416,9 +420,14 @@ fn parse_ambient_function(
let binding_range = p.cur_range();

parse_ts_type_parameters(p, TypeContext::default().and_allow_const_modifier(true)).ok();
parse_parameter_list(p, ParameterContext::Declaration, SignatureFlags::empty())
.or_add_diagnostic(p, expected_parameters);
parse_ts_return_type_annotation(p).ok();
parse_parameter_list(
p,
ParameterContext::Declaration,
TypeContext::default(),
SignatureFlags::empty(),
)
.or_add_diagnostic(p, expected_parameters);
parse_ts_return_type_annotation(p, TypeContext::default()).ok();

if let Present(body) = parse_function_body(p, SignatureFlags::empty()) {
p.error(
Expand Down Expand Up @@ -460,13 +469,17 @@ fn parse_ambient_function(
}

pub(crate) fn parse_ts_type_annotation_or_error(p: &mut JsParser) -> ParsedSyntax {
TypeScript.parse_exclusive_syntax(p, parse_ts_type_annotation, |p, annotation| {
p.err_builder(
"return types can only be used in TypeScript files",
annotation.range(p),
)
.hint("remove this type annotation")
})
TypeScript.parse_exclusive_syntax(
p,
|p| parse_ts_type_annotation(p, TypeContext::default()),
|p, annotation| {
p.err_builder(
"return types can only be used in TypeScript files",
annotation.range(p),
)
.hint("remove this type annotation")
},
)
}

/// Tells [is_at_async_function] if it needs to check line breaks
Expand Down Expand Up @@ -562,6 +575,7 @@ fn try_parse_parenthesized_arrow_function_head(
parse_parameter_list(
p,
ParameterContext::Arrow,
TypeContext::default(),
arrow_function_parameter_flags(p, flags),
)
.or_add_diagnostic(p, expected_parameters);
Expand All @@ -571,9 +585,11 @@ fn try_parse_parenthesized_arrow_function_head(
}

TypeScript
.parse_exclusive_syntax(p, parse_ts_return_type_annotation, |p, annotation| {
ts_only_syntax_error(p, "return type annotation", annotation.range(p))
})
.parse_exclusive_syntax(
p,
|p| parse_ts_return_type_annotation(p, TypeContext::default()),
|p, annotation| ts_only_syntax_error(p, "return type annotation", annotation.range(p)),
)
.ok();

if p.has_preceding_line_break() {
Expand Down Expand Up @@ -884,6 +900,7 @@ pub(crate) fn parse_any_parameter(
decorator_list: ParsedSyntax,
parameter_context: ParameterContext,
expression_context: ExpressionContext,
type_context: TypeContext,
) -> ParsedSyntax {
let parameter = match p.cur() {
T![...] => parse_rest_parameter(p, decorator_list, expression_context),
Expand All @@ -900,9 +917,15 @@ pub(crate) fn parse_any_parameter(
decorator_list.change_to_bogus(p);
decorator_list
});
parse_ts_this_parameter(p)
parse_ts_this_parameter(p, type_context)
}
_ => parse_formal_parameter(p, decorator_list, parameter_context, expression_context),
_ => parse_formal_parameter(
p,
decorator_list,
parameter_context,
expression_context,
type_context,
),
};

parameter.map(|mut parameter| {
Expand Down Expand Up @@ -955,9 +978,11 @@ pub(crate) fn parse_rest_parameter(

// type annotation `...foo: number[]`
TypeScript
.parse_exclusive_syntax(p, parse_ts_type_annotation, |p, annotation| {
ts_only_syntax_error(p, "type annotation", annotation.range(p))
})
.parse_exclusive_syntax(
p,
|p| parse_ts_type_annotation(p, TypeContext::default()),
|p, annotation| ts_only_syntax_error(p, "type annotation", annotation.range(p)),
)
.ok();

if let Present(initializer) = parse_initializer_clause(p, ExpressionContext::default()) {
Expand Down Expand Up @@ -994,14 +1019,14 @@ pub(crate) fn parse_rest_parameter(
// test ts ts_this_parameter
// function a(this) {}
// function b(this: string) {}
pub(crate) fn parse_ts_this_parameter(p: &mut JsParser) -> ParsedSyntax {
pub(crate) fn parse_ts_this_parameter(p: &mut JsParser, context: TypeContext) -> ParsedSyntax {
if !p.at(T![this]) {
return Absent;
}

let parameter = p.start();
p.expect(T![this]);
parse_ts_type_annotation(p).ok();
parse_ts_type_annotation(p, context).ok();
Present(parameter.complete(p, TS_THIS_PARAMETER))
}

Expand Down Expand Up @@ -1077,6 +1102,7 @@ pub(crate) fn parse_formal_parameter(
decorator_list: ParsedSyntax,
parameter_context: ParameterContext,
expression_context: ExpressionContext,
type_context: TypeContext,
) -> ParsedSyntax {
// test ts ts_formal_parameter_decorator { "parse_class_parameter_decorators": true }
// class Foo {
Expand Down Expand Up @@ -1149,9 +1175,11 @@ pub(crate) fn parse_formal_parameter(
}

TypeScript
.parse_exclusive_syntax(p, parse_ts_type_annotation, |p, annotation| {
ts_only_syntax_error(p, "Type annotations", annotation.range(p))
})
.parse_exclusive_syntax(
p,
|p| parse_ts_type_annotation(p, type_context),
|p, annotation| ts_only_syntax_error(p, "Type annotations", annotation.range(p)),
)
.ok();

if let Present(initializer) = parse_initializer_clause(p, expression_context) {
Expand Down Expand Up @@ -1208,6 +1236,7 @@ pub(super) fn skip_parameter_start(p: &mut JsParser) -> bool {
pub(super) fn parse_parameter_list(
p: &mut JsParser,
parameter_context: ParameterContext,
type_context: TypeContext,
flags: SignatureFlags,
) -> ParsedSyntax {
if !p.at(T!['(']) {
Expand Down Expand Up @@ -1288,7 +1317,13 @@ pub(super) fn parse_parameter_list(
.into()
};

parse_any_parameter(p, decorator_list, parameter_context, expression_context)
parse_any_parameter(
p,
decorator_list,
parameter_context,
expression_context,
type_context,
)
},
JS_PARAMETER_LIST,
);
Expand Down
3 changes: 2 additions & 1 deletion crates/biome_js_parser/src/syntax/jsx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::syntax::jsx::jsx_parse_errors::{
jsx_expected_attribute, jsx_expected_attribute_value, jsx_expected_children,
jsx_expected_closing_tag,
};
use crate::syntax::typescript::TypeContext;
use crate::JsSyntaxFeature::TypeScript;
use crate::{parser::RecoveryResult, JsParser, ParseRecovery, ParsedSyntax};
use crate::{Absent, Present};
Expand Down Expand Up @@ -158,7 +159,7 @@ fn parse_any_jsx_opening_tag(p: &mut JsParser, in_expression: bool) -> Option<Op
// <NonGeneric />;
// <Generic<true> />;
// <Generic<true>></Generic>;
let _ = parse_ts_type_arguments(p);
let _ = parse_ts_type_arguments(p, TypeContext::default());
}

JsxAttributeList.parse_list(p);
Expand Down
Loading

0 comments on commit fac7d7a

Please sign in to comment.