diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs index d843ac1cad..684eaf1c3b 100644 --- a/crates/hir-def/src/body.rs +++ b/crates/hir-def/src/body.rs @@ -10,6 +10,7 @@ use std::ops::{Deref, Index}; use base_db::CrateId; use cfg::{CfgExpr, CfgOptions}; +use either::Either; use hir_expand::{name::Name, ExpandError, InFile}; use la_arena::{Arena, ArenaMap, Idx, RawIdx}; use rustc_hash::FxHashMap; @@ -22,8 +23,8 @@ use crate::{ db::DefDatabase, expander::Expander, hir::{ - dummy_expr_id, Array, AsmOperand, Binding, BindingId, Expr, ExprId, Label, LabelId, Pat, - PatId, RecordFieldPat, Statement, + dummy_expr_id, Array, AsmOperand, Binding, BindingId, Expr, ExprId, ExprOrPatId, Label, + LabelId, Pat, PatId, RecordFieldPat, Statement, }, item_tree::AttrOwner, nameres::DefMap, @@ -68,9 +69,12 @@ pub type LabelSource = InFile; pub type FieldPtr = AstPtr; pub type FieldSource = InFile; -pub type PatFieldPtr = AstPtr; +pub type PatFieldPtr = AstPtr>; pub type PatFieldSource = InFile; +pub type ExprOrPatPtr = AstPtr>; +pub type ExprOrPatSource = InFile; + /// An item body together with the mapping from syntax nodes to HIR expression /// IDs. This is needed to go from e.g. a position in a file to the HIR /// expression containing it; but for type inference etc., we want to operate on @@ -84,11 +88,13 @@ pub type PatFieldSource = InFile; /// this properly for macros. #[derive(Default, Debug, Eq, PartialEq)] pub struct BodySourceMap { - expr_map: FxHashMap, + // AST expressions can create patterns in destructuring assignments. Therefore, `ExprSource` can also map + // to `PatId`, and `PatId` can also map to `ExprSource` (the other way around is unaffected). + expr_map: FxHashMap, expr_map_back: ArenaMap, pat_map: FxHashMap, - pat_map_back: ArenaMap, + pat_map_back: ArenaMap, label_map: FxHashMap, label_map_back: ArenaMap, @@ -372,7 +378,7 @@ impl Body { if let &Some(expr) = else_branch { f(expr); } - walk_exprs_in_pat(self, *pat, &mut f); + self.walk_exprs_in_pat(*pat, &mut f); } Statement::Expr { expr: expression, .. } => f(*expression), Statement::Item => (), @@ -448,18 +454,18 @@ impl Body { } }, &Expr::Assignment { target, value } => { - walk_exprs_in_pat(self, target, &mut f); + self.walk_exprs_in_pat(target, &mut f); f(value); } } + } - fn walk_exprs_in_pat(this: &Body, pat_id: PatId, f: &mut impl FnMut(ExprId)) { - this.walk_pats(pat_id, &mut |pat| { - if let Pat::Expr(expr) | Pat::ConstBlock(expr) = this[pat] { - f(expr); - } - }); - } + pub fn walk_exprs_in_pat(&self, pat_id: PatId, f: &mut impl FnMut(ExprId)) { + self.walk_pats(pat_id, &mut |pat| { + if let Pat::Expr(expr) | Pat::ConstBlock(expr) = self[pat] { + f(expr); + } + }); } } @@ -514,11 +520,18 @@ impl Index for Body { // FIXME: Change `node_` prefix to something more reasonable. // Perhaps `expr_syntax` and `expr_id`? impl BodySourceMap { + pub fn expr_or_pat_syntax(&self, id: ExprOrPatId) -> Result { + match id { + ExprOrPatId::ExprId(id) => self.expr_syntax(id).map(|it| it.map(AstPtr::wrap_left)), + ExprOrPatId::PatId(id) => self.pat_syntax(id), + } + } + pub fn expr_syntax(&self, expr: ExprId) -> Result { self.expr_map_back.get(expr).cloned().ok_or(SyntheticSyntax) } - pub fn node_expr(&self, node: InFile<&ast::Expr>) -> Option { + pub fn node_expr(&self, node: InFile<&ast::Expr>) -> Option { let src = node.map(AstPtr::new); self.expr_map.get(&src).cloned() } @@ -534,7 +547,7 @@ impl BodySourceMap { self.expansions.iter().map(|(&a, &b)| (a, b)) } - pub fn pat_syntax(&self, pat: PatId) -> Result { + pub fn pat_syntax(&self, pat: PatId) -> Result { self.pat_map_back.get(pat).cloned().ok_or(SyntheticSyntax) } @@ -567,7 +580,7 @@ impl BodySourceMap { self.pat_field_map_back[&pat] } - pub fn macro_expansion_expr(&self, node: InFile<&ast::MacroExpr>) -> Option { + pub fn macro_expansion_expr(&self, node: InFile<&ast::MacroExpr>) -> Option { let src = node.map(AstPtr::new).map(AstPtr::upcast::).map(AstPtr::upcast); self.expr_map.get(&src).copied() } @@ -583,7 +596,11 @@ impl BodySourceMap { node: InFile<&ast::FormatArgsExpr>, ) -> Option<&[(syntax::TextRange, Name)]> { let src = node.map(AstPtr::new).map(AstPtr::upcast::); - self.template_map.as_ref()?.0.get(self.expr_map.get(&src)?).map(std::ops::Deref::deref) + self.template_map + .as_ref()? + .0 + .get(&self.expr_map.get(&src)?.as_expr()?) + .map(std::ops::Deref::deref) } pub fn asm_template_args( @@ -591,8 +608,8 @@ impl BodySourceMap { 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)?; - Some(*expr).zip(self.template_map.as_ref()?.1.get(expr).map(std::ops::Deref::deref)) + 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)) } /// Get a reference to the body source map's diagnostics. diff --git a/crates/hir-def/src/body/lower.rs b/crates/hir-def/src/body/lower.rs index 3dff6ecd75..4b74028b83 100644 --- a/crates/hir-def/src/body/lower.rs +++ b/crates/hir-def/src/body/lower.rs @@ -424,7 +424,7 @@ impl ExprCollector<'_> { let inner = self.collect_expr_opt(e.expr()); // make the paren expr point to the inner expression as well for IDE resolution let src = self.expander.in_file(syntax_ptr); - self.source_map.expr_map.insert(src, inner); + self.source_map.expr_map.insert(src, inner.into()); inner } ast::Expr::ReturnExpr(e) => { @@ -660,7 +660,7 @@ impl ExprCollector<'_> { // Make the macro-call point to its expanded expression so we can query // semantics on syntax pointers to the macro let src = self.expander.in_file(syntax_ptr); - self.source_map.expr_map.insert(src, id); + self.source_map.expr_map.insert(src, id.into()); id } None => self.alloc_expr(Expr::Missing, syntax_ptr), @@ -686,9 +686,12 @@ impl ExprCollector<'_> { fn collect_expr_as_pat(&mut self, expr: ast::Expr) -> PatId { self.maybe_collect_expr_as_pat(&expr).unwrap_or_else(|| { - let syntax_ptr = AstPtr::new(&expr); + let src = self.expander.in_file(AstPtr::new(&expr).wrap_left()); let expr = self.collect_expr(expr); - self.alloc_pat_from_expr(Pat::Expr(expr), syntax_ptr) + // Do not use `alloc_pat_from_expr()` here, it will override the entry in `expr_map`. + let id = self.body.pats.alloc(Pat::Expr(expr)); + self.source_map.pat_map_back.insert(id, src); + id }) } @@ -743,16 +746,12 @@ impl ExprCollector<'_> { ast::Expr::MacroExpr(e) => { let e = e.macro_call()?; let macro_ptr = AstPtr::new(&e); + let src = self.expander.in_file(AstPtr::new(expr)); let id = self.collect_macro_call(e, macro_ptr, true, |this, expansion| { - expansion.map(|it| this.collect_expr_as_pat(it)) + this.collect_expr_as_pat_opt(expansion) }); - match id { - Some(id) => { - // FIXME: Insert pat into source map. - id - } - None => self.alloc_pat_from_expr(Pat::Missing, syntax_ptr), - } + self.source_map.expr_map.insert(src, id.into()); + id } ast::Expr::RecordExpr(e) => { let path = @@ -767,9 +766,8 @@ impl ExprCollector<'_> { let field_expr = f.expr()?; let pat = self.collect_expr_as_pat(field_expr); let name = f.field_name()?.as_name(); - // FIXME: Enable this. - // let src = self.expander.in_file(AstPtr::new(&f)); - // self.source_map.pat_field_map_back.insert(pat, src); + let src = self.expander.in_file(AstPtr::new(&f).wrap_left()); + self.source_map.pat_field_map_back.insert(pat, src); Some(RecordFieldPat { name, pat }) }) .collect(); @@ -813,7 +811,10 @@ impl ExprCollector<'_> { None => Either::Left(this.missing_pat()), }, ); - // FIXME: Insert pat into source map. + if let Either::Left(pat) = pat { + let src = this.expander.in_file(AstPtr::new(&expr).wrap_left()); + this.source_map.pat_map_back.insert(pat, src); + } pat } None => { @@ -1236,7 +1237,7 @@ impl ExprCollector<'_> { // Make the macro-call point to its expanded expression so we can query // semantics on syntax pointers to the macro let src = self.expander.in_file(syntax_ptr); - self.source_map.expr_map.insert(src, tail); + self.source_map.expr_map.insert(src, tail.into()); }) } @@ -1500,7 +1501,7 @@ impl ExprCollector<'_> { let ast_pat = f.pat()?; let pat = self.collect_pat(ast_pat, binding_list); let name = f.field_name()?.as_name(); - let src = self.expander.in_file(AstPtr::new(&f)); + let src = self.expander.in_file(AstPtr::new(&f).wrap_right()); self.source_map.pat_field_map_back.insert(pat, src); Some(RecordFieldPat { name, pat }) }) @@ -2187,7 +2188,7 @@ impl ExprCollector<'_> { let src = self.expander.in_file(ptr); let id = self.body.exprs.alloc(expr); self.source_map.expr_map_back.insert(id, src); - self.source_map.expr_map.insert(src, id); + self.source_map.expr_map.insert(src, id.into()); id } // FIXME: desugared exprs don't have ptr, that's wrong and should be fixed. @@ -2215,14 +2216,17 @@ impl ExprCollector<'_> { binding } - fn alloc_pat_from_expr(&mut self, pat: Pat, _ptr: ExprPtr) -> PatId { - // FIXME: Insert into source map. - self.body.pats.alloc(pat) + fn alloc_pat_from_expr(&mut self, pat: Pat, ptr: ExprPtr) -> PatId { + let src = self.expander.in_file(ptr); + let id = self.body.pats.alloc(pat); + self.source_map.expr_map.insert(src, id.into()); + self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_left)); + id } fn alloc_pat(&mut self, pat: Pat, ptr: PatPtr) -> PatId { let src = self.expander.in_file(ptr); let id = self.body.pats.alloc(pat); - self.source_map.pat_map_back.insert(id, src); + self.source_map.pat_map_back.insert(id, src.map(AstPtr::wrap_right)); self.source_map.pat_map.insert(src, id); id } diff --git a/crates/hir-def/src/body/scope.rs b/crates/hir-def/src/body/scope.rs index 56d6ed2330..c6967961b3 100644 --- a/crates/hir-def/src/body/scope.rs +++ b/crates/hir-def/src/body/scope.rs @@ -333,6 +333,8 @@ mod tests { let expr_id = source_map .node_expr(InFile { file_id: file_id.into(), value: &marker.into() }) + .unwrap() + .as_expr() .unwrap(); let scope = scopes.scope_for(expr_id); @@ -488,8 +490,11 @@ fn foo() { let expr_scope = { let expr_ast = name_ref.syntax().ancestors().find_map(ast::Expr::cast).unwrap(); - let expr_id = - source_map.node_expr(InFile { file_id: file_id.into(), value: &expr_ast }).unwrap(); + let expr_id = source_map + .node_expr(InFile { file_id: file_id.into(), value: &expr_ast }) + .unwrap() + .as_expr() + .unwrap(); scopes.scope_for(expr_id).unwrap() }; diff --git a/crates/hir-def/src/hir.rs b/crates/hir-def/src/hir.rs index 3c62b04f97..a575a2d199 100644 --- a/crates/hir-def/src/hir.rs +++ b/crates/hir-def/src/hir.rs @@ -48,6 +48,22 @@ pub enum ExprOrPatId { ExprId(ExprId), PatId(PatId), } + +impl ExprOrPatId { + pub fn as_expr(self) -> Option { + match self { + Self::ExprId(v) => Some(v), + _ => None, + } + } + + pub fn as_pat(self) -> Option { + match self { + Self::PatId(v) => Some(v), + _ => None, + } + } +} stdx::impl_from!(ExprId, PatId for ExprOrPatId); #[derive(Debug, Clone, Eq, PartialEq)] diff --git a/crates/hir-def/src/test_db.rs b/crates/hir-def/src/test_db.rs index 4db21eb46b..0c36c88fb0 100644 --- a/crates/hir-def/src/test_db.rs +++ b/crates/hir-def/src/test_db.rs @@ -198,7 +198,10 @@ impl TestDB { .filter_map(|node| { let block = ast::BlockExpr::cast(node)?; let expr = ast::Expr::from(block); - let expr_id = source_map.node_expr(InFile::new(position.file_id.into(), &expr))?; + let expr_id = source_map + .node_expr(InFile::new(position.file_id.into(), &expr))? + .as_expr() + .unwrap(); let scope = scopes.scope_for(expr_id).unwrap(); Some(scope) }); diff --git a/crates/hir-ty/src/diagnostics/unsafe_check.rs b/crates/hir-ty/src/diagnostics/unsafe_check.rs index f610c9e96c..492262c7a4 100644 --- a/crates/hir-ty/src/diagnostics/unsafe_check.rs +++ b/crates/hir-ty/src/diagnostics/unsafe_check.rs @@ -3,7 +3,7 @@ use hir_def::{ body::Body, - hir::{Expr, ExprId, UnaryOp}, + hir::{Expr, ExprId, ExprOrPatId, Pat, UnaryOp}, resolver::{resolver_for_expr, ResolveValueResult, Resolver, ValueNs}, type_ref::Rawness, DefWithBodyId, @@ -16,7 +16,7 @@ use crate::{ /// Returns `(unsafe_exprs, fn_is_unsafe)`. /// /// If `fn_is_unsafe` is false, `unsafe_exprs` are hard errors. If true, they're `unsafe_op_in_unsafe_fn`. -pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec, bool) { +pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec, bool) { let _p = tracing::info_span!("missing_unsafe").entered(); let mut res = Vec::new(); @@ -32,7 +32,7 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec, let infer = db.infer(def); unsafe_expressions(db, &infer, def, &body, body.body_expr, &mut |expr| { if !expr.inside_unsafe_block { - res.push(expr.expr); + res.push(expr.node); } }); @@ -40,7 +40,7 @@ pub fn missing_unsafe(db: &dyn HirDatabase, def: DefWithBodyId) -> (Vec, } pub struct UnsafeExpr { - pub expr: ExprId, + pub node: ExprOrPatId, pub inside_unsafe_block: bool, } @@ -75,26 +75,28 @@ fn walk_unsafe( inside_unsafe_block: bool, unsafe_expr_cb: &mut dyn FnMut(UnsafeExpr), ) { + let mut mark_unsafe_path = |path, node| { + let g = resolver.update_to_inner_scope(db.upcast(), def, current); + let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path); + if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial { + let static_data = db.static_data(id); + if static_data.mutable || (static_data.is_extern && !static_data.has_safe_kw) { + unsafe_expr_cb(UnsafeExpr { node, inside_unsafe_block }); + } + } + resolver.reset_to_guard(g); + }; + let expr = &body.exprs[current]; match expr { &Expr::Call { callee, .. } => { if let Some(func) = infer[callee].as_fn_def(db) { if is_fn_unsafe_to_call(db, func) { - unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block }); + unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block }); } } } - Expr::Path(path) => { - let g = resolver.update_to_inner_scope(db.upcast(), def, current); - let value_or_partial = resolver.resolve_path_in_value_ns(db.upcast(), path); - if let Some(ResolveValueResult::ValueNs(ValueNs::StaticId(id), _)) = value_or_partial { - let static_data = db.static_data(id); - if static_data.mutable || (static_data.is_extern && !static_data.has_safe_kw) { - unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block }); - } - } - resolver.reset_to_guard(g); - } + Expr::Path(path) => mark_unsafe_path(path, current.into()), Expr::Ref { expr, rawness: Rawness::RawPtr, mutability: _ } => { if let Expr::Path(_) = body.exprs[*expr] { // Do not report unsafe for `addr_of[_mut]!(EXTERN_OR_MUT_STATIC)`, @@ -108,12 +110,12 @@ fn walk_unsafe( .map(|(func, _)| is_fn_unsafe_to_call(db, func)) .unwrap_or(false) { - unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block }); + unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block }); } } Expr::UnaryOp { expr, op: UnaryOp::Deref } => { if let TyKind::Raw(..) = &infer[*expr].kind(Interner) { - unsafe_expr_cb(UnsafeExpr { expr: current, inside_unsafe_block }); + unsafe_expr_cb(UnsafeExpr { node: current.into(), inside_unsafe_block }); } } Expr::Unsafe { .. } => { @@ -121,6 +123,13 @@ fn walk_unsafe( walk_unsafe(db, infer, body, resolver, def, child, true, unsafe_expr_cb); }); } + &Expr::Assignment { target, value: _ } => { + body.walk_pats(target, &mut |pat| { + if let Pat::Path(path) = &body[pat] { + mark_unsafe_path(path, pat.into()); + } + }); + } _ => {} } diff --git a/crates/hir-ty/src/infer.rs b/crates/hir-ty/src/infer.rs index f2c860c9eb..db16899d9f 100644 --- a/crates/hir-ty/src/infer.rs +++ b/crates/hir-ty/src/infer.rs @@ -482,12 +482,27 @@ impl InferenceResult { pub fn variant_resolution_for_pat(&self, id: PatId) -> Option { self.variant_resolutions.get(&id.into()).copied() } + pub fn variant_resolution_for_expr_or_pat(&self, id: ExprOrPatId) -> Option { + match id { + ExprOrPatId::ExprId(id) => self.variant_resolution_for_expr(id), + ExprOrPatId::PatId(id) => self.variant_resolution_for_pat(id), + } + } pub fn assoc_resolutions_for_expr(&self, id: ExprId) -> Option<(AssocItemId, Substitution)> { self.assoc_resolutions.get(&id.into()).cloned() } pub fn assoc_resolutions_for_pat(&self, id: PatId) -> Option<(AssocItemId, Substitution)> { self.assoc_resolutions.get(&id.into()).cloned() } + pub fn assoc_resolutions_for_expr_or_pat( + &self, + id: ExprOrPatId, + ) -> Option<(AssocItemId, Substitution)> { + match id { + ExprOrPatId::ExprId(id) => self.assoc_resolutions_for_expr(id), + ExprOrPatId::PatId(id) => self.assoc_resolutions_for_pat(id), + } + } pub fn type_mismatch_for_expr(&self, expr: ExprId) -> Option<&TypeMismatch> { self.type_mismatches.get(&expr.into()) } @@ -506,6 +521,12 @@ impl InferenceResult { pub fn closure_info(&self, closure: &ClosureId) -> &(Vec, FnTrait) { self.closure_info.get(closure).unwrap() } + pub fn type_of_expr_or_pat(&self, id: ExprOrPatId) -> Option<&Ty> { + match id { + ExprOrPatId::ExprId(id) => self.type_of_expr.get(id), + ExprOrPatId::PatId(id) => self.type_of_pat.get(id), + } + } } impl Index for InferenceResult { @@ -524,6 +545,14 @@ impl Index for InferenceResult { } } +impl Index for InferenceResult { + type Output = Ty; + + fn index(&self, id: ExprOrPatId) -> &Ty { + self.type_of_expr_or_pat(id).unwrap_or(&self.standard_types.unknown) + } +} + impl Index for InferenceResult { type Output = Ty; diff --git a/crates/hir-ty/src/tests/simple.rs b/crates/hir-ty/src/tests/simple.rs index a8170b6060..e6ef56b80d 100644 --- a/crates/hir-ty/src/tests/simple.rs +++ b/crates/hir-ty/src/tests/simple.rs @@ -3418,11 +3418,11 @@ struct TS(usize); fn main() { let x; [x,] = &[1,]; - //^^^^expected &'? [i32; 1], got [{unknown}; _] + //^^^^expected &'? [i32; 1], got [{unknown}] let x; [(x,),] = &[(1,),]; - //^^^^^^^expected &'? [(i32,); 1], got [{unknown}; _] + //^^^^^^^expected &'? [(i32,); 1], got [{unknown}] let x; ((x,),) = &((1,),); diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 24c1b80bdf..2fedffe047 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -257,7 +257,7 @@ pub struct PrivateField { #[derive(Debug)] pub struct MissingUnsafe { - pub expr: InFile>, + pub expr: InFile>>, /// If true, the diagnostics is an `unsafe_op_in_unsafe_fn` lint instead of a hard error. pub only_lint: bool, } @@ -398,56 +398,46 @@ impl AnyDiagnostic { .map(|idx| variant_data.fields()[idx].name.clone()) .collect(); - match record { - Either::Left(record_expr) => match source_map.expr_syntax(record_expr) { - Ok(source_ptr) => { - let root = source_ptr.file_syntax(db.upcast()); - if let ast::Expr::RecordExpr(record_expr) = - source_ptr.value.to_node(&root) - { - if record_expr.record_expr_field_list().is_some() { - let field_list_parent_path = - record_expr.path().map(|path| AstPtr::new(&path)); - return Some( - MissingFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(&Either::Left( - record_expr, - )), - field_list_parent_path, - missed_fields, - } - .into(), - ); + let record = match record { + Either::Left(record_expr) => { + source_map.expr_syntax(record_expr).ok()?.map(AstPtr::wrap_left) + } + Either::Right(record_pat) => source_map.pat_syntax(record_pat).ok()?, + }; + let file = record.file_id; + let root = record.file_syntax(db.upcast()); + match record.value.to_node(&root) { + Either::Left(ast::Expr::RecordExpr(record_expr)) => { + if record_expr.record_expr_field_list().is_some() { + let field_list_parent_path = + record_expr.path().map(|path| AstPtr::new(&path)); + return Some( + MissingFields { + file, + field_list_parent: AstPtr::new(&Either::Left(record_expr)), + field_list_parent_path, + missed_fields, } - } + .into(), + ); } - Err(SyntheticSyntax) => (), - }, - Either::Right(record_pat) => match source_map.pat_syntax(record_pat) { - Ok(source_ptr) => { - if let Some(ptr) = source_ptr.value.cast::() { - let root = source_ptr.file_syntax(db.upcast()); - let record_pat = ptr.to_node(&root); - if record_pat.record_pat_field_list().is_some() { - let field_list_parent_path = - record_pat.path().map(|path| AstPtr::new(&path)); - return Some( - MissingFields { - file: source_ptr.file_id, - field_list_parent: AstPtr::new(&Either::Right( - record_pat, - )), - field_list_parent_path, - missed_fields, - } - .into(), - ); + } + Either::Right(ast::Pat::RecordPat(record_pat)) => { + if record_pat.record_pat_field_list().is_some() { + let field_list_parent_path = + record_pat.path().map(|path| AstPtr::new(&path)); + return Some( + MissingFields { + file, + field_list_parent: AstPtr::new(&Either::Right(record_pat)), + field_list_parent_path, + missed_fields, } - } + .into(), + ); } - Err(SyntheticSyntax) => (), - }, + } + _ => {} } } BodyValidationDiagnostic::ReplaceFilterMapNextWithFindMap { method_call_expr } => { @@ -543,7 +533,7 @@ impl AnyDiagnostic { }; 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).map(|it| it.map(AstPtr::wrap_right)), + ExprOrPatId::PatId(pat) => pat_syntax(pat), }; Some(match d { &InferenceDiagnostic::NoSuchField { field: expr, private, variant } => { @@ -551,9 +541,7 @@ impl AnyDiagnostic { ExprOrPatId::ExprId(expr) => { source_map.field_syntax(expr).map(AstPtr::wrap_left) } - ExprOrPatId::PatId(pat) => { - source_map.pat_field_syntax(pat).map(AstPtr::wrap_right) - } + ExprOrPatId::PatId(pat) => source_map.pat_field_syntax(pat), }; NoSuchField { field: expr_or_pat, private, variant }.into() } diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index 30e023e1a4..4a18795e7d 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -1885,7 +1885,7 @@ impl DefWithBody { let (unafe_exprs, only_lint) = hir_ty::diagnostics::missing_unsafe(db, self.into()); for expr in unafe_exprs { - match source_map.expr_syntax(expr) { + match source_map.expr_or_pat_syntax(expr) { Ok(expr) => acc.push(MissingUnsafe { expr, only_lint }.into()), Err(SyntheticSyntax) => { // FIXME: Here and elsewhere in this file, the `expr` was @@ -3481,7 +3481,7 @@ impl Local { LocalSource { local: self, source: src.map(|ast| match ast.to_node(&root) { - ast::Pat::IdentPat(it) => Either::Left(it), + Either::Right(ast::Pat::IdentPat(it)) => Either::Left(it), _ => unreachable!("local with non ident-pattern"), }), } @@ -3510,7 +3510,7 @@ impl Local { LocalSource { local: self, source: src.map(|ast| match ast.to_node(&root) { - ast::Pat::IdentPat(it) => Either::Left(it), + Either::Right(ast::Pat::IdentPat(it)) => Either::Left(it), _ => unreachable!("local with non ident-pattern"), }), } @@ -4235,10 +4235,7 @@ impl CaptureUsages { } mir::MirSpan::PatId(pat) => { if let Ok(pat) = source_map.pat_syntax(pat) { - result.push(CaptureUsageSource { - is_ref, - source: pat.map(AstPtr::wrap_right), - }); + result.push(CaptureUsageSource { is_ref, source: pat }); } } mir::MirSpan::BindingId(binding) => result.extend( @@ -4246,10 +4243,7 @@ impl CaptureUsages { .patterns_for_binding(binding) .iter() .filter_map(|&pat| source_map.pat_syntax(pat).ok()) - .map(|pat| CaptureUsageSource { - is_ref, - source: pat.map(AstPtr::wrap_right), - }), + .map(|pat| CaptureUsageSource { is_ref, source: pat }), ), mir::MirSpan::SelfParam | mir::MirSpan::Unknown => { unreachable!("invalid capture usage span") diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs index b27f1fbb5d..d210c26714 100644 --- a/crates/hir/src/semantics.rs +++ b/crates/hir/src/semantics.rs @@ -11,7 +11,7 @@ use std::{ use either::Either; use hir_def::{ - hir::Expr, + hir::{Expr, ExprOrPatId}, lower::LowerCtx, nameres::{MacroSubNs, ModuleOrigin}, path::ModPath, @@ -1755,7 +1755,9 @@ impl<'db> SemanticsImpl<'db> { } if let Some(parent) = ast::Expr::cast(parent.clone()) { - if let Some(expr_id) = source_map.node_expr(InFile { file_id, value: &parent }) { + if let Some(ExprOrPatId::ExprId(expr_id)) = + source_map.node_expr(InFile { file_id, value: &parent }) + { if let Expr::Unsafe { .. } = body[expr_id] { break true; } diff --git a/crates/hir/src/semantics/source_to_def.rs b/crates/hir/src/semantics/source_to_def.rs index fd6d52d6c9..e53a7da7ed 100644 --- a/crates/hir/src/semantics/source_to_def.rs +++ b/crates/hir/src/semantics/source_to_def.rs @@ -306,7 +306,7 @@ impl SourceToDefCtx<'_, '_> { .position(|it| it == *src.value)?; let container = self.find_pat_or_label_container(src.syntax_ref())?; let (_, source_map) = self.db.body_with_source_map(container); - let expr = source_map.node_expr(src.with_value(&ast::Expr::AsmExpr(asm)))?; + let expr = source_map.node_expr(src.with_value(&ast::Expr::AsmExpr(asm)))?.as_expr()?; Some(InlineAsmOperand { owner: container, expr, index }) } @@ -350,7 +350,8 @@ impl SourceToDefCtx<'_, '_> { let break_or_continue = ast::Expr::cast(src.value.syntax().parent()?)?; let container = self.find_pat_or_label_container(src.syntax_ref())?; let (body, source_map) = self.db.body_with_source_map(container); - let break_or_continue = source_map.node_expr(src.with_value(&break_or_continue))?; + let break_or_continue = + source_map.node_expr(src.with_value(&break_or_continue))?.as_expr()?; let (Expr::Break { label, .. } | Expr::Continue { label }) = body[break_or_continue] else { return None; }; diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs index 3da67ae23f..f2f27517fd 100644 --- a/crates/hir/src/source_analyzer.rs +++ b/crates/hir/src/source_analyzer.rs @@ -13,7 +13,7 @@ use hir_def::{ scope::{ExprScopes, ScopeId}, Body, BodySourceMap, }, - hir::{BindingId, ExprId, Pat, PatId}, + hir::{BindingId, ExprId, ExprOrPatId, Pat, PatId}, lang_item::LangItem, lower::LowerCtx, nameres::MacroSubNs, @@ -120,7 +120,7 @@ impl SourceAnalyzer { self.def.as_ref().map(|(_, body, _)| &**body) } - fn expr_id(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option { + fn expr_id(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option { let src = match expr { ast::Expr::MacroExpr(expr) => { self.expand_expr(db, InFile::new(self.file_id, expr.macro_call()?))?.into() @@ -174,7 +174,9 @@ impl SourceAnalyzer { db: &dyn HirDatabase, expr: &ast::Expr, ) -> Option<&[Adjustment]> { - let expr_id = self.expr_id(db, expr)?; + // It is safe to omit destructuring assignments here because they have no adjustments (neither + // expressions nor patterns). + let expr_id = self.expr_id(db, expr)?.as_expr()?; let infer = self.infer.as_ref()?; infer.expr_adjustments.get(&expr_id).map(|v| &**v) } @@ -186,9 +188,9 @@ impl SourceAnalyzer { ) -> Option<(Type, Option)> { let expr_id = self.expr_id(db, expr)?; let infer = self.infer.as_ref()?; - let coerced = infer - .expr_adjustments - .get(&expr_id) + let coerced = expr_id + .as_expr() + .and_then(|expr_id| infer.expr_adjustments.get(&expr_id)) .and_then(|adjusts| adjusts.last().map(|adjust| adjust.target.clone())); let ty = infer[expr_id].clone(); let mk_ty = |ty| Type::new_with_resolver(db, &self.resolver, ty); @@ -268,7 +270,7 @@ impl SourceAnalyzer { db: &dyn HirDatabase, call: &ast::MethodCallExpr, ) -> Option { - let expr_id = self.expr_id(db, &call.clone().into())?; + let expr_id = self.expr_id(db, &call.clone().into())?.as_expr()?; let (func, substs) = self.infer.as_ref()?.method_resolution(expr_id)?; let ty = db.value_ty(func.into())?.substitute(Interner, &substs); let ty = Type::new_with_resolver(db, &self.resolver, ty); @@ -282,7 +284,7 @@ impl SourceAnalyzer { db: &dyn HirDatabase, call: &ast::MethodCallExpr, ) -> Option { - let expr_id = self.expr_id(db, &call.clone().into())?; + let expr_id = self.expr_id(db, &call.clone().into())?.as_expr()?; let (f_in_trait, substs) = self.infer.as_ref()?.method_resolution(expr_id)?; Some(self.resolve_impl_method_or_trait_def(db, f_in_trait, substs).into()) @@ -293,7 +295,7 @@ impl SourceAnalyzer { db: &dyn HirDatabase, call: &ast::MethodCallExpr, ) -> Option> { - let expr_id = self.expr_id(db, &call.clone().into())?; + let expr_id = self.expr_id(db, &call.clone().into())?.as_expr()?; let inference_result = self.infer.as_ref()?; match inference_result.method_resolution(expr_id) { Some((f_in_trait, substs)) => Some(Either::Left( @@ -322,7 +324,7 @@ impl SourceAnalyzer { field: &ast::FieldExpr, ) -> Option> { let &(def, ..) = self.def.as_ref()?; - let expr_id = self.expr_id(db, &field.clone().into())?; + let expr_id = self.expr_id(db, &field.clone().into())?.as_expr()?; self.infer.as_ref()?.field_resolution(expr_id).map(|it| { it.map_either(Into::into, |f| TupleField { owner: def, tuple: f.tuple, index: f.index }) }) @@ -334,7 +336,7 @@ impl SourceAnalyzer { field: &ast::FieldExpr, ) -> Option, Function>> { let &(def, ..) = self.def.as_ref()?; - let expr_id = self.expr_id(db, &field.clone().into())?; + let expr_id = self.expr_id(db, &field.clone().into())?.as_expr()?; let inference_result = self.infer.as_ref()?; match inference_result.field_resolution(expr_id) { Some(field) => Some(Either::Left(field.map_either(Into::into, |f| TupleField { @@ -403,7 +405,7 @@ impl SourceAnalyzer { self.infer .as_ref() .and_then(|infer| { - let expr = self.expr_id(db, &prefix_expr.clone().into())?; + let expr = self.expr_id(db, &prefix_expr.clone().into())?.as_expr()?; let (func, _) = infer.method_resolution(expr)?; let (deref_mut_trait, deref_mut) = self.lang_trait_fn( db, @@ -449,7 +451,7 @@ impl SourceAnalyzer { .infer .as_ref() .and_then(|infer| { - let expr = self.expr_id(db, &index_expr.clone().into())?; + let expr = self.expr_id(db, &index_expr.clone().into())?.as_expr()?; let (func, _) = infer.method_resolution(expr)?; let (index_mut_trait, index_mut_fn) = self.lang_trait_fn( db, @@ -537,8 +539,8 @@ impl SourceAnalyzer { _ => None, } }; - let (_, subst) = self.infer.as_ref()?.type_of_expr.get(expr_id)?.as_adt()?; - let variant = self.infer.as_ref()?.variant_resolution_for_expr(expr_id)?; + let (_, subst) = self.infer.as_ref()?.type_of_expr_or_pat(expr_id)?.as_adt()?; + let variant = self.infer.as_ref()?.variant_resolution_for_expr_or_pat(expr_id)?; let variant_data = variant.variant_data(db.upcast()); let field = FieldId { parent: variant, local_id: variant_data.field(&local_name)? }; let field_ty = @@ -606,10 +608,10 @@ impl SourceAnalyzer { let infer = self.infer.as_deref()?; if let Some(path_expr) = parent().and_then(ast::PathExpr::cast) { let expr_id = self.expr_id(db, &path_expr.into())?; - if let Some((assoc, subs)) = infer.assoc_resolutions_for_expr(expr_id) { + if let Some((assoc, subs)) = infer.assoc_resolutions_for_expr_or_pat(expr_id) { let assoc = match assoc { AssocItemId::FunctionId(f_in_trait) => { - match infer.type_of_expr.get(expr_id) { + match infer.type_of_expr_or_pat(expr_id) { None => assoc, Some(func_ty) => { if let TyKind::FnDef(_fn_def, subs) = func_ty.kind(Interner) { @@ -634,7 +636,7 @@ impl SourceAnalyzer { return Some(PathResolution::Def(AssocItem::from(assoc).into())); } if let Some(VariantId::EnumVariantId(variant)) = - infer.variant_resolution_for_expr(expr_id) + infer.variant_resolution_for_expr_or_pat(expr_id) { return Some(PathResolution::Def(ModuleDef::Variant(variant.into()))); } @@ -658,7 +660,7 @@ impl SourceAnalyzer { } else if let Some(rec_lit) = parent().and_then(ast::RecordExpr::cast) { let expr_id = self.expr_id(db, &rec_lit.into())?; if let Some(VariantId::EnumVariantId(variant)) = - infer.variant_resolution_for_expr(expr_id) + infer.variant_resolution_for_expr_or_pat(expr_id) { return Some(PathResolution::Def(ModuleDef::Variant(variant.into()))); } @@ -790,10 +792,16 @@ impl SourceAnalyzer { let infer = self.infer.as_ref()?; let expr_id = self.expr_id(db, &literal.clone().into())?; - let substs = infer.type_of_expr[expr_id].as_adt()?.1; + let substs = infer[expr_id].as_adt()?.1; - let (variant, missing_fields, _exhaustive) = - record_literal_missing_fields(db, infer, expr_id, &body[expr_id])?; + let (variant, missing_fields, _exhaustive) = match expr_id { + ExprOrPatId::ExprId(expr_id) => { + record_literal_missing_fields(db, infer, expr_id, &body[expr_id])? + } + ExprOrPatId::PatId(pat_id) => { + record_pattern_missing_fields(db, infer, pat_id, &body[pat_id])? + } + }; let res = self.missing_fields(db, substs, variant, missing_fields); Some(res) } @@ -856,7 +864,7 @@ impl SourceAnalyzer { ) -> Option { let infer = self.infer.as_ref()?; let expr_id = self.expr_id(db, &record_lit.into())?; - infer.variant_resolution_for_expr(expr_id) + infer.variant_resolution_for_expr_or_pat(expr_id) } pub(crate) fn is_unsafe_macro_call_expr( @@ -867,14 +875,24 @@ impl SourceAnalyzer { if let (Some((def, body, sm)), Some(infer)) = (&self.def, &self.infer) { if let Some(expanded_expr) = sm.macro_expansion_expr(macro_expr) { let mut is_unsafe = false; - unsafe_expressions( - db, - infer, - *def, - body, - expanded_expr, - &mut |UnsafeExpr { inside_unsafe_block, .. }| is_unsafe |= !inside_unsafe_block, - ); + let mut walk_expr = |expr_id| { + unsafe_expressions( + db, + infer, + *def, + body, + expr_id, + &mut |UnsafeExpr { inside_unsafe_block, .. }| { + is_unsafe |= !inside_unsafe_block + }, + ) + }; + match expanded_expr { + ExprOrPatId::ExprId(expanded_expr) => walk_expr(expanded_expr), + ExprOrPatId::PatId(expanded_pat) => { + body.walk_exprs_in_pat(expanded_pat, &mut walk_expr) + } + } return is_unsafe; } } @@ -991,7 +1009,7 @@ impl SourceAnalyzer { } fn ty_of_expr(&self, db: &dyn HirDatabase, expr: &ast::Expr) -> Option<&Ty> { - self.infer.as_ref()?.type_of_expr.get(self.expr_id(db, expr)?) + self.infer.as_ref()?.type_of_expr_or_pat(self.expr_id(db, expr)?) } } @@ -1004,7 +1022,7 @@ fn scope_for( node.ancestors_with_macros(db.upcast()) .take_while(|it| !ast::Item::can_cast(it.kind()) || ast::MacroCall::can_cast(it.kind())) .filter_map(|it| it.map(ast::Expr::cast).transpose()) - .filter_map(|it| source_map.node_expr(it.as_ref())) + .filter_map(|it| source_map.node_expr(it.as_ref())?.as_expr()) .find_map(|it| scopes.scope_for(it)) } diff --git a/crates/ide-diagnostics/src/handlers/missing_fields.rs b/crates/ide-diagnostics/src/handlers/missing_fields.rs index 86c237f7b5..3a622c6968 100644 --- a/crates/ide-diagnostics/src/handlers/missing_fields.rs +++ b/crates/ide-diagnostics/src/handlers/missing_fields.rs @@ -308,22 +308,27 @@ struct T(S); fn regular(a: S) { let s; S { s, .. } = a; + _ = s; } fn nested(a: S2) { let s; S2 { s: S { s, .. }, .. } = a; + _ = s; } fn in_tuple(a: (S,)) { let s; (S { s, .. },) = a; + _ = s; } fn in_array(a: [S;1]) { let s; [S { s, .. },] = a; + _ = s; } fn in_tuple_struct(a: T) { let s; T(S { s, .. }) = a; + _ = s; } ", ); diff --git a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs index cc0f4bfccc..98063bf4fe 100644 --- a/crates/ide-diagnostics/src/handlers/missing_unsafe.rs +++ b/crates/ide-diagnostics/src/handlers/missing_unsafe.rs @@ -32,7 +32,8 @@ fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingUnsafe) -> Option, d: &hir::NeedMut) -> Option { + let root = ctx.sema.db.parse_or_expand(d.span.file_id); + let node = d.span.value.to_node(&root); + let mut span = d.span; + if let Some(parent) = node.parent() { + if ast::BinExpr::can_cast(parent.kind()) { + // In case of an assignment, the diagnostic is provided on the variable name. + // We want to expand it to include the whole assignment, but only when this + // is an ordinary assignment, not a destructuring assignment. So, the direct + // parent is an assignment expression. + span = d.span.with_value(SyntaxNodePtr::new(&parent)); + } + }; + let fixes = (|| { if d.local.is_ref(ctx.sema.db) { // There is no simple way to add `mut` to `ref x` and `ref mut x` return None; } - let file_id = d.span.file_id.file_id()?; + let file_id = span.file_id.file_id()?; let mut edit_builder = TextEdit::builder(); - let use_range = d.span.value.text_range(); + let use_range = span.value.text_range(); for source in d.local.sources(ctx.sema.db) { let Some(ast) = source.name() else { continue }; // FIXME: macros @@ -29,6 +43,7 @@ pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Option use_range, )]) })(); + Some( Diagnostic::new_with_syntax_node_ptr( ctx, @@ -38,7 +53,7 @@ pub(crate) fn need_mut(ctx: &DiagnosticsContext<'_>, d: &hir::NeedMut) -> Option "cannot mutate immutable variable `{}`", d.local.name(ctx.sema.db).display(ctx.sema.db, ctx.edition) ), - d.span, + span, ) .with_fixes(fixes), ) @@ -929,7 +944,6 @@ fn fn_once(mut x: impl FnOnce(u8) -> u8) -> u8 { #[test] fn closure() { - // FIXME: Diagnostic spans are inconsistent inside and outside closure check_diagnostics( r#" //- minicore: copy, fn @@ -942,11 +956,11 @@ fn fn_once(mut x: impl FnOnce(u8) -> u8) -> u8 { fn f() { let x = 5; let closure1 = || { x = 2; }; - //^ 💡 error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` let _ = closure1(); //^^^^^^^^ 💡 error: cannot mutate immutable variable `closure1` let closure2 = || { x = x; }; - //^ 💡 error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` let closure3 = || { let x = 2; x = 5; @@ -988,7 +1002,7 @@ fn f() { || { let x = 2; || { || { x = 5; } } - //^ 💡 error: cannot mutate immutable variable `x` + //^^^^^ 💡 error: cannot mutate immutable variable `x` } } };