Merge pull request #18696 from ChayimFriedman2/syn-syn

feat: Report unresolved idents for implicit captures in `format_args!()`

Author: Veykril

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<ExprId, (HygieneId, Vec<(syntax::TextRange, Name)>)>,
-            // asm!
-            FxHashMap<ExprId, Vec<Vec<(syntax::TextRange, usize)>>>,
-        )>,
-    >,
+    template_map: Option<Box<FormatTemplate>>,
 
     expansions: FxHashMap<InFile<AstPtr<ast::MacroCall>>, MacroFileId>,
 
@@ -160,6 +153,20 @@ pub struct BodySourceMap {
     diagnostics: Vec<BodyDiagnostic>,
 }
 
+#[derive(Default, Debug, Eq, PartialEq)]
+struct FormatTemplate {
+    /// A map from `format_args!()` expressions to their captures.
+    format_args_to_captures: FxHashMap<ExprId, (HygieneId, Vec<(syntax::TextRange, Name)>)>,
+    /// A map from `asm!()` expressions to their captures.
+    asm_to_captures: FxHashMap<ExprId, Vec<Vec<(syntax::TextRange, usize)>>>,
+    /// 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<ExprId, InFile<(AstPtr<ast::Expr>, TextRange)>>,
+}
+
 #[derive(Debug, Eq, PartialEq)]
 pub enum BodyDiagnostic {
     InactiveCode { node: InFile<SyntaxNodePtr>, 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::<ast::Expr>);
-        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<InFile<(AstPtr<ast::Expr>, 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::<ast::Expr>);
         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();

crates/hir-def/src/body/lower.rs

@@ -1957,17 +1957,29 @@ 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(
                     &s,
                     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
     }

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
     }
 }

crates/hir-def/src/hir/format_args.rs

@@ -1,13 +1,14 @@
 //! Parses `format_args` input.
 
+use either::Either;
 use hir_expand::name::Name;
 use intern::Symbol;
 use rustc_parse_format as parse;
 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<usize, usize>,
+    pub index: Result<usize, Either<usize, Name>>,
     /// 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<FormatCount>,
@@ -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<String>,
     mut args: FormatArgumentsCollector,
     is_direct_literal: bool,
-    mut synth: impl FnMut(Name) -> ExprId,
+    mut synth: impl FnMut(Name, Option<TextRange>) -> ExprId,
     mut record_usage: impl FnMut(Name, Option<TextRange>),
     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),
-                    }))
                 }
             }
         };

crates/hir/src/diagnostics.rs

@@ -262,7 +262,7 @@ pub struct UnresolvedAssocItem {
 
 #[derive(Debug)]
 pub struct UnresolvedIdent {
-    pub expr_or_pat: InFile<AstPtr<Either<ast::Expr, ast::Pat>>>,
+    pub node: InFile<(AstPtr<Either<ast::Expr, ast::Pat>>, Option<TextRange>)>,
 }
 
 #[derive(Debug)]
@@ -550,11 +550,10 @@ impl AnyDiagnostic {
         source_map: &hir_def::body::BodySourceMap,
     ) -> Option<AnyDiagnostic> {
         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)?;

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<Vec<(TextRange, Option<Either<PathResolution, InlineAsmOperand>>)>> {
-        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<Either<PathResolution, InlineAsmOperand>>)> {
-        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)
         })