Skip to content

Commit 389323c

Browse files
VeykrilVeykril
committed
minor: Simplify impl-ty parse validation
1 parent 749fde9 commit 389323c

File tree

4 files changed

+74
-55
lines changed

4 files changed

+74
-55
lines changed

‎crates/syntax/src/validation.rs‎

Lines changed: 71 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
55
mod block;
66

7+
use itertools::Itertools;
78
use rowan::Direction;
89
use rustc_lexer::unescape::{self, Mode, unescape_mixed, unescape_unicode};
910

@@ -37,7 +38,8 @@ pub(crate) fn validate(root: &SyntaxNode, errors: &mut Vec<SyntaxError>) {
3738
ast::FnPtrType(it) => validate_trait_object_fn_ptr_ret_ty(it, errors),
3839
ast::MacroRules(it) => validate_macro_rules(it, errors),
3940
ast::LetExpr(it) => validate_let_expr(it, errors),
40-
ast::ImplTraitType(it) => validate_impl_object_ty(it, errors),
41+
ast::DynTraitType(it) => errors.extend(validate_trait_object_ty(it)),
42+
ast::ImplTraitType(it) => errors.extend(validate_impl_object_ty(it)),
4143
_ => (),
4244
}
4345
}
@@ -316,87 +318,104 @@ fn validate_path_keywords(segment: ast::PathSegment, errors: &mut Vec<SyntaxErro
316318
}
317319

318320
fn validate_trait_object_ref_ty(ty: ast::RefType, errors: &mut Vec<SyntaxError>) {
319-
if let Some(ast::Type::DynTraitType(ty)) = ty.ty() {
320-
if let Some(err) = validate_trait_object_ty(ty) {
321-
errors.push(err);
321+
match ty.ty() {
322+
Some(ast::Type::DynTraitType(ty)) => {
323+
if let Some(err) = validate_trait_object_ty_plus(ty) {
324+
errors.push(err);
325+
}
326+
}
327+
Some(ast::Type::ImplTraitType(ty)) => {
328+
if let Some(err) = validate_impl_object_ty_plus(ty) {
329+
errors.push(err);
330+
}
322331
}
332+
_ => (),
323333
}
324334
}
325335

326336
fn validate_trait_object_ptr_ty(ty: ast::PtrType, errors: &mut Vec<SyntaxError>) {
327-
if let Some(ast::Type::DynTraitType(ty)) = ty.ty() {
328-
if let Some(err) = validate_trait_object_ty(ty) {
329-
errors.push(err);
337+
match ty.ty() {
338+
Some(ast::Type::DynTraitType(ty)) => {
339+
if let Some(err) = validate_trait_object_ty_plus(ty) {
340+
errors.push(err);
341+
}
330342
}
343+
Some(ast::Type::ImplTraitType(ty)) => {
344+
if let Some(err) = validate_impl_object_ty_plus(ty) {
345+
errors.push(err);
346+
}
347+
}
348+
_ => (),
331349
}
332350
}
333351

334352
fn validate_trait_object_fn_ptr_ret_ty(ty: ast::FnPtrType, errors: &mut Vec<SyntaxError>) {
335-
if let Some(ast::Type::DynTraitType(ty)) = ty.ret_type().and_then(|ty| ty.ty()) {
336-
if let Some(err) = validate_trait_object_ty(ty) {
337-
errors.push(err);
353+
match ty.ret_type().and_then(|ty| ty.ty()) {
354+
Some(ast::Type::DynTraitType(ty)) => {
355+
if let Some(err) = validate_trait_object_ty_plus(ty) {
356+
errors.push(err);
357+
}
358+
}
359+
Some(ast::Type::ImplTraitType(ty)) => {
360+
if let Some(err) = validate_impl_object_ty_plus(ty) {
361+
errors.push(err);
362+
}
338363
}
364+
_ => (),
339365
}
340366
}
341367

342368
fn validate_trait_object_ty(ty: ast::DynTraitType) -> Option<SyntaxError> {
343369
let tbl = ty.type_bound_list()?;
344-
let bounds_count = tbl.bounds().count();
370+
let no_bounds = tbl.bounds().filter_map(|it| it.ty()).next().is_none();
345371

346-
match bounds_count {
347-
0 => Some(SyntaxError::new(
372+
match no_bounds {
373+
true => Some(SyntaxError::new(
348374
"At least one trait is required for an object type",
349375
ty.syntax().text_range(),
350376
)),
351-
_ if bounds_count > 1 => {
352-
let dyn_token = ty.dyn_token()?;
353-
let preceding_token =
354-
algo::skip_trivia_token(dyn_token.prev_token()?, Direction::Prev)?;
355-
356-
if !matches!(preceding_token.kind(), T!['('] | T![<] | T![=]) {
357-
return Some(SyntaxError::new("ambiguous `+` in a type", ty.syntax().text_range()));
358-
}
359-
None
360-
}
361-
_ => None,
377+
false => None,
362378
}
363379
}
364380

365-
fn validate_impl_object_ty(ty: ast::ImplTraitType, errors: &mut Vec<SyntaxError>) {
366-
let Some(bound_list) = ty.type_bound_list() else {
367-
errors.push(SyntaxError::new(
368-
"At least one trait must be specified",
369-
ty.syntax().text_range(),
370-
));
371-
return;
372-
};
373-
374-
let bounds: Vec<_> = bound_list.bounds().collect();
381+
fn validate_impl_object_ty(ty: ast::ImplTraitType) -> Option<SyntaxError> {
382+
let tbl = ty.type_bound_list()?;
383+
let no_bounds = tbl.bounds().filter_map(|it| it.ty()).next().is_none();
375384

376-
if !bounds.iter().any(|b| !matches!(b.kind(), ast::TypeBoundKind::Lifetime(_))) {
377-
errors.push(SyntaxError::new(
378-
"At least one trait must be specified",
385+
match no_bounds {
386+
true => Some(SyntaxError::new(
387+
"At least one trait is required for an object type",
379388
ty.syntax().text_range(),
380-
));
381-
return;
389+
)),
390+
false => None,
382391
}
392+
}
383393

384-
if bounds.len() == 1 {
385-
return;
394+
// FIXME: This is not a validation error, this is a context dependent parse error
395+
fn validate_trait_object_ty_plus(ty: ast::DynTraitType) -> Option<SyntaxError> {
396+
let dyn_token = ty.dyn_token()?;
397+
let preceding_token = algo::skip_trivia_token(dyn_token.prev_token()?, Direction::Prev)?;
398+
let tbl = ty.type_bound_list()?;
399+
let more_than_one_bound = tbl.bounds().next_tuple::<(_, _)>().is_some();
400+
401+
if more_than_one_bound && !matches!(preceding_token.kind(), T!['('] | T![<] | T![=]) {
402+
Some(SyntaxError::new("ambiguous `+` in a type", ty.syntax().text_range()))
403+
} else {
404+
None
386405
}
406+
}
387407

388-
let Some(preceding_token) = ty
389-
.impl_token()
390-
.and_then(|token| token.prev_token())
391-
.and_then(|prev| algo::skip_trivia_token(prev, Direction::Prev))
392-
else {
393-
return;
394-
};
408+
// FIXME: This is not a validation error, this is a context dependent parse error
409+
fn validate_impl_object_ty_plus(ty: ast::ImplTraitType) -> Option<SyntaxError> {
410+
let dyn_token = ty.impl_token()?;
411+
let preceding_token = algo::skip_trivia_token(dyn_token.prev_token()?, Direction::Prev)?;
412+
let tbl = ty.type_bound_list()?;
413+
let more_than_one_bound = tbl.bounds().next_tuple::<(_, _)>().is_some();
395414

396-
if !matches!(preceding_token.kind(), T!['('] | T![<] | T![=])
397-
&& matches!(preceding_token.kind(), T![&])
398-
{
399-
errors.push(SyntaxError::new("ambiguous `+` in a type", ty.syntax().text_range()));
415+
if more_than_one_bound && !matches!(preceding_token.kind(), T!['('] | T![<] | T![=]) {
416+
Some(SyntaxError::new("ambiguous `+` in a type", ty.syntax().text_range()))
417+
} else {
418+
None
400419
}
401420
}
402421

‎crates/syntax/test_data/parser/validation/dangling_impl.rast‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ SOURCE_FILE@0..16
2020
STMT_LIST@14..16
2121
L_CURLY@14..15 "{"
2222
R_CURLY@15..16 "}"
23-
error 8..12: At least one trait must be specified
23+
error 8..12: At least one trait is required for an object type

‎crates/syntax/test_data/parser/validation/dangling_impl_reference.rast‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,4 @@ SOURCE_FILE@0..17
2222
STMT_LIST@15..17
2323
L_CURLY@15..16 "{"
2424
R_CURLY@16..17 "}"
25-
error 9..13: At least one trait must be specified
25+
error 9..13: At least one trait is required for an object type

‎crates/syntax/test_data/parser/validation/impl_trait_lifetime_only.rast‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,4 +26,4 @@ SOURCE_FILE@0..20
2626
STMT_LIST@18..20
2727
L_CURLY@18..19 "{"
2828
R_CURLY@19..20 "}"
29-
error 9..16: At least one trait must be specified
29+
error 9..16: At least one trait is required for an object type

0 commit comments

Comments
 (0)