Skip to content

Resynthesize foo<bar>( and foo<bar>:: in check_no_chained_comparison #144884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
165 changes: 154 additions & 11 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rustc_ast::token::{self, Lit, LitKind, Token, TokenKind};
use rustc_ast::util::parser::AssocOp;
use rustc_ast::{
AngleBracketedArg, AngleBracketedArgs, AnonConst, AttrVec, BinOpKind, BindingMode, Block,
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, Item, ItemKind, Param, Pat, PatKind,
Path, PathSegment, QSelf, Recovered, Ty, TyKind,
BlockCheckMode, DUMMY_NODE_ID, Expr, ExprKind, GenericArg, GenericArgs, Generics, Item,
ItemKind, Param, Pat, PatKind, Path, PathSegment, QSelf, Recovered, Ty, TyKind,
};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -1453,6 +1453,70 @@ impl<'a> Parser<'a> {
let mk_err_expr =
|this: &Self, span, guar| Ok(Some(this.mk_expr(span, ExprKind::Err(guar))));

// Helper function to check if we should attempt turbofish recovery and resynthesize
// FIXME: Check more cases
let mk_turbofish_expr = |this: &Self,
span: Span,
l1: &P<Expr>,
r1: &P<Expr>,
as_call: bool|
-> Option<P<Expr>> {
// First, actually check that the two expressions in the binop are paths.
// Only proceed if both sides are simple paths.
let (func_path, type_arg_path) = match (&l1.kind, &r1.kind) {
(ExprKind::Path(None, func_path), ExprKind::Path(None, type_arg_path)) => {
// Ensure both paths have no existing generic arguments to avoid confusion
let func_has_generics = func_path.segments.iter().any(|seg| seg.args.is_some());
let type_has_generics =
type_arg_path.segments.iter().any(|seg| seg.args.is_some());
if func_has_generics || type_has_generics {
return None;
}
(func_path, type_arg_path)
}
_ => return None,
};

// Only handle simple function calls (not complex paths)
if func_path.segments.len() != 1 || type_arg_path.segments.len() != 1 {
return None;
}

// Build the corrected turbofish expression
let type_arg = P(Ty {
id: DUMMY_NODE_ID,
kind: TyKind::Path(None, type_arg_path.clone()),
span: r1.span,
tokens: None,
});

let generic_arg = GenericArg::Type(type_arg);
let angle_bracketed_arg = AngleBracketedArg::Arg(generic_arg);
let angle_bracketed_args = AngleBracketedArgs {
span: l1.span.to(r1.span),
args: thin_vec![angle_bracketed_arg],
};
let generic_args = GenericArgs::AngleBracketed(angle_bracketed_args);

// Create the function path with generic arguments
let mut func_path_with_generics = func_path.clone();
if let Some(last_segment) = func_path_with_generics.segments.last_mut() {
last_segment.args = Some(P(generic_args));
}

// Create the appropriate expression based on context
if as_call {
// For `foo<bar>()` case - create a function call expression
let func_expr = this
.mk_expr(l1.span.to(r1.span), ExprKind::Path(None, func_path_with_generics));
let args = thin_vec![]; // Empty arguments since they were already consumed
Some(this.mk_expr(span, ExprKind::Call(func_expr, args)))
} else {
// For `foo<bar>::` case - create a path expression
Some(this.mk_expr(span, ExprKind::Path(None, func_path_with_generics)))
}
};

match &inner_op.kind {
ExprKind::Binary(op, l1, r1) if op.node.is_comparison() => {
let mut err = ComparisonOperatorsCannotBeChained {
Expand Down Expand Up @@ -1496,13 +1560,84 @@ impl<'a> Parser<'a> {

// Consume the rest of the likely `foo<bar>::new()` or return at `foo<bar>`.
match self.parse_expr() {
Ok(_) => {
Ok(parsed_expr) => {
// 99% certain that the suggestion is correct, continue parsing.
let guar = self.dcx().emit_err(err);
// FIXME: actually check that the two expressions in the binop are
// paths and resynthesize new fn call expression instead of using
// `ExprKind::Err` placeholder.
mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar)
// Check if we can resynthesize the complete expression combining
// the corrected path with the parsed suffix
if let Some(corrected_path_expr) =
mk_turbofish_expr(self, l1.span.to(r1.span), l1, r1, false)
{
// We have a corrected path (e.g., Vec::<i32>), now combine it with
// the parsed expression (e.g., new()) to form the complete expression
// (e.g., Vec::<i32>::new())

// Try to construct the complete path expression
if let ExprKind::Path(qself, corrected_path) =
corrected_path_expr.kind
{
// Combine the corrected path with the parsed expression
// The parsed_expr should be handled as a continuation of the path
match &parsed_expr.kind {
ExprKind::Call(func, args) => {
// Handle case like Vec::<i32>::new()
if let ExprKind::Path(None, method_path) =
&func.kind
{
// Extend the corrected path with the method path
let mut extended_path = corrected_path;
extended_path
.segments
.extend(method_path.segments.clone());
let extended_func = self.mk_expr(
l1.span.to(func.span),
ExprKind::Path(qself, extended_path),
);
let complete_expr = self.mk_expr(
inner_op.span.to(self.prev_token.span),
ExprKind::Call(extended_func, args.clone()),
);
Ok(Some(complete_expr))
} else {
mk_err_expr(
self,
inner_op.span.to(self.prev_token.span),
guar,
)
}
}
ExprKind::Path(None, method_path) => {
// Handle case like Vec::<i32>::CONSTANT
let mut extended_path = corrected_path;
extended_path
.segments
.extend(method_path.segments.clone());
let complete_expr = self.mk_expr(
inner_op.span.to(self.prev_token.span),
ExprKind::Path(qself, extended_path),
);
Ok(Some(complete_expr))
}
_ => {
// For other cases, fall back to error expr
mk_err_expr(
self,
inner_op.span.to(self.prev_token.span),
guar,
)
}
}
} else {
mk_err_expr(
self,
inner_op.span.to(self.prev_token.span),
guar,
)
}
} else {
// Validation failed: not both paths, use error expr
mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar)
}
}
Err(expr_err) => {
expr_err.cancel();
Expand All @@ -1527,10 +1662,18 @@ impl<'a> Parser<'a> {
Err(()) => Err(self.dcx().create_err(err)),
Ok(()) => {
let guar = self.dcx().emit_err(err);
// FIXME: actually check that the two expressions in the binop are
// paths and resynthesize new fn call expression instead of using
// `ExprKind::Err` placeholder.
mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar)
// Try to resynthesize the function call expression (for `foo<bar>()` case)
if let Some(call_expr) = mk_turbofish_expr(
self,
inner_op.span.to(self.prev_token.span),
l1,
r1,
true,
) {
Ok(Some(call_expr))
} else {
mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar)
}
}
}
} else {
Expand Down
3 changes: 3 additions & 0 deletions tests/ui/parser/require-parens-for-chained-comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ fn main() {
f<X>();
//~^ ERROR comparison operators cannot be chained
//~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
//~| ERROR cannot find type `X` in this scope [E0412]
//~| ERROR cannot find function `f` in this scope [E0425]

f<Result<Option<X>, Option<Option<X>>>(1, 2);
//~^ ERROR comparison operators cannot be chained
//~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
//~| ERROR cannot find function `f` in this scope [E0425]

let _ = f<u8, i8>();
//~^ ERROR expected one of
Expand Down
40 changes: 30 additions & 10 deletions tests/ui/parser/require-parens-for-chained-comparison.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ LL | f::<X>();
| ++

error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:14:6
--> $DIR/require-parens-for-chained-comparison.rs:16:6
|
LL | f<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^ ^
Expand All @@ -43,7 +43,7 @@ LL | f::<Result<Option<X>, Option<Option<X>>>(1, 2);
| ++

error: expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, or an operator, found `,`
--> $DIR/require-parens-for-chained-comparison.rs:18:17
--> $DIR/require-parens-for-chained-comparison.rs:21:17
|
LL | let _ = f<u8, i8>();
| ^ expected one of 8 possible tokens
Expand All @@ -54,13 +54,13 @@ LL | let _ = f::<u8, i8>();
| ++

error: invalid label name `'_`
--> $DIR/require-parens-for-chained-comparison.rs:22:15
--> $DIR/require-parens-for-chained-comparison.rs:25:15
|
LL | let _ = f<'_, i8>();
| ^^

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/require-parens-for-chained-comparison.rs:22:17
--> $DIR/require-parens-for-chained-comparison.rs:25:17
|
LL | let _ = f<'_, i8>();
| ^ expected `while`, `for`, `loop` or `{` after a label
Expand All @@ -71,7 +71,7 @@ LL | let _ = f<'_', i8>();
| +

error: expected one of `.`, `:`, `;`, `?`, `else`, `for`, `loop`, `while`, or an operator, found `,`
--> $DIR/require-parens-for-chained-comparison.rs:22:17
--> $DIR/require-parens-for-chained-comparison.rs:25:17
|
LL | let _ = f<'_, i8>();
| ^ expected one of 9 possible tokens
Expand All @@ -82,13 +82,13 @@ LL | let _ = f::<'_, i8>();
| ++

error: invalid label name `'_`
--> $DIR/require-parens-for-chained-comparison.rs:29:7
--> $DIR/require-parens-for-chained-comparison.rs:32:7
|
LL | f<'_>();
| ^^

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/require-parens-for-chained-comparison.rs:29:9
--> $DIR/require-parens-for-chained-comparison.rs:32:9
|
LL | f<'_>();
| ^ expected `while`, `for`, `loop` or `{` after a label
Expand All @@ -99,7 +99,7 @@ LL | f<'_'>();
| +

error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:29:6
--> $DIR/require-parens-for-chained-comparison.rs:32:6
|
LL | f<'_>();
| ^ ^
Expand All @@ -110,13 +110,33 @@ LL | f::<'_>();
| ++

error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:36:14
--> $DIR/require-parens-for-chained-comparison.rs:39:14
|
LL | let _ = f<u8>;
| ^ ^
|
= help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
= help: or use `(...)` if you meant to specify fn arguments

error: aborting due to 12 previous errors
error[E0412]: cannot find type `X` in this scope
--> $DIR/require-parens-for-chained-comparison.rs:10:7
|
LL | f<X>();
| ^ not found in this scope

error[E0425]: cannot find function `f` in this scope
--> $DIR/require-parens-for-chained-comparison.rs:10:5
|
LL | f<X>();
| ^ not found in this scope

error[E0425]: cannot find function `f` in this scope
--> $DIR/require-parens-for-chained-comparison.rs:16:5
|
LL | f<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^ not found in this scope

error: aborting due to 15 previous errors

Some errors have detailed explanations: E0412, E0425.
For more information about an error, try `rustc --explain E0412`.
Loading