Merge #2500 #2505 #2506

2500: Fix format_args expansion & go to definition r=matklad a=flodiebold

The expansion of format_args wasn't yet correct enough to type-check. Also make macros in statement position expand to expressions for now, since it's not handled correctly in HIR lowering yet. This finally fixes go to definition within print macros, I think 🙂 

2505: Remove more dead code r=matklad a=matklad



2506: Remove one more Ty r=matklad a=matklad



Co-authored-by: Florian Diebold <flodiebold@gmail.com>
Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_assists/src/test_db.rs

@@ -43,5 +43,3 @@ impl FileLoader for TestDB {
         FileLoaderDelegate(self).relevant_crates(file_id)
     }
 }
-
-impl hir::debug::HirDebugHelper for TestDB {}

crates/ra_hir/src/code_model.rs

@@ -1,7 +1,4 @@
 //! FIXME: write short doc here
-
-pub(crate) mod src;
-
 use std::sync::Arc;
 
 use either::Either;
@@ -989,11 +986,6 @@ impl Type {
         None
     }
 
-    // FIXME: remove
-    pub fn into_ty(self) -> Ty {
-        self.ty.value
-    }
-
     pub fn as_adt(&self) -> Option<Adt> {
         let (adt, _subst) = self.ty.value.as_adt()?;
         Some(adt.into())

crates/ra_hir/src/debug.rs

@@ -1,94 +0,0 @@
-//! XXX: This does not work at the moment.
-//!
-//! printf debugging infrastructure for rust-analyzer.
-//!
-//! When you print a hir type, like a module, using `eprintln!("{:?}", module)`,
-//! you usually get back a numeric ID, which doesn't tell you much:
-//! `Module(92)`.
-//!
-//! This module adds convenience `debug` methods to various types, which resolve
-//! the id to a human-readable location info:
-//!
-//! ```not_rust
-//! eprintln!("{:?}", module.debug(db));
-//! =>
-//! Module { name: collections, path: "liballoc/collections/mod.rs" }
-//! ```
-//!
-//! Note that to get this info, we might need to execute queries! So
-//!
-//! * don't use the `debug` methods for logging
-//! * when debugging, be aware that interference is possible.
-
-use std::fmt;
-
-use hir_expand::HirFileId;
-use ra_db::{CrateId, FileId};
-
-use crate::{db::HirDatabase, Crate, Module, Name};
-
-impl Crate {
-    pub fn debug(self, db: &impl HirDebugDatabase) -> impl fmt::Debug + '_ {
-        debug_fn(move |fmt| db.debug_crate(self, fmt))
-    }
-}
-
-impl Module {
-    pub fn debug(self, db: &impl HirDebugDatabase) -> impl fmt::Debug + '_ {
-        debug_fn(move |fmt| db.debug_module(self, fmt))
-    }
-}
-
-pub trait HirDebugHelper: HirDatabase {
-    fn crate_name(&self, _krate: CrateId) -> Option<String> {
-        None
-    }
-    fn file_path(&self, _file_id: FileId) -> Option<String> {
-        None
-    }
-}
-
-pub trait HirDebugDatabase {
-    fn debug_crate(&self, krate: Crate, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
-    fn debug_module(&self, module: Module, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
-    fn debug_hir_file_id(&self, file_id: HirFileId, fmt: &mut fmt::Formatter<'_>) -> fmt::Result;
-}
-
-impl<DB: HirDebugHelper> HirDebugDatabase for DB {
-    fn debug_crate(&self, krate: Crate, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let mut builder = fmt.debug_tuple("Crate");
-        match self.crate_name(krate.id) {
-            Some(name) => builder.field(&name),
-            None => builder.field(&krate.id),
-        }
-        .finish()
-    }
-
-    fn debug_module(&self, module: Module, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let file_id = module.definition_source(self).file_id.original_file(self);
-        let path = self.file_path(file_id).unwrap_or_else(|| "N/A".to_string());
-        fmt.debug_struct("Module")
-            .field("name", &module.name(self).unwrap_or_else(Name::missing))
-            .field("path", &path)
-            .finish()
-    }
-
-    fn debug_hir_file_id(&self, file_id: HirFileId, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        let original = file_id.original_file(self);
-        let path = self.file_path(original).unwrap_or_else(|| "N/A".to_string());
-        let is_macro = file_id != original.into();
-        fmt.debug_struct("HirFileId").field("path", &path).field("macro", &is_macro).finish()
-    }
-}
-
-fn debug_fn(f: impl Fn(&mut fmt::Formatter<'_>) -> fmt::Result) -> impl fmt::Debug {
-    struct DebugFn<F>(F);
-
-    impl<F: Fn(&mut fmt::Formatter<'_>) -> fmt::Result> fmt::Debug for DebugFn<F> {
-        fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-            (&self.0)(fmt)
-        }
-    }
-
-    DebugFn(f)
-}

crates/ra_hir/src/lib.rs

@@ -26,8 +26,6 @@ macro_rules! impl_froms {
     }
 }
 
-pub mod debug;
-
 pub mod db;
 pub mod source_binder;
 
@@ -36,16 +34,18 @@ pub mod diagnostics;
 mod from_id;
 mod code_model;
 
-pub mod from_source;
+mod has_source;
+mod from_source;
 
 pub use crate::{
     code_model::{
-        src::HasSource, Adt, AssocItem, AttrDef, Const, Container, Crate, CrateDependency,
+        Adt, AssocItem, AttrDef, Const, Container, Crate, CrateDependency, DefWithBody, Docs, Enum,
-        DefWithBody, Docs, Enum, EnumVariant, FieldSource, Function, GenericDef, HasAttrs,
+        EnumVariant, FieldSource, Function, GenericDef, HasAttrs, ImplBlock, Import, Local,
-        ImplBlock, Import, Local, MacroDef, Module, ModuleDef, ScopeDef, Static, Struct,
+        MacroDef, Module, ModuleDef, ScopeDef, Static, Struct, StructField, Trait, Type, TypeAlias,
-        StructField, Trait, Type, TypeAlias, TypeParam, Union, VariantDef,
+        TypeParam, Union, VariantDef,
     },
     from_source::FromSource,
+    has_source::HasSource,
     source_binder::{PathResolution, ScopeEntryWithSyntax, SourceAnalyzer},
 };
 

crates/ra_hir/src/source_binder.rs

@@ -427,7 +427,7 @@ impl SourceAnalyzer {
 
     /// Checks that particular type `ty` implements `std::future::Future`.
     /// This function is used in `.await` syntax completion.
-    pub fn impls_future(&self, db: &impl HirDatabase, ty: Ty) -> bool {
+    pub fn impls_future(&self, db: &impl HirDatabase, ty: Type) -> bool {
         let std_future_path = known::std_future_future();
 
         let std_future_trait = match self.resolver.resolve_known_trait(db, &std_future_path) {
@@ -440,7 +440,7 @@ impl SourceAnalyzer {
             _ => return false,
         };
 
-        let canonical_ty = Canonical { value: ty, num_vars: 0 };
+        let canonical_ty = Canonical { value: ty.ty.value, num_vars: 0 };
         implements_trait(&canonical_ty, db, &self.resolver, krate.into(), std_future_trait)
     }
 

crates/ra_hir_def/src/body/lower.rs

@@ -437,9 +437,7 @@ where
                     None => self.alloc_expr(Expr::Missing, syntax_ptr),
                 }
             }
-
+            // FIXME expand to statements in statement position
-            // FIXME implement HIR for these:
-            ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
             ast::Expr::MacroCall(e) => match self.expander.enter_expand(self.db, e) {
                 Some((mark, expansion)) => {
                     let id = self.collect_expr(expansion);
@@ -448,6 +446,9 @@ where
                 }
                 None => self.alloc_expr(Expr::Missing, syntax_ptr),
             },
+
+            // FIXME implement HIR for these:
+            ast::Expr::Label(_e) => self.alloc_expr(Expr::Missing, syntax_ptr),
         }
     }
 

crates/ra_hir_expand/src/builtin_macro.rs

@@ -208,15 +208,20 @@ fn format_args_expand(
     _id: MacroCallId,
     tt: &tt::Subtree,
 ) -> Result<tt::Subtree, mbe::ExpandError> {
-    // We expand `format_args!("", arg1, arg2)` to
+    // We expand `format_args!("", a1, a2)` to
-    // `std::fmt::Arguments::new_v1(&[], &[&arg1, &arg2])`,
+    // ```
+    // std::fmt::Arguments::new_v1(&[], &[
+    //   std::fmt::ArgumentV1::new(&arg1,std::fmt::Display::fmt),
+    //   std::fmt::ArgumentV1::new(&arg2,std::fmt::Display::fmt),
+    // ])
+    // ```,
     // which is still not really correct, but close enough for now
     let mut args = Vec::new();
     let mut current = Vec::new();
     for tt in tt.token_trees.iter().cloned() {
         match tt {
             tt::TokenTree::Leaf(tt::Leaf::Punct(p)) if p.char == ',' => {
-                args.push(tt::Subtree { delimiter: tt::Delimiter::None, token_trees: current });
+                args.push(current);
                 current = Vec::new();
             }
             _ => {
@@ -225,13 +230,15 @@ fn format_args_expand(
         }
     }
     if !current.is_empty() {
-        args.push(tt::Subtree { delimiter: tt::Delimiter::None, token_trees: current });
+        args.push(current);
     }
     if args.is_empty() {
         return Err(mbe::ExpandError::NoMatchingRule);
     }
     let _format_string = args.remove(0);
-    let arg_tts = args.into_iter().flat_map(|arg| (quote! { & #arg , }).token_trees);
+    let arg_tts = args.into_iter().flat_map(|arg| {
+        quote! { std::fmt::ArgumentV1::new(&(##arg), std::fmt::Display::fmt), }
+    }.token_trees).collect::<Vec<_>>();
     let expanded = quote! {
         std::fmt::Arguments::new_v1(&[], &[##arg_tts])
     };
@@ -360,6 +367,6 @@ mod tests {
             BuiltinFnLikeExpander::FormatArgs,
         );
 
-        assert_eq!(expanded, r#"std::fmt::Arguments::new_v1(&[] ,&[&arg1(a,b,c),&arg2,])"#);
+        assert_eq!(expanded, r#"std::fmt::Arguments::new_v1(&[] ,&[std::fmt::ArgumentV1::new(&(arg1(a,b,c)),std::fmt::Display::fmt),std::fmt::ArgumentV1::new(&(arg2),std::fmt::Display::fmt),])"#);
     }
 }

crates/ra_hir_expand/src/db.rs

@@ -183,8 +183,8 @@ fn to_fragment_kind(db: &dyn AstDatabase, macro_call_id: MacroCallId) -> Fragmen
             // FIXME: Handle Pattern
             FragmentKind::Expr
         }
-        EXPR_STMT => FragmentKind::Statements,
+        // FIXME: Expand to statements in appropriate positions; HIR lowering needs to handle that
-        BLOCK => FragmentKind::Statements,
+        EXPR_STMT | BLOCK => FragmentKind::Expr,
         ARG_LIST => FragmentKind::Expr,
         TRY_EXPR => FragmentKind::Expr,
         TUPLE_EXPR => FragmentKind::Expr,

crates/ra_ide/src/completion/complete_dot.rs

@@ -27,7 +27,7 @@ pub(super) fn complete_dot(acc: &mut Completions, ctx: &CompletionContext) {
     complete_methods(acc, ctx, &receiver_ty);
 
     // Suggest .await syntax for types that implement Future trait
-    if ctx.analyzer.impls_future(ctx.db, receiver_ty.into_ty()) {
+    if ctx.analyzer.impls_future(ctx.db, receiver_ty) {
         CompletionItem::new(CompletionKind::Keyword, ctx.source_range(), "await")
             .detail("expr.await")
             .insert_text("await")

crates/ra_ide/src/db.rs

@@ -5,7 +5,7 @@ use std::sync::Arc;
 use ra_db::{
     salsa::{self, Database, Durability},
     Canceled, CheckCanceled, CrateId, FileId, FileLoader, FileLoaderDelegate, RelativePath,
-    SourceDatabase, SourceDatabaseExt, SourceRootId,
+    SourceDatabase, SourceRootId,
 };
 use rustc_hash::FxHashMap;
 
@@ -49,18 +49,6 @@ impl FileLoader for RootDatabase {
     }
 }
 
-impl hir::debug::HirDebugHelper for RootDatabase {
-    fn crate_name(&self, krate: CrateId) -> Option<String> {
-        self.debug_data.crate_names.get(&krate).cloned()
-    }
-    fn file_path(&self, file_id: FileId) -> Option<String> {
-        let source_root_id = self.file_source_root(file_id);
-        let source_root_path = self.debug_data.root_paths.get(&source_root_id)?;
-        let file_path = self.file_relative_path(file_id);
-        Some(format!("{}/{}", source_root_path, file_path))
-    }
-}
-
 impl salsa::Database for RootDatabase {
     fn salsa_runtime(&self) -> &salsa::Runtime<RootDatabase> {
         &self.runtime

crates/ra_ide/src/goto_definition.rs

@@ -693,7 +693,6 @@ mod tests {
         );
     }
 
-    #[should_panic] // currently failing because of expr mapping problems
     #[test]
     fn goto_through_format() {
         check_goto(
@@ -718,7 +717,7 @@ mod tests {
                 format!(\"{}\", fo<|>o())
             }
             ",
-            "foo FN_DEF FileId(1) [359; 376) [362; 365)",
+            "foo FN_DEF FileId(1) [398; 415) [401; 404)",
         );
     }