From 4704a962e768e0133ee43348fe5bb41da580d0af Mon Sep 17 00:00:00 2001 From: roife Date: Fri, 13 Dec 2024 23:34:03 +0800 Subject: [PATCH 01/36] feat: improve name generation in destructure_tuple_binding --- .../crates/ide-assists/src/assist_context.rs | 5 ++ .../src/handlers/destructure_tuple_binding.rs | 75 +++++++++++-------- .../ide-db/src/syntax_helpers/suggest_name.rs | 2 + 3 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs b/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs index 0146369f298b5..074d943719f1b 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/assist_context.rs @@ -3,6 +3,7 @@ use hir::{FileRange, Semantics}; use ide_db::EditionedFileId; use ide_db::{label::Label, FileId, RootDatabase}; +use syntax::Edition; use syntax::{ algo::{self, find_node_at_offset, find_node_at_range}, AstNode, AstToken, Direction, SourceFile, SyntaxElement, SyntaxKind, SyntaxToken, TextRange, @@ -94,6 +95,10 @@ impl<'a> AssistContext<'a> { self.frange.file_id } + pub(crate) fn edition(&self) -> Edition { + self.frange.file_id.edition() + } + pub(crate) fn has_empty_selection(&self) -> bool { self.trimmed_range.is_empty() } diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs index 3f0d5cf152c23..50d89a0a3b18f 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs @@ -1,10 +1,12 @@ -use ide_db::text_edit::TextRange; use ide_db::{ assists::{AssistId, AssistKind}, defs::Definition, search::{FileReference, SearchScope, UsageSearchResult}, + syntax_helpers::suggest_name, + text_edit::TextRange, }; use itertools::Itertools; +use syntax::SmolStr; use syntax::{ ast::{self, make, AstNode, FieldExpr, HasName, IdentPat}, ted, @@ -131,24 +133,31 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext<'_>) -> Option name, + None => name_generator.suggest_name(&format!("_{}", id)), + } + .to_string() + }) .collect::>(); Some(TupleData { ident_pat, ref_type, field_names, usages }) } -fn generate_name( - _ctx: &AssistContext<'_>, - index: usize, - _tuple_name: &str, - _ident_pat: &IdentPat, - _usages: &Option, -) -> String { - // FIXME: detect if name already used - format!("_{index}") -} - enum RefType { ReadOnly, Mutable, @@ -1769,14 +1778,14 @@ struct S4 { } fn foo() -> Option<()> { - let ($0_0, _1, _2, _3, _4, _5) = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); + let ($0_0, _1, _2, _3, s4, _5) = &(0, (1,"1"), Some(2), [3;3], S4 { value: 4 }, &5); let v: i32 = *_0; // deref, no parens let v: &i32 = _0; // no deref, no parens, remove `&` f1(*_0); // deref, no parens f2(_0); // `&*` -> cancel out -> no deref, no parens // https://github.com/rust-lang/rust-analyzer/issues/1109#issuecomment-658868639 // let v: i32 = t.1.0; // no deref, no parens - let v: i32 = _4.value; // no deref, no parens + let v: i32 = s4.value; // no deref, no parens (*_0).do_stuff(); // deref, parens let v: i32 = (*_2)?; // deref, parens let v: i32 = _3[0]; // no deref, no parens @@ -1815,8 +1824,8 @@ impl S { } fn main() { - let ($0_0, _1) = &(S,2); - let s = _0.f(); + let ($0s, _1) = &(S,2); + let s = s.f(); } "#, ) @@ -1845,8 +1854,8 @@ impl S { } fn main() { - let ($0_0, _1) = &(S,2); - let s = (*_0).f(); + let ($0s, _1) = &(S,2); + let s = (*s).f(); } "#, ) @@ -1882,8 +1891,8 @@ impl T for &S { } fn main() { - let ($0_0, _1) = &(S,2); - let s = (*_0).f(); + let ($0s, _1) = &(S,2); + let s = (*s).f(); } "#, ) @@ -1923,8 +1932,8 @@ impl T for &S { } fn main() { - let ($0_0, _1) = &(S,2); - let s = (*_0).f(); + let ($0s, _1) = &(S,2); + let s = (*s).f(); } "#, ) @@ -1951,8 +1960,8 @@ impl S { fn do_stuff(&self) -> i32 { 42 } } fn main() { - let ($0_0, _1) = &(S,&S); - let v = _0.do_stuff(); + let ($0s, s1) = &(S,&S); + let v = s.do_stuff(); } "#, ) @@ -1973,7 +1982,7 @@ fn main() { // `t.0` gets auto-refed -> no deref needed -> no parens let v = t.0.do_stuff(); // no deref, no parens let v = &t.0.do_stuff(); // `&` is for result -> no deref, no parens - // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + // deref: `s1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` let v = t.1.do_stuff(); // deref, parens } "#, @@ -1984,13 +1993,13 @@ impl S { fn do_stuff(&self) -> i32 { 42 } } fn main() { - let ($0_0, _1) = &(S,&S); - let v = _0.do_stuff(); // no deref, remove parens + let ($0s, s1) = &(S,&S); + let v = s.do_stuff(); // no deref, remove parens // `t.0` gets auto-refed -> no deref needed -> no parens - let v = _0.do_stuff(); // no deref, no parens - let v = &_0.do_stuff(); // `&` is for result -> no deref, no parens - // deref: `_1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` - let v = (*_1).do_stuff(); // deref, parens + let v = s.do_stuff(); // no deref, no parens + let v = &s.do_stuff(); // `&` is for result -> no deref, no parens + // deref: `s1` is `&&S`, but method called is on `&S` -> there might be a method accepting `&&S` + let v = (*s1).do_stuff(); // deref, parens } "#, ) diff --git a/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/suggest_name.rs b/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/suggest_name.rs index b3ecc26cb22f5..1e08e8e309806 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/suggest_name.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/syntax_helpers/suggest_name.rs @@ -377,6 +377,8 @@ fn name_of_type(ty: &hir::Type, db: &RootDatabase, edition: Edition) -> Option Date: Sat, 14 Dec 2024 13:36:11 -0500 Subject: [PATCH 02/36] feat: Use string literal contents as a name when extracting into variable --- .../src/handlers/extract_variable.rs | 183 ++++++++++++++++-- 1 file changed, 168 insertions(+), 15 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs index a8d71ed7f4d72..6735d7dcbe10b 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/extract_variable.rs @@ -1,5 +1,8 @@ use hir::{HirDisplay, TypeInfo}; -use ide_db::{assists::GroupLabel, syntax_helpers::suggest_name}; +use ide_db::{ + assists::GroupLabel, + syntax_helpers::{suggest_name, LexedStr}, +}; use syntax::{ ast::{ self, edit::IndentLevel, edit_in_place::Indent, make, syntax_factory::SyntaxFactory, @@ -320,24 +323,58 @@ impl ExtractionKind { ctx: &AssistContext<'_>, to_extract: &ast::Expr, ) -> (String, SyntaxNode) { - let field_shorthand = to_extract - .syntax() - .parent() - .and_then(ast::RecordExprField::cast) - .filter(|field| field.name_ref().is_some()); - let (var_name, expr_replace) = match field_shorthand { - Some(field) => (field.to_string(), field.syntax().clone()), - None => { - (suggest_name::for_variable(to_extract, &ctx.sema), to_extract.syntax().clone()) + // We only do this sort of extraction for fields because they should have lowercase names + if let ExtractionKind::Variable = self { + let field_shorthand = to_extract + .syntax() + .parent() + .and_then(ast::RecordExprField::cast) + .filter(|field| field.name_ref().is_some()); + + if let Some(field) = field_shorthand { + return (field.to_string(), field.syntax().clone()); } + } + + let var_name = if let Some(literal_name) = get_literal_name(ctx, to_extract) { + literal_name + } else { + suggest_name::for_variable(to_extract, &ctx.sema) }; let var_name = match self { - ExtractionKind::Variable => var_name, + ExtractionKind::Variable => var_name.to_lowercase(), ExtractionKind::Constant | ExtractionKind::Static => var_name.to_uppercase(), }; - (var_name, expr_replace) + (var_name, to_extract.syntax().clone()) + } +} + +fn get_literal_name(ctx: &AssistContext<'_>, expr: &ast::Expr) -> Option { + let literal = match expr { + ast::Expr::Literal(literal) => literal, + _ => return None, + }; + let inner = match literal.kind() { + ast::LiteralKind::String(string) => string.value().ok()?.into_owned(), + ast::LiteralKind::ByteString(byte_string) => { + String::from_utf8(byte_string.value().ok()?.into_owned()).ok()? + } + ast::LiteralKind::CString(cstring) => { + String::from_utf8(cstring.value().ok()?.into_owned()).ok()? + } + _ => return None, + }; + + // Entirely arbitrary + if inner.len() > 32 { + return None; + } + + match LexedStr::single_token(ctx.file_id().edition(), &inner) { + Some((SyntaxKind::IDENT, None)) => Some(inner), + _ => None, } } @@ -493,7 +530,7 @@ fn main() { "#, r#" fn main() { - let $0var_name = "hello"; + let $0hello = "hello"; } "#, "Extract into variable", @@ -588,7 +625,7 @@ fn main() { "#, r#" fn main() { - const $0VAR_NAME: &str = "hello"; + const $0HELLO: &str = "hello"; } "#, "Extract into constant", @@ -683,7 +720,7 @@ fn main() { "#, r#" fn main() { - static $0VAR_NAME: &str = "hello"; + static $0HELLO: &str = "hello"; } "#, "Extract into static", @@ -2479,4 +2516,120 @@ fn foo() { "Extract into variable", ); } + + #[test] + fn extract_string_literal() { + check_assist_by_label( + extract_variable, + r#" +struct Entry(&str); +fn foo() { + let entry = Entry($0"Hello"$0); +} +"#, + r#" +struct Entry(&str); +fn foo() { + let $0hello = "Hello"; + let entry = Entry(hello); +} +"#, + "Extract into variable", + ); + + check_assist_by_label( + extract_variable, + r#" +struct Entry(&str); +fn foo() { + let entry = Entry($0"Hello"$0); +} +"#, + r#" +struct Entry(&str); +fn foo() { + const $0HELLO: &str = "Hello"; + let entry = Entry(HELLO); +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +struct Entry(&str); +fn foo() { + let entry = Entry($0"Hello"$0); +} +"#, + r#" +struct Entry(&str); +fn foo() { + static $0HELLO: &str = "Hello"; + let entry = Entry(HELLO); +} +"#, + "Extract into static", + ); + } + + #[test] + fn extract_variable_string_literal_use_field_shorthand() { + // When field shorthand is available, it should + // only be used when extracting into a variable + check_assist_by_label( + extract_variable, + r#" +struct Entry { message: &str } +fn foo() { + let entry = Entry { message: $0"Hello"$0 }; +} +"#, + r#" +struct Entry { message: &str } +fn foo() { + let $0message = "Hello"; + let entry = Entry { message }; +} +"#, + "Extract into variable", + ); + + check_assist_by_label( + extract_variable, + r#" +struct Entry { message: &str } +fn foo() { + let entry = Entry { message: $0"Hello"$0 }; +} +"#, + r#" +struct Entry { message: &str } +fn foo() { + const $0HELLO: &str = "Hello"; + let entry = Entry { message: HELLO }; +} +"#, + "Extract into constant", + ); + + check_assist_by_label( + extract_variable, + r#" +struct Entry { message: &str } +fn foo() { + let entry = Entry { message: $0"Hello"$0 }; +} +"#, + r#" +struct Entry { message: &str } +fn foo() { + static $0HELLO: &str = "Hello"; + let entry = Entry { message: HELLO }; +} +"#, + "Extract into static", + ); + } } From de555200f7914ca126b8a2e9a91526121c09b33b Mon Sep 17 00:00:00 2001 From: roife Date: Fri, 13 Dec 2024 23:40:02 +0800 Subject: [PATCH 03/36] refactor: simplify `edit_tuple_usages` in destructure_tuple_binding --- .../src/handlers/destructure_tuple_binding.rs | 76 ++++++++----------- 1 file changed, 30 insertions(+), 46 deletions(-) diff --git a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs index 50d89a0a3b18f..b9142d0318aad 100644 --- a/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs +++ b/src/tools/rust-analyzer/crates/ide-assists/src/handlers/destructure_tuple_binding.rs @@ -1,7 +1,7 @@ use ide_db::{ assists::{AssistId, AssistKind}, defs::Definition, - search::{FileReference, SearchScope, UsageSearchResult}, + search::{FileReference, SearchScope}, syntax_helpers::suggest_name, text_edit::TextRange, }; @@ -124,22 +124,25 @@ fn collect_data(ident_pat: IdentPat, ctx: &AssistContext<'_>) -> Option, field_names: Vec, - usages: Option, + usages: Option>, } fn edit_tuple_assignment( ctx: &AssistContext<'_>, @@ -222,42 +225,23 @@ fn edit_tuple_usages( ctx: &AssistContext<'_>, in_sub_pattern: bool, ) -> Option> { - let mut current_file_usages = None; - - if let Some(usages) = data.usages.as_ref() { - // We need to collect edits first before actually applying them - // as mapping nodes to their mutable node versions requires an - // unmodified syntax tree. - // - // We also defer editing usages in the current file first since - // tree mutation in the same file breaks when `builder.edit_file` - // is called - - if let Some((_, refs)) = usages.iter().find(|(file_id, _)| *file_id == ctx.file_id()) { - current_file_usages = Some( - refs.iter() - .filter_map(|r| edit_tuple_usage(ctx, edit, r, data, in_sub_pattern)) - .collect_vec(), - ); - } - - for (file_id, refs) in usages.iter() { - if file_id == ctx.file_id() { - continue; - } - - edit.edit_file(file_id.file_id()); - - let tuple_edits = refs - .iter() - .filter_map(|r| edit_tuple_usage(ctx, edit, r, data, in_sub_pattern)) - .collect_vec(); - - tuple_edits.into_iter().for_each(|tuple_edit| tuple_edit.apply(edit)) - } - } - - current_file_usages + // We need to collect edits first before actually applying them + // as mapping nodes to their mutable node versions requires an + // unmodified syntax tree. + // + // We also defer editing usages in the current file first since + // tree mutation in the same file breaks when `builder.edit_file` + // is called + + let edits = data + .usages + .as_ref()? + .as_slice() + .iter() + .filter_map(|r| edit_tuple_usage(ctx, edit, r, data, in_sub_pattern)) + .collect_vec(); + + Some(edits) } fn edit_tuple_usage( ctx: &AssistContext<'_>, From ffe908d3ee206ef959b6d1898ca59dbef576bf28 Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 16 Dec 2024 07:45:25 +0200 Subject: [PATCH 04/36] Report unresolved idents for implicit captures in `format_args!()` And also a bit of cleanup by storing the capture's span with the open quote included. --- .../rust-analyzer/crates/hir-def/src/body.rs | 52 ++++++++++++++----- .../crates/hir-def/src/body/lower.rs | 20 +++++-- .../crates/hir-def/src/body/lower/asm.rs | 10 ++-- .../crates/hir-def/src/hir/format_args.rs | 38 +++++++------- .../crates/hir/src/diagnostics.rs | 21 +++++--- .../rust-analyzer/crates/hir/src/semantics.rs | 16 +++--- .../src/handlers/unresolved_ident.rs | 16 +++--- 7 files changed, 110 insertions(+), 63 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir-def/src/body.rs b/src/tools/rust-analyzer/crates/hir-def/src/body.rs index 867bee95bed67..433a956ff9a4c 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/body.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/body.rs @@ -18,6 +18,7 @@ use smallvec::SmallVec; use span::{Edition, MacroFileId}; use syntax::{ast, AstPtr, SyntaxNodePtr}; use triomphe::Arc; +use tt::TextRange; use crate::{ db::DefDatabase, @@ -143,15 +144,7 @@ pub struct BodySourceMap { pub types: TypesSourceMap, - // FIXME: Make this a sane struct. - template_map: Option< - Box<( - // format_args! - FxHashMap)>, - // asm! - FxHashMap>>, - )>, - >, + template_map: Option>, expansions: FxHashMap>, MacroFileId>, @@ -160,6 +153,20 @@ pub struct BodySourceMap { diagnostics: Vec, } +#[derive(Default, Debug, Eq, PartialEq)] +struct FormatTemplate { + /// A map from `format_args!()` expressions to their captures. + format_args_to_captures: FxHashMap)>, + /// A map from `asm!()` expressions to their captures. + asm_to_captures: FxHashMap>>, + /// A map from desugared expressions of implicit captures to their source. + /// + /// The value stored for each capture is its template literal and offset inside it. The template literal + /// is from the `format_args[_nl]!()` macro and so needs to be mapped up once to go to the user-written + /// template. + implicit_capture_to_source: FxHashMap, TextRange)>>, +} + #[derive(Debug, Eq, PartialEq)] pub enum BodyDiagnostic { InactiveCode { node: InFile, cfg: CfgExpr, opts: CfgOptions }, @@ -798,18 +805,29 @@ impl BodySourceMap { node: InFile<&ast::FormatArgsExpr>, ) -> Option<(HygieneId, &[(syntax::TextRange, Name)])> { let src = node.map(AstPtr::new).map(AstPtr::upcast::); - let (hygiene, names) = - self.template_map.as_ref()?.0.get(&self.expr_map.get(&src)?.as_expr()?)?; + let (hygiene, names) = self + .template_map + .as_ref()? + .format_args_to_captures + .get(&self.expr_map.get(&src)?.as_expr()?)?; Some((*hygiene, &**names)) } + pub fn format_args_implicit_capture( + &self, + capture_expr: ExprId, + ) -> Option, TextRange)>> { + self.template_map.as_ref()?.implicit_capture_to_source.get(&capture_expr).copied() + } + pub fn asm_template_args( &self, node: InFile<&ast::AsmExpr>, ) -> Option<(ExprId, &[Vec<(syntax::TextRange, usize)>])> { let src = node.map(AstPtr::new).map(AstPtr::upcast::); let expr = self.expr_map.get(&src)?.as_expr()?; - Some(expr).zip(self.template_map.as_ref()?.1.get(&expr).map(std::ops::Deref::deref)) + Some(expr) + .zip(self.template_map.as_ref()?.asm_to_captures.get(&expr).map(std::ops::Deref::deref)) } /// Get a reference to the body source map's diagnostics. @@ -835,8 +853,14 @@ impl BodySourceMap { types, } = self; if let Some(template_map) = template_map { - template_map.0.shrink_to_fit(); - template_map.1.shrink_to_fit(); + let FormatTemplate { + format_args_to_captures, + asm_to_captures, + implicit_capture_to_source, + } = &mut **template_map; + format_args_to_captures.shrink_to_fit(); + asm_to_captures.shrink_to_fit(); + implicit_capture_to_source.shrink_to_fit(); } expr_map.shrink_to_fit(); expr_map_back.shrink_to_fit(); diff --git a/src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs b/src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs index 3b73d409634b1..eed9f9468fd96 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/body/lower.rs @@ -1957,8 +1957,10 @@ impl ExprCollector<'_> { _ => None, }); let mut mappings = vec![]; - let (fmt, hygiene) = match template.and_then(|it| self.expand_macros_to_string(it)) { - Some((s, is_direct_literal)) => { + let (fmt, hygiene) = match template.and_then(|template| { + self.expand_macros_to_string(template.clone()).map(|it| (it, template)) + }) { + Some(((s, is_direct_literal), template)) => { let call_ctx = self.expander.syntax_context(); let hygiene = self.hygiene_id_for(s.syntax().text_range().start()); let fmt = format_args::parse( @@ -1966,8 +1968,18 @@ impl ExprCollector<'_> { fmt_snippet, args, is_direct_literal, - |name| { + |name, range| { let expr_id = self.alloc_expr_desugared(Expr::Path(Path::from(name))); + if let Some(range) = range { + self.source_map + .template_map + .get_or_insert_with(Default::default) + .implicit_capture_to_source + .insert( + expr_id, + self.expander.in_file((AstPtr::new(&template), range)), + ); + } if !hygiene.is_root() { self.body.expr_hygiene.insert(expr_id, hygiene); } @@ -2139,7 +2151,7 @@ impl ExprCollector<'_> { self.source_map .template_map .get_or_insert_with(Default::default) - .0 + .format_args_to_captures .insert(idx, (hygiene, mappings)); idx } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/body/lower/asm.rs b/src/tools/rust-analyzer/crates/hir-def/src/body/lower/asm.rs index c1b58dbdd0cb3..68c7173d1e409 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/body/lower/asm.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/body/lower/asm.rs @@ -6,7 +6,7 @@ use syntax::{ ast::{self, HasName, IsString}, AstNode, AstPtr, AstToken, T, }; -use tt::{TextRange, TextSize}; +use tt::TextRange; use crate::{ body::lower::{ExprCollector, FxIndexSet}, @@ -224,7 +224,7 @@ impl ExprCollector<'_> { TextRange::new( inner_span.start.try_into().unwrap(), inner_span.end.try_into().unwrap(), - ) - TextSize::from(str_style.map(|it| it + 1).unwrap_or(0) as u32 + 1) + ) }) }; for piece in unverified_pieces { @@ -268,7 +268,11 @@ impl ExprCollector<'_> { Expr::InlineAsm(InlineAsm { operands: operands.into_boxed_slice(), options }), syntax_ptr, ); - self.source_map.template_map.get_or_insert_with(Default::default).1.insert(idx, mappings); + self.source_map + .template_map + .get_or_insert_with(Default::default) + .asm_to_captures + .insert(idx, mappings); idx } } diff --git a/src/tools/rust-analyzer/crates/hir-def/src/hir/format_args.rs b/src/tools/rust-analyzer/crates/hir-def/src/hir/format_args.rs index e1c3bd25bcf0f..e64e498c17074 100644 --- a/src/tools/rust-analyzer/crates/hir-def/src/hir/format_args.rs +++ b/src/tools/rust-analyzer/crates/hir-def/src/hir/format_args.rs @@ -1,5 +1,6 @@ //! Parses `format_args` input. +use either::Either; use hir_expand::name::Name; use intern::Symbol; use rustc_parse_format as parse; @@ -7,7 +8,7 @@ use span::SyntaxContextId; use stdx::TupleExt; use syntax::{ ast::{self, IsString}, - TextRange, TextSize, + TextRange, }; use crate::hir::ExprId; @@ -33,7 +34,7 @@ pub enum FormatArgsPiece { Placeholder(FormatPlaceholder), } -#[derive(Copy, Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct FormatPlaceholder { /// Index into [`FormatArgs::arguments`]. pub argument: FormatArgPosition, @@ -45,11 +46,11 @@ pub struct FormatPlaceholder { pub format_options: FormatOptions, } -#[derive(Copy, Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct FormatArgPosition { /// Which argument this position refers to (Ok), /// or would've referred to if it existed (Err). - pub index: Result, + pub index: Result>, /// What kind of position this is. See [`FormatArgPositionKind`]. pub kind: FormatArgPositionKind, /// The span of the name or number. @@ -88,7 +89,7 @@ pub enum FormatTrait { UpperHex, } -#[derive(Copy, Clone, Default, Debug, PartialEq, Eq)] +#[derive(Clone, Default, Debug, PartialEq, Eq)] pub struct FormatOptions { /// The width. E.g. `{:5}` or `{:width$}`. pub width: Option, @@ -133,7 +134,7 @@ pub enum FormatAlignment { Center, } -#[derive(Copy, Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum FormatCount { /// `{:5}` or `{:.5}` Literal(usize), @@ -173,7 +174,7 @@ pub(crate) fn parse( fmt_snippet: Option, mut args: FormatArgumentsCollector, is_direct_literal: bool, - mut synth: impl FnMut(Name) -> ExprId, + mut synth: impl FnMut(Name, Option) -> ExprId, mut record_usage: impl FnMut(Name, Option), call_ctx: SyntaxContextId, ) -> FormatArgs { @@ -192,7 +193,6 @@ pub(crate) fn parse( } None => None, }; - let mut parser = parse::Parser::new(&text, str_style, fmt_snippet, false, parse::ParseMode::Format); @@ -217,7 +217,6 @@ pub(crate) fn parse( let to_span = |inner_span: parse::InnerSpan| { is_source_literal.then(|| { TextRange::new(inner_span.start.try_into().unwrap(), inner_span.end.try_into().unwrap()) - - TextSize::from(str_style.map(|it| it + 1).unwrap_or(0) as u32 + 1) }) }; @@ -245,8 +244,8 @@ pub(crate) fn parse( Ok(index) } else { // Doesn't exist as an explicit argument. - invalid_refs.push((index, span, used_as, kind)); - Err(index) + invalid_refs.push((Either::Left(index), span, used_as, kind)); + Err(Either::Left(index)) } } ArgRef::Name(name, span) => { @@ -265,14 +264,17 @@ pub(crate) fn parse( // For the moment capturing variables from format strings expanded from macros is // disabled (see RFC #2795) // FIXME: Diagnose + invalid_refs.push((Either::Right(name.clone()), span, used_as, kind)); + Err(Either::Right(name)) + } else { + record_usage(name.clone(), span); + Ok(args.add(FormatArgument { + kind: FormatArgumentKind::Captured(name.clone()), + // FIXME: This is problematic, we might want to synthesize a dummy + // expression proper and/or desugar these. + expr: synth(name, span), + })) } - record_usage(name.clone(), span); - Ok(args.add(FormatArgument { - kind: FormatArgumentKind::Captured(name.clone()), - // FIXME: This is problematic, we might want to synthesize a dummy - // expression proper and/or desugar these. - expr: synth(name), - })) } } }; diff --git a/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs b/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs index 612c6adb2071c..cbb1ed95ed64d 100644 --- a/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/diagnostics.rs @@ -262,7 +262,7 @@ pub struct UnresolvedAssocItem { #[derive(Debug)] pub struct UnresolvedIdent { - pub expr_or_pat: InFile>>, + pub node: InFile<(AstPtr>, Option)>, } #[derive(Debug)] @@ -550,11 +550,10 @@ impl AnyDiagnostic { source_map: &hir_def::body::BodySourceMap, ) -> Option { let expr_syntax = |expr| { - source_map.expr_syntax(expr).inspect_err(|_| tracing::error!("synthetic syntax")).ok() - }; - let pat_syntax = |pat| { - source_map.pat_syntax(pat).inspect_err(|_| tracing::error!("synthetic syntax")).ok() + source_map.expr_syntax(expr).inspect_err(|_| stdx::never!("synthetic syntax")).ok() }; + let pat_syntax = + |pat| source_map.pat_syntax(pat).inspect_err(|_| stdx::never!("synthetic syntax")).ok(); let expr_or_pat_syntax = |id| match id { ExprOrPatId::ExprId(expr) => expr_syntax(expr).map(|it| it.map(AstPtr::wrap_left)), ExprOrPatId::PatId(pat) => pat_syntax(pat), @@ -626,8 +625,16 @@ impl AnyDiagnostic { UnresolvedAssocItem { expr_or_pat }.into() } &InferenceDiagnostic::UnresolvedIdent { id } => { - let expr_or_pat = expr_or_pat_syntax(id)?; - UnresolvedIdent { expr_or_pat }.into() + let node = match id { + ExprOrPatId::ExprId(id) => match source_map.expr_syntax(id) { + Ok(syntax) => syntax.map(|it| (it.wrap_left(), None)), + Err(SyntheticSyntax) => source_map + .format_args_implicit_capture(id)? + .map(|(node, range)| (node.wrap_left(), Some(range))), + }, + ExprOrPatId::PatId(id) => pat_syntax(id)?.map(|it| (it, None)), + }; + UnresolvedIdent { node }.into() } &InferenceDiagnostic::BreakOutsideOfLoop { expr, is_break, bad_value_break } => { let expr = expr_syntax(expr)?; diff --git a/src/tools/rust-analyzer/crates/hir/src/semantics.rs b/src/tools/rust-analyzer/crates/hir/src/semantics.rs index b896cda9ddf7d..f030c1862af84 100644 --- a/src/tools/rust-analyzer/crates/hir/src/semantics.rs +++ b/src/tools/rust-analyzer/crates/hir/src/semantics.rs @@ -38,7 +38,7 @@ use span::{AstIdMap, EditionedFileId, FileId, HirFileIdRepr, SyntaxContextId}; use stdx::TupleExt; use syntax::{ algo::skip_trivia_token, - ast::{self, HasAttrs as _, HasGenericParams, IsString as _}, + ast::{self, HasAttrs as _, HasGenericParams}, AstNode, AstToken, Direction, SyntaxKind, SyntaxNode, SyntaxNodePtr, SyntaxToken, TextRange, TextSize, }; @@ -643,8 +643,7 @@ impl<'db> SemanticsImpl<'db> { &self, string: &ast::String, ) -> Option>)>> { - let quote = string.open_quote_text_range()?; - + let string_start = string.syntax().text_range().start(); let token = self.wrap_token_infile(string.syntax().clone()).into_real_file().ok()?; self.descend_into_macros_breakable(token, |token, _| { (|| { @@ -658,7 +657,7 @@ impl<'db> SemanticsImpl<'db> { let format_args = self.wrap_node_infile(format_args); let res = source_analyzer .as_format_args_parts(self.db, format_args.as_ref())? - .map(|(range, res)| (range + quote.end(), res.map(Either::Left))) + .map(|(range, res)| (range + string_start, res.map(Either::Left))) .collect(); Some(res) } else { @@ -672,7 +671,7 @@ impl<'db> SemanticsImpl<'db> { .iter() .map(|&(range, index)| { ( - range + quote.end(), + range + string_start, Some(Either::Right(InlineAsmOperand { owner, expr, index })), ) }) @@ -690,17 +689,16 @@ impl<'db> SemanticsImpl<'db> { original_token: SyntaxToken, offset: TextSize, ) -> Option<(TextRange, Option>)> { - let original_string = ast::String::cast(original_token.clone())?; + let string_start = original_token.text_range().start(); let original_token = self.wrap_token_infile(original_token).into_real_file().ok()?; - let quote = original_string.open_quote_text_range()?; self.descend_into_macros_breakable(original_token, |token, _| { (|| { let token = token.value; self.resolve_offset_in_format_args( ast::String::cast(token)?, - offset.checked_sub(quote.end())?, + offset.checked_sub(string_start)?, ) - .map(|(range, res)| (range + quote.end(), res)) + .map(|(range, res)| (range + string_start, res)) })() .map_or(ControlFlow::Continue(()), ControlFlow::Break) }) diff --git a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_ident.rs b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_ident.rs index 68f14a97f5942..4f64dabeb52fc 100644 --- a/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_ident.rs +++ b/src/tools/rust-analyzer/crates/ide-diagnostics/src/handlers/unresolved_ident.rs @@ -7,20 +7,19 @@ pub(crate) fn unresolved_ident( ctx: &DiagnosticsContext<'_>, d: &hir::UnresolvedIdent, ) -> Diagnostic { - Diagnostic::new_with_syntax_node_ptr( - ctx, - DiagnosticCode::RustcHardError("E0425"), - "no such value in this scope", - d.expr_or_pat.map(Into::into), - ) - .experimental() + let mut range = + ctx.sema.diagnostics_display_range(d.node.map(|(node, _)| node.syntax_node_ptr())); + if let Some(in_node_range) = d.node.value.1 { + range.range = in_node_range + range.range.start(); + } + Diagnostic::new(DiagnosticCode::RustcHardError("E0425"), "no such value in this scope", range) + .experimental() } #[cfg(test)] mod tests { use crate::tests::check_diagnostics; - // FIXME: This should show a diagnostic #[test] fn feature() { check_diagnostics( @@ -28,6 +27,7 @@ mod tests { //- minicore: fmt fn main() { format_args!("{unresolved}"); + // ^^^^^^^^^^ error: no such value in this scope } "#, ) From 1cace0aa4135acad746516992bd8e6a89e4fd861 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 16 Dec 2024 10:01:35 +0100 Subject: [PATCH 05/36] internal: Simplify ratoml testdir usage --- .../rust-analyzer/tests/slow-tests/support.rs | 39 +++++-------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/support.rs b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/support.rs index 5a88a5515c7f5..1f52f366c5469 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/support.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/tests/slow-tests/support.rs @@ -1,7 +1,7 @@ use std::{ cell::{Cell, RefCell}, env, fs, - sync::{Once, OnceLock}, + sync::Once, time::Duration, }; @@ -141,34 +141,15 @@ impl Project<'_> { /// file in the config dir after server is run, something where our naive approach comes short. /// Using a `prelock` allows us to force a lock when we know we need it. pub(crate) fn server_with_lock(self, config_lock: bool) -> Server { - static CONFIG_DIR_LOCK: OnceLock<(Utf8PathBuf, Mutex<()>)> = OnceLock::new(); + static CONFIG_DIR_LOCK: Mutex<()> = Mutex::new(()); let config_dir_guard = if config_lock { Some({ - let (path, mutex) = CONFIG_DIR_LOCK.get_or_init(|| { - let value = TestDir::new().keep().path().to_owned(); - env::set_var("__TEST_RA_USER_CONFIG_DIR", &value); - (value, Mutex::new(())) - }); - #[allow(dyn_drop)] - (mutex.lock(), { - Box::new({ - struct Dropper(Utf8PathBuf); - impl Drop for Dropper { - fn drop(&mut self) { - for entry in fs::read_dir(&self.0).unwrap() { - let path = entry.unwrap().path(); - if path.is_file() { - fs::remove_file(path).unwrap(); - } else if path.is_dir() { - fs::remove_dir_all(path).unwrap(); - } - } - } - } - Dropper(path.clone()) - }) as Box - }) + let guard = CONFIG_DIR_LOCK.lock(); + let test_dir = TestDir::new(); + let value = test_dir.path().to_owned(); + env::set_var("__TEST_RA_USER_CONFIG_DIR", &value); + (guard, test_dir) }) } else { None @@ -311,14 +292,12 @@ pub(crate) struct Server { client: Connection, /// XXX: remove the tempdir last dir: TestDir, - #[allow(dyn_drop)] - _config_dir_guard: Option<(MutexGuard<'static, ()>, Box)>, + _config_dir_guard: Option<(MutexGuard<'static, ()>, TestDir)>, } impl Server { - #[allow(dyn_drop)] fn new( - config_dir_guard: Option<(MutexGuard<'static, ()>, Box)>, + config_dir_guard: Option<(MutexGuard<'static, ()>, TestDir)>, dir: TestDir, config: Config, ) -> Server { From 5c678215dd52e20efade9b69d949fbdc7a1ccd50 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Mon, 16 Dec 2024 11:14:56 +0100 Subject: [PATCH 06/36] internal: Don't serialize empty fields in completions and resolve payloads --- .../rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs | 4 ++++ .../crates/rust-analyzer/src/lsp/to_proto.rs | 10 +++++++--- src/tools/rust-analyzer/docs/dev/lsp-extensions.md | 2 +- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs index df06270a8b1b2..c0173d9c2470f 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/ext.rs @@ -823,8 +823,11 @@ impl Request for OnTypeFormatting { #[derive(Debug, Serialize, Deserialize)] pub struct CompletionResolveData { pub position: lsp_types::TextDocumentPositionParams, + #[serde(skip_serializing_if = "Vec::is_empty", default)] pub imports: Vec, + #[serde(skip_serializing_if = "Option::is_none", default)] pub version: Option, + #[serde(skip_serializing_if = "Option::is_none", default)] pub trigger_character: Option, pub for_ref: bool, pub hash: String, @@ -836,6 +839,7 @@ pub struct InlayHintResolveData { // This is a string instead of a u64 as javascript can't represent u64 fully pub hash: String, pub resolve_range: lsp_types::Range, + #[serde(skip_serializing_if = "Option::is_none", default)] pub version: Option, } diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs index 612cb547b4134..c3e7543d92116 100644 --- a/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/src/tools/rust-analyzer/crates/rust-analyzer/src/lsp/to_proto.rs @@ -2,6 +2,7 @@ use std::{ iter::once, mem, + ops::Not as _, sync::atomic::{AtomicU32, Ordering}, }; @@ -358,9 +359,12 @@ fn completion_item( filter_text, kind: Some(completion_item_kind(item.kind)), text_edit, - additional_text_edits: Some(additional_text_edits), + additional_text_edits: additional_text_edits + .is_empty() + .not() + .then_some(additional_text_edits), documentation, - deprecated: Some(item.deprecated), + deprecated: item.deprecated.then_some(item.deprecated), tags, command, insert_text_format, @@ -370,7 +374,7 @@ fn completion_item( if config.completion_label_details_support() { if fields_to_resolve.resolve_label_details { something_to_resolve |= true; - } else { + } else if item.label_detail.is_some() || item.detail.is_some() { lsp_item.label_details = Some(lsp_types::CompletionItemLabelDetails { detail: item.label_detail.as_ref().map(ToString::to_string), description: item.detail.clone(), diff --git a/src/tools/rust-analyzer/docs/dev/lsp-extensions.md b/src/tools/rust-analyzer/docs/dev/lsp-extensions.md index 2aad2cfa36137..0e37611a5493e 100644 --- a/src/tools/rust-analyzer/docs/dev/lsp-extensions.md +++ b/src/tools/rust-analyzer/docs/dev/lsp-extensions.md @@ -1,5 +1,5 @@