From f335d00263fd429d12fd844134df54657357de56 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sat, 8 Mar 2025 20:38:47 +0200 Subject: [PATCH 1/2] Lower Return Type Notation (`Type::method(..): Send`) We do it the way rustc does it, by only marking segments with it, and not the whole path. This will allow extending where it is allowed in the future. --- .../crates/hir-def/src/generics.rs | 2 +- .../rust-analyzer/crates/hir-def/src/path.rs | 29 +++- .../crates/hir-def/src/path/lower.rs | 26 +++- .../crates/hir-ty/src/display.rs | 131 +++++++++--------- .../crates/hir-ty/src/lower/path.rs | 17 ++- 5 files changed, 125 insertions(+), 80 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir-def/src/generics.rs b/src/tools/rust-analyzer/crates/hir-def/src/generics.rs index 6f1650adeb6f..682f21ab3b4c 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/generics.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/generics.rs @@ -872,7 +872,7 @@ fn copy_generic_args( args, has_self_type: generic_args.has_self_type, bindings, - desugared_from_fn: generic_args.desugared_from_fn, + parenthesized: generic_args.parenthesized, } }) } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/path.rs b/src/tools/rust-analyzer/crates/hir-def/src/path.rs index 713e7389736a..8f170fb983ac 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/path.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/path.rs @@ -79,6 +79,19 @@ thin_vec_with_header_struct! { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum GenericArgsParentheses { + No, + /// Bounds of the form `Type::method(..): Send` or `impl Trait`, + /// aka. Return Type Notation or RTN. + ReturnTypeNotation, + /// `Fn`-family parenthesized traits, e.g. `impl Fn(u32) -> String`. + /// + /// This is desugared into one generic argument containing a tuple of all arguments, + /// and an associated type binding for `Output` for the return type. + ParenSugar, +} + /// Generic arguments to a path segment (e.g. the `i32` in `Option`). This /// also includes bindings of associated types, like in `Iterator`. #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -92,9 +105,8 @@ pub struct GenericArgs { pub has_self_type: bool, /// Associated type bindings like in `Iterator`. pub bindings: Box<[AssociatedTypeBinding]>, - /// Whether these generic args were desugared from `Trait(Arg) -> Output` - /// parenthesis notation typically used for the `Fn` traits. - pub desugared_from_fn: bool, + /// Whether these generic args were written with parentheses and how. + pub parenthesized: GenericArgsParentheses, } /// An associated type binding like in `Iterator`. @@ -326,7 +338,16 @@ impl GenericArgs { args: Box::default(), has_self_type: false, bindings: Box::default(), - desugared_from_fn: false, + parenthesized: GenericArgsParentheses::No, + } + } + + pub(crate) fn return_type_notation() -> GenericArgs { + GenericArgs { + args: Box::default(), + has_self_type: false, + bindings: Box::default(), + parenthesized: GenericArgsParentheses::ReturnTypeNotation, } } } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/path/lower.rs b/src/tools/rust-analyzer/crates/hir-def/src/path/lower.rs index 7a6d6973298b..c6ea3c4d7105 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/path/lower.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/path/lower.rs @@ -13,7 +13,10 @@ use stdx::thin_vec::EmptyOptimizedThinVec; use syntax::ast::{self, AstNode, HasGenericArgs, HasTypeBounds}; use crate::{ - path::{AssociatedTypeBinding, GenericArg, GenericArgs, ModPath, Path, PathKind}, + path::{ + AssociatedTypeBinding, GenericArg, GenericArgs, GenericArgsParentheses, ModPath, Path, + PathKind, + }, type_ref::{LifetimeRef, TypeBound, TypeRef}, }; @@ -73,6 +76,9 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option< segment.parenthesized_arg_list(), segment.ret_type(), ) + }) + .or_else(|| { + segment.return_type_syntax().map(|_| GenericArgs::return_type_notation()) }); if args.is_some() { generic_args.resize(segments.len(), None); @@ -126,7 +132,7 @@ pub(super) fn lower_path(ctx: &mut LowerCtx<'_>, mut path: ast::Path) -> Option< has_self_type: true, bindings: it.bindings.clone(), - desugared_from_fn: it.desugared_from_fn, + parenthesized: it.parenthesized, }, None => GenericArgs { args: Box::new([self_type]), @@ -281,7 +287,12 @@ pub(super) fn lower_generic_args( let name = name_ref.as_name(); let args = assoc_type_arg .generic_arg_list() - .and_then(|args| lower_generic_args(lower_ctx, args)); + .and_then(|args| lower_generic_args(lower_ctx, args)) + .or_else(|| { + assoc_type_arg + .return_type_syntax() + .map(|_| GenericArgs::return_type_notation()) + }); let type_ref = assoc_type_arg.ty().map(|it| TypeRef::from_ast(lower_ctx, it)); let type_ref = type_ref @@ -315,7 +326,7 @@ pub(super) fn lower_generic_args( args: args.into_boxed_slice(), has_self_type: false, bindings: bindings.into_boxed_slice(), - desugared_from_fn: false, + parenthesized: GenericArgsParentheses::No, }) } @@ -353,5 +364,10 @@ fn lower_generic_args_from_fn_path( bounds: Box::default(), }]) }; - Some(GenericArgs { args, has_self_type: false, bindings, desugared_from_fn: true }) + Some(GenericArgs { + args, + has_self_type: false, + bindings, + parenthesized: GenericArgsParentheses::ParenSugar, + }) } diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/display.rs b/src/tools/rust-analyzer/crates/hir-ty/src/display.rs index 2ae7e746ba20..db305e98da43 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/display.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/display.rs @@ -2303,78 +2303,83 @@ impl HirDisplayWithTypesMap for Path { if let Some(generic_args) = segment.args_and_bindings { // We should be in type context, so format as `Foo` instead of `Foo::`. // Do we actually format expressions? - if generic_args.desugared_from_fn { - // First argument will be a tuple, which already includes the parentheses. - // If the tuple only contains 1 item, write it manually to avoid the trailing `,`. - let tuple = match generic_args.args[0] { - hir_def::path::GenericArg::Type(ty) => match &types_map[ty] { - TypeRef::Tuple(it) => Some(it), + match generic_args.parenthesized { + hir_def::path::GenericArgsParentheses::ReturnTypeNotation => { + write!(f, "(..)")?; + } + hir_def::path::GenericArgsParentheses::ParenSugar => { + // First argument will be a tuple, which already includes the parentheses. + // If the tuple only contains 1 item, write it manually to avoid the trailing `,`. + let tuple = match generic_args.args[0] { + hir_def::path::GenericArg::Type(ty) => match &types_map[ty] { + TypeRef::Tuple(it) => Some(it), + _ => None, + }, _ => None, - }, - _ => None, - }; - if let Some(v) = tuple { - if v.len() == 1 { - write!(f, "(")?; - v[0].hir_fmt(f, types_map)?; - write!(f, ")")?; - } else { - generic_args.args[0].hir_fmt(f, types_map)?; + }; + if let Some(v) = tuple { + if v.len() == 1 { + write!(f, "(")?; + v[0].hir_fmt(f, types_map)?; + write!(f, ")")?; + } else { + generic_args.args[0].hir_fmt(f, types_map)?; + } + } + if let Some(ret) = generic_args.bindings[0].type_ref { + if !matches!(&types_map[ret], TypeRef::Tuple(v) if v.is_empty()) { + write!(f, " -> ")?; + ret.hir_fmt(f, types_map)?; + } } } - if let Some(ret) = generic_args.bindings[0].type_ref { - if !matches!(&types_map[ret], TypeRef::Tuple(v) if v.is_empty()) { - write!(f, " -> ")?; - ret.hir_fmt(f, types_map)?; + hir_def::path::GenericArgsParentheses::No => { + let mut first = true; + // Skip the `Self` bound if exists. It's handled outside the loop. + for arg in &generic_args.args[generic_args.has_self_type as usize..] { + if first { + first = false; + write!(f, "<")?; + } else { + write!(f, ", ")?; + } + arg.hir_fmt(f, types_map)?; + } + for binding in generic_args.bindings.iter() { + if first { + first = false; + write!(f, "<")?; + } else { + write!(f, ", ")?; + } + write!(f, "{}", binding.name.display(f.db.upcast(), f.edition()))?; + match &binding.type_ref { + Some(ty) => { + write!(f, " = ")?; + ty.hir_fmt(f, types_map)? + } + None => { + write!(f, ": ")?; + f.write_joined( + binding.bounds.iter().map(TypesMapAdapter::wrap(types_map)), + " + ", + )?; + } + } } - } - return Ok(()); - } - let mut first = true; - // Skip the `Self` bound if exists. It's handled outside the loop. - for arg in &generic_args.args[generic_args.has_self_type as usize..] { - if first { - first = false; - write!(f, "<")?; - } else { - write!(f, ", ")?; - } - arg.hir_fmt(f, types_map)?; - } - for binding in generic_args.bindings.iter() { - if first { - first = false; - write!(f, "<")?; - } else { - write!(f, ", ")?; - } - write!(f, "{}", binding.name.display(f.db.upcast(), f.edition()))?; - match &binding.type_ref { - Some(ty) => { - write!(f, " = ")?; - ty.hir_fmt(f, types_map)? + // There may be no generic arguments to print, in case of a trait having only a + // single `Self` bound which is converted to `::Assoc`. + if !first { + write!(f, ">")?; } - None => { - write!(f, ": ")?; - f.write_joined( - binding.bounds.iter().map(TypesMapAdapter::wrap(types_map)), - " + ", - )?; + + // Current position: `|` + if generic_args.has_self_type { + write!(f, ">")?; } } } - - // There may be no generic arguments to print, in case of a trait having only a - // single `Self` bound which is converted to `::Assoc`. - if !first { - write!(f, ">")?; - } - - // Current position: `|` - if generic_args.has_self_type { - write!(f, ">")?; - } } } diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs b/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs index a165932ddcc8..58b143e84e0c 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs @@ -8,7 +8,7 @@ use hir_def::{ data::TraitFlags, expr_store::HygieneId, generics::{TypeParamProvenance, WherePredicate, WherePredicateTypeTarget}, - path::{GenericArg, GenericArgs, Path, PathSegment, PathSegments}, + path::{GenericArg, GenericArgs, GenericArgsParentheses, Path, PathSegment, PathSegments}, resolver::{ResolveValueResult, TypeNs, ValueNs}, type_ref::{TypeBound, TypeRef, TypesMap}, GenericDefId, GenericParamId, ItemContainerId, Lookup, TraitId, @@ -138,12 +138,15 @@ impl<'a, 'b> PathLoweringContext<'a, 'b> { fn prohibit_parenthesized_generic_args(&mut self) -> bool { if let Some(generic_args) = self.current_or_prev_segment.args_and_bindings { - if generic_args.desugared_from_fn { - let segment = self.current_segment_u32(); - self.on_diagnostic( - PathLoweringDiagnostic::ParenthesizedGenericArgsWithoutFnTrait { segment }, - ); - return true; + match generic_args.parenthesized { + GenericArgsParentheses::No => {} + GenericArgsParentheses::ReturnTypeNotation | GenericArgsParentheses::ParenSugar => { + let segment = self.current_segment_u32(); + self.on_diagnostic( + PathLoweringDiagnostic::ParenthesizedGenericArgsWithoutFnTrait { segment }, + ); + return true; + } } } false From f31ddcf52406c28dad8c35c7d5bbbe496e9cf2fb Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Sat, 8 Mar 2025 20:56:54 +0200 Subject: [PATCH 2/2] Emit an error when RTN is used in an incorrect place We miss one place: associated type bindings aka. `impl Trait`, but we also miss it for Fn-style parenthesizes error so I left it out for now. --- .../crates/hir-ty/src/lower/path.rs | 10 +++- .../crates/hir/src/diagnostics.rs | 18 +++++++ .../ide-diagnostics/src/handlers/bad_rtn.rs | 52 +++++++++++++++++++ .../crates/ide-diagnostics/src/lib.rs | 2 + 4 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/bad_rtn.rs diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs b/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs index 58b143e84e0c..e237009ebaf6 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/lower/path.rs @@ -607,8 +607,14 @@ impl<'a, 'b> PathLoweringContext<'a, 'b> { ) -> Substitution { let prohibit_parens = match def { GenericDefId::TraitId(trait_) => { - let trait_data = self.ctx.db.trait_data(trait_); - !trait_data.flags.contains(TraitFlags::RUSTC_PAREN_SUGAR) + // RTN is prohibited anyways if we got here. + let is_rtn = + self.current_or_prev_segment.args_and_bindings.is_some_and(|generics| { + generics.parenthesized == GenericArgsParentheses::ReturnTypeNotation + }); + let is_fn_trait = + !self.ctx.db.trait_data(trait_).flags.contains(TraitFlags::RUSTC_PAREN_SUGAR); + is_rtn || is_fn_trait } _ => true, }; diff --git a/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs b/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs index 1ed0daa37563..b383fa625e3b 100644 --- a/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs @@ -113,6 +113,7 @@ diagnostics![ UnusedVariable, GenericArgsProhibited, ParenthesizedGenericArgsWithoutFnTrait, + BadRtn, ]; #[derive(Debug)] @@ -420,6 +421,11 @@ pub struct ParenthesizedGenericArgsWithoutFnTrait { pub args: InFile>, } +#[derive(Debug)] +pub struct BadRtn { + pub rtn: InFile>, +} + impl AnyDiagnostic { pub(crate) fn body_validation_diagnostic( db: &dyn HirDatabase, @@ -712,6 +718,12 @@ impl AnyDiagnostic { Some(match *diag { PathLoweringDiagnostic::GenericArgsProhibited { segment, reason } => { let segment = hir_segment_to_ast_segment(&path.value, segment)?; + + if let Some(rtn) = segment.return_type_syntax() { + // RTN errors are emitted as `GenericArgsProhibited` or `ParenthesizedGenericArgsWithoutFnTrait`. + return Some(BadRtn { rtn: path.with_value(AstPtr::new(&rtn)) }.into()); + } + let args = if let Some(generics) = segment.generic_arg_list() { AstPtr::new(&generics).wrap_left() } else { @@ -722,6 +734,12 @@ impl AnyDiagnostic { } PathLoweringDiagnostic::ParenthesizedGenericArgsWithoutFnTrait { segment } => { let segment = hir_segment_to_ast_segment(&path.value, segment)?; + + if let Some(rtn) = segment.return_type_syntax() { + // RTN errors are emitted as `GenericArgsProhibited` or `ParenthesizedGenericArgsWithoutFnTrait`. + return Some(BadRtn { rtn: path.with_value(AstPtr::new(&rtn)) }.into()); + } + let args = AstPtr::new(&segment.parenthesized_arg_list()?); let args = path.with_value(args); ParenthesizedGenericArgsWithoutFnTrait { args }.into() diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/bad_rtn.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/bad_rtn.rs new file mode 100644 index 000000000000..9ed85f9f208e --- /dev/null +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/bad_rtn.rs @@ -0,0 +1,52 @@ +use ide_db::Severity; + +use crate::{Diagnostic, DiagnosticCode, DiagnosticsContext}; + +// Diagnostic: bad-rtn +// +// This diagnostic is shown when a RTN (Return Type Notation, `Type::method(..): Send`) is written in an improper place. +pub(crate) fn bad_rtn(ctx: &DiagnosticsContext<'_>, d: &hir::BadRtn) -> Diagnostic { + Diagnostic::new_with_syntax_node_ptr( + ctx, + DiagnosticCode::Ra("bad-rtn", Severity::Error), + "return type notation not allowed in this position yet", + d.rtn.map(Into::into), + ) +} + +#[cfg(test)] +mod tests { + use crate::tests::check_diagnostics; + + #[test] + fn fn_traits_also_emit() { + check_diagnostics( + r#" +//- minicore: fn +fn foo< + A: Fn(..), + // ^^^^ error: return type notation not allowed in this position yet +>() {} + "#, + ); + } + + #[test] + fn bad_rtn() { + check_diagnostics( + r#" +mod module { + pub struct Type; +} +trait Trait {} + +fn foo() +where + module(..)::Type: Trait + // ^^^^ error: return type notation not allowed in this position yet +{ +} + "#, + ); + } +} diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs index f4ced736b3df..d1d852faf00e 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/lib.rs @@ -25,6 +25,7 @@ mod handlers { pub(crate) mod await_outside_of_async; + pub(crate) mod bad_rtn; pub(crate) mod break_outside_of_loop; pub(crate) mod expected_function; pub(crate) mod generic_args_prohibited; @@ -493,6 +494,7 @@ pub fn semantic_diagnostics( AnyDiagnostic::ParenthesizedGenericArgsWithoutFnTrait(d) => { handlers::parenthesized_generic_args_without_fn_trait::parenthesized_generic_args_without_fn_trait(&ctx, &d) } + AnyDiagnostic::BadRtn(d) => handlers::bad_rtn::bad_rtn(&ctx, &d), }; res.push(d) }