From cf083fefc4ceb508ef34f02d189a24039f3457ef Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Fri, 8 Dec 2023 20:13:52 +0100 Subject: [PATCH] fix: Fix completion failing in format_args! with invalid template --- crates/hir-def/src/body/lower.rs | 14 +- crates/hir-def/src/hir/format_args.rs | 14 +- crates/ide-completion/src/tests/expression.rs | 154 ++++++++++++++++++ crates/ide/src/view_hir.rs | 6 +- crates/ide/src/view_mir.rs | 6 +- crates/rust-analyzer/tests/slow-tests/tidy.rs | 1 + 6 files changed, 187 insertions(+), 8 deletions(-) diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index c22bd6e2e57..c6a90932015 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -1611,7 +1611,11 @@ impl ExprCollector<'_> { } }, ), - None => FormatArgs { template: Default::default(), arguments: args.finish() }, + None => FormatArgs { + template: Default::default(), + arguments: args.finish(), + orphans: Default::default(), + }, }; // Create a list of all _unique_ (argument, format trait) combinations. @@ -1750,7 +1754,13 @@ impl ExprCollector<'_> { }); let unsafe_arg_new = self.alloc_expr_desugared(Expr::Unsafe { id: None, - statements: Box::default(), + // We collect the unused expressions here so that we still infer them instead of + // dropping them out of the expression tree + statements: fmt + .orphans + .into_iter() + .map(|expr| Statement::Expr { expr, has_semi: true }) + .collect(), tail: Some(unsafe_arg_new), }); diff --git a/crates/hir-def/src/hir/format_args.rs b/crates/hir-def/src/hir/format_args.rs index 068abb27a25..7fc33abc7c9 100644 --- a/crates/hir-def/src/hir/format_args.rs +++ b/crates/hir-def/src/hir/format_args.rs @@ -3,6 +3,7 @@ use std::mem; use hir_expand::name::Name; use rustc_dependencies::parse_format as parse; +use stdx::TupleExt; use syntax::{ ast::{self, IsString}, SmolStr, TextRange, TextSize, @@ -14,6 +15,7 @@ use crate::hir::ExprId; pub struct FormatArgs { pub template: Box<[FormatArgsPiece]>, pub arguments: FormatArguments, + pub orphans: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -196,7 +198,11 @@ pub(crate) fn parse( let is_source_literal = parser.is_source_literal; if !parser.errors.is_empty() { // FIXME: Diagnose - return FormatArgs { template: Default::default(), arguments: args.finish() }; + return FormatArgs { + template: Default::default(), + arguments: args.finish(), + orphans: vec![], + }; } let to_span = |inner_span: parse::InnerSpan| { @@ -419,7 +425,11 @@ pub(crate) fn parse( // FIXME: Diagnose } - FormatArgs { template: template.into_boxed_slice(), arguments: args.finish() } + FormatArgs { + template: template.into_boxed_slice(), + arguments: args.finish(), + orphans: unused.into_iter().map(TupleExt::head).collect(), + } } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/crates/ide-completion/src/tests/expression.rs b/crates/ide-completion/src/tests/expression.rs index e6969c8db30..b4f936b35ae 100644 --- a/crates/ide-completion/src/tests/expression.rs +++ b/crates/ide-completion/src/tests/expression.rs @@ -1094,3 +1094,157 @@ pub struct UnstableButWeAreOnNightlyAnyway; "#]], ); } + +#[test] +fn inside_format_args_completions_work() { + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("{}", Foo.$0); +} +"#, + expect![[r#" + me foo() fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn match match expr {} + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + "#]], + ); + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("{}", Foo.f$0); +} +"#, + expect![[r#" + me foo() fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn match match expr {} + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + "#]], + ); +} + +#[test] +fn inside_faulty_format_args_completions_work() { + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("", Foo.$0); +} +"#, + expect![[r#" + me foo() fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn match match expr {} + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + "#]], + ); + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("", Foo.f$0); +} +"#, + expect![[r#" + me foo() fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn match match expr {} + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + "#]], + ); + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("{} {named} {captured} {named} {}", a, named = c, Foo.f$0); +} +"#, + expect![[r#" + me foo() fn(&self) + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn match match expr {} + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + "#]], + ); + check_empty( + r#" +//- minicore: fmt +struct Foo; +impl Foo { + fn foo(&self) {} +} + +fn main() { + format_args!("{", Foo.f$0); +} +"#, + expect![[r#" + sn box Box::new(expr) + sn call function(expr) + sn dbg dbg!(expr) + sn dbgr dbg!(&expr) + sn if if expr {} + sn match match expr {} + sn not !expr + sn ref &expr + sn refm &mut expr + sn unsafe unsafe {} + sn while while expr {} + "#]], + ); +} diff --git a/crates/ide/src/view_hir.rs b/crates/ide/src/view_hir.rs index d2bbbf6d26a..738b370a068 100644 --- a/crates/ide/src/view_hir.rs +++ b/crates/ide/src/view_hir.rs @@ -1,7 +1,7 @@ use hir::{DefWithBody, Semantics}; use ide_db::base_db::FilePosition; use ide_db::RootDatabase; -use syntax::{algo::find_node_at_offset, ast, AstNode}; +use syntax::{algo::ancestors_at_offset, ast, AstNode}; // Feature: View Hir // @@ -19,7 +19,9 @@ fn body_hir(db: &RootDatabase, position: FilePosition) -> Option { let sema = Semantics::new(db); let source_file = sema.parse(position.file_id); - let item = find_node_at_offset::(source_file.syntax(), position.offset)?; + let item = ancestors_at_offset(source_file.syntax(), position.offset) + .filter(|it| ast::MacroCall::can_cast(it.kind())) + .find_map(ast::Item::cast)?; let def: DefWithBody = match item { ast::Item::Fn(it) => sema.to_def(&it)?.into(), ast::Item::Const(it) => sema.to_def(&it)?.into(), diff --git a/crates/ide/src/view_mir.rs b/crates/ide/src/view_mir.rs index a36aba58bc0..52ed420669a 100644 --- a/crates/ide/src/view_mir.rs +++ b/crates/ide/src/view_mir.rs @@ -1,7 +1,7 @@ use hir::{DefWithBody, Semantics}; use ide_db::base_db::FilePosition; use ide_db::RootDatabase; -use syntax::{algo::find_node_at_offset, ast, AstNode}; +use syntax::{algo::ancestors_at_offset, ast, AstNode}; // Feature: View Mir // @@ -18,7 +18,9 @@ fn body_mir(db: &RootDatabase, position: FilePosition) -> Option { let sema = Semantics::new(db); let source_file = sema.parse(position.file_id); - let item = find_node_at_offset::(source_file.syntax(), position.offset)?; + let item = ancestors_at_offset(source_file.syntax(), position.offset) + .filter(|it| ast::MacroCall::can_cast(it.kind())) + .find_map(ast::Item::cast)?; let def: DefWithBody = match item { ast::Item::Fn(it) => sema.to_def(&it)?.into(), ast::Item::Const(it) => sema.to_def(&it)?.into(), diff --git a/crates/rust-analyzer/tests/slow-tests/tidy.rs b/crates/rust-analyzer/tests/slow-tests/tidy.rs index 45adbf5c573..dba336ea7d6 100644 --- a/crates/rust-analyzer/tests/slow-tests/tidy.rs +++ b/crates/rust-analyzer/tests/slow-tests/tidy.rs @@ -250,6 +250,7 @@ fn check_dbg(path: &Path, text: &str) { // We have .dbg postfix "ide-completion/src/completions/postfix.rs", "ide-completion/src/completions/keyword.rs", + "ide-completion/src/tests/expression.rs", "ide-completion/src/tests/proc_macros.rs", // The documentation in string literals may contain anything for its own purposes "ide-completion/src/lib.rs",