Repository: rust-analyzer/rust-analyzer
Pull request #5780 (2020-08-17 08:28:41)

Fixup whitespace when adding missing impl items
Generate properly formatted whitespace when adding impl items - with an empty line between items and removing extra whitespace that often appears at the end.

This is my first time working on rust analyzer so I'm not very familiar with its internal APIs. If there's a better way to do such syntax tree editing I'd be glad to hear it.


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index 24dea133dd..2386c8f3a5 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -162,9 +162,9 @@ checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822"
 
 [[package]]
 name = "chalk-derive"
-version = "0.23.0"
+version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "c3cb438e961fd7f1183dc5e0bdcfd09253bf9b90592cf665d1ce6787d8a4908f"
+checksum = "c1df0dbb57d74b4acd20f20fa66ab2acd09776b79eaeb9d8f947b2f3e01c40bf"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -174,9 +174,9 @@ dependencies = [
 
 [[package]]
 name = "chalk-ir"
-version = "0.23.0"
+version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bb332abfcb015b148c6fbab39b1d13282745b0f7f312019dd8e138f5f3f0855d"
+checksum = "44361a25dbdb1dc428f56ad7a3c21ba9ca12f3225c26a47919ff6fcb10a583d4"
 dependencies = [
  "chalk-derive",
  "lazy_static",
@@ -184,9 +184,9 @@ dependencies = [
 
 [[package]]
 name = "chalk-recursive"
-version = "0.23.0"
+version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "e7c7673f10c5fa1acf7fa07d4f4c5917cbcf161ed3a952d14530c79950de32d2"
+checksum = "dd89556b98de156d5eaf21077d297cd2198628f10f2df140798ea3a5dd84bc86"
 dependencies = [
  "chalk-derive",
  "chalk-ir",
@@ -197,9 +197,9 @@ dependencies = [
 
 [[package]]
 name = "chalk-solve"
-version = "0.23.0"
+version = "0.21.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "802de4eff72e5a5d2828e6c07224c74d66949dc6308aff025d0ae2871a11b4eb"
+checksum = "a886da37a0dc457057d86f78f026f7a09c6d8088aa13f4f4127fdb8dc80119a3"
 dependencies = [
  "chalk-derive",
  "chalk-ir",
@@ -214,9 +214,9 @@ dependencies = [
 
 [[package]]
 name = "chrono"
-version = "0.4.15"
+version = "0.4.13"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "942f72db697d8767c22d46a598e01f2d3b475501ea43d0db4f16d90259182d0b"
+checksum = "c74d84029116787153e02106bf53e66828452a4b325cc8652b788b5967c0a0b6"
 dependencies = [
  "num-integer",
  "num-traits",
@@ -1097,17 +1097,12 @@ dependencies = [
  "mbe",
  "memmap",
  "proc_macro_api",
- "proc_macro_test",
  "serde_derive",
  "test_utils",
  "toolchain",
  "tt",
 ]
 
-[[package]]
-name = "proc_macro_test"
-version = "0.0.0"
-
 [[package]]
 name = "profile"
 version = "0.0.0"
@@ -1264,9 +1259,9 @@ dependencies = [
 
 [[package]]
 name = "rustc-ap-rustc_lexer"
-version = "673.0.0"
+version = "671.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f6b71fa1285bdefe5fb61e59b63d6cc246abf337f4acafdd620d721bc488e671"
+checksum = "22e1221f3bfa2943c942cf8da319ab2346887f8757778c29c7f1822cd27b521f"
 dependencies = [
  "unicode-xid",
 ]
diff --git a/crates/assists/src/ast_transform.rs b/crates/assists/src/ast_transform.rs
index 5216862ba5..4c41c16d86 100644
--- a/crates/assists/src/ast_transform.rs
+++ b/crates/assists/src/ast_transform.rs
@@ -7,17 +7,6 @@ use syntax::{
     ast::{self, AstNode},
 };
 
-pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N {
-    SyntaxRewriter::from_fn(|element| match element {
-        syntax::SyntaxElement::Node(n) => {
-            let replacement = transformer.get_substitution(&n)?;
-            Some(replacement.into())
-        }
-        _ => None,
-    })
-    .rewrite_ast(&node)
-}
-
 pub trait AstTransform<'a> {
     fn get_substitution(&self, node: &syntax::SyntaxNode) -> Option<syntax::SyntaxNode>;
 
@@ -118,7 +107,10 @@ impl<'a> SubstituteTypeParams<'a> {
             ast::Type::PathType(path_type) => path_type.path()?,
             _ => return None,
         };
-        let resolution = self.source_scope.speculative_resolve(&path)?;
+        // FIXME: use `hir::Path::from_src` instead.
+        #[allow(deprecated)]
+        let path = hir::Path::from_ast(path)?;
+        let resolution = self.source_scope.resolve_hir_path(&path)?;
         match resolution {
             hir::PathResolution::TypeParam(tp) => Some(self.substs.get(&tp)?.syntax().clone()),
             _ => None,
@@ -154,7 +146,10 @@ impl<'a> QualifyPaths<'a> {
             // don't try to qualify `Fn(Foo) -> Bar` paths, they are in prelude anyway
             return None;
         }
-        let resolution = self.source_scope.speculative_resolve(&p)?;
+        // FIXME: use `hir::Path::from_src` instead.
+        #[allow(deprecated)]
+        let hir_path = hir::Path::from_ast(p.clone());
+        let resolution = self.source_scope.resolve_hir_path(&hir_path?)?;
         match resolution {
             PathResolution::Def(def) => {
                 let found_path = from.find_use_path(self.source_scope.db.upcast(), def)?;
@@ -180,6 +175,17 @@ impl<'a> QualifyPaths<'a> {
     }
 }
 
+pub fn apply<'a, N: AstNode>(transformer: &dyn AstTransform<'a>, node: N) -> N {
+    SyntaxRewriter::from_fn(|element| match element {
+        syntax::SyntaxElement::Node(n) => {
+            let replacement = transformer.get_substitution(&n)?;
+            Some(replacement.into())
+        }
+        _ => None,
+    })
+    .rewrite_ast(&node)
+}
+
 impl<'a> AstTransform<'a> for QualifyPaths<'a> {
     fn get_substitution(&self, node: &syntax::SyntaxNode) -> Option<syntax::SyntaxNode> {
         self.get_substitution_inner(node).or_else(|| self.previous.get_substitution(node))
diff --git a/crates/assists/src/handlers/add_missing_impl_members.rs b/crates/assists/src/handlers/add_missing_impl_members.rs
index 81b61ebf8e..83a2ada9a2 100644
--- a/crates/assists/src/handlers/add_missing_impl_members.rs
+++ b/crates/assists/src/handlers/add_missing_impl_members.rs
@@ -48,7 +48,6 @@ enum AddMissingImplMembersMode {
 //     fn foo(&self) -> u32 {
 //         ${0:todo!()}
 //     }
-//
 // }
 // ```
 pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
@@ -89,8 +88,8 @@ pub(crate) fn add_missing_impl_members(acc: &mut Assists, ctx: &AssistContext) -
 // impl Trait for () {
 //     Type X = ();
 //     fn foo(&self) {}
-//     $0fn bar(&self) {}
 //
+//     $0fn bar(&self) {}
 // }
 // ```
 pub(crate) fn add_missing_default_members(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
@@ -240,15 +239,18 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
+
     $0type Output;
+
     const CONST: usize = 42;
+
     fn foo(&self) {
         todo!()
     }
+
     fn baz(&self) {
         todo!()
     }
-
 }"#,
         );
     }
@@ -281,10 +283,10 @@ struct S;
 
 impl Foo for S {
     fn bar(&self) {}
+
     fn foo(&self) {
         ${0:todo!()}
     }
-
 }"#,
         );
     }
@@ -599,6 +601,7 @@ trait Foo {
 struct S;
 impl Foo for S {
     $0type Output;
+
     fn foo(&self) {
         todo!()
     }
@@ -705,6 +708,58 @@ trait Tr {
 
 impl Tr for () {
     $0type Ty;
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_whitespace_fixup_preserves_bad_tokens() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Tr {
+    fn foo();
+}
+
+impl Tr for ()<|> {
+    +++
+}"#,
+            r#"
+trait Tr {
+    fn foo();
+}
+
+impl Tr for () {
+    fn foo() {
+        ${0:todo!()}
+    }
+    +++
+}"#,
+        )
+    }
+
+    #[test]
+    fn test_whitespace_fixup_preserves_comments() {
+        check_assist(
+            add_missing_impl_members,
+            r#"
+trait Tr {
+    fn foo();
+}
+
+impl Tr for ()<|> {
+    // very important
+}"#,
+            r#"
+trait Tr {
+    fn foo();
+}
+
+impl Tr for () {
+    fn foo() {
+        ${0:todo!()}
+    }
+    // very important
 }"#,
         )
     }
diff --git a/crates/assists/src/handlers/auto_import.rs b/crates/assists/src/handlers/auto_import.rs
index b9ec3f10b6..cce789972e 100644
--- a/crates/assists/src/handlers/auto_import.rs
+++ b/crates/assists/src/handlers/auto_import.rs
@@ -53,7 +53,7 @@ pub(crate) fn auto_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()>
             |builder| {
                 insert_use_statement(
                     &auto_import_assets.syntax_under_caret,
-                    &import.to_string(),
+                    &import,
                     ctx,
                     builder.text_edit_builder(),
                 );
diff --git a/crates/assists/src/handlers/expand_glob_import.rs b/crates/assists/src/handlers/expand_glob_import.rs
index 81d0af2f35..f690ec343b 100644
--- a/crates/assists/src/handlers/expand_glob_import.rs
+++ b/crates/assists/src/handlers/expand_glob_import.rs
@@ -1,4 +1,3 @@
-use either::Either;
 use hir::{AssocItem, MacroDef, ModuleDef, Name, PathResolution, ScopeDef, SemanticsScope};
 use ide_db::{
     defs::{classify_name_ref, Definition, NameRefClass},
@@ -11,6 +10,8 @@ use crate::{
     AssistId, AssistKind,
 };
 
+use either::Either;
+
 // Assist: expand_glob_import
 //
 // Expands glob imports.
@@ -39,15 +40,11 @@ use crate::{
 pub(crate) fn expand_glob_import(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let star = ctx.find_token_at_offset(T![*])?;
     let mod_path = find_mod_path(&star)?;
-    let module = match ctx.sema.resolve_path(&mod_path)? {
-        PathResolution::Def(ModuleDef::Module(it)) => it,
-        _ => return None,
-    };
 
     let source_file = ctx.source_file();
     let scope = ctx.sema.scope_at_offset(source_file.syntax(), ctx.offset());
 
-    let defs_in_mod = find_defs_in_mod(ctx, scope, module)?;
+    let defs_in_mod = find_defs_in_mod(ctx, scope, &mod_path)?;
     let name_refs_in_source_file =
         source_file.syntax().descendants().filter_map(ast::NameRef::cast).collect();
     let used_names = find_used_names(ctx, defs_in_mod, name_refs_in_source_file);
@@ -85,8 +82,17 @@ impl Def {
 fn find_defs_in_mod(
     ctx: &AssistContext,
     from: SemanticsScope<'_>,
-    module: hir::Module,
+    path: &ast::Path,
 ) -> Option<Vec<Def>> {
+    let hir_path = ctx.sema.lower_path(&path)?;
+    let module = if let Some(PathResolution::Def(ModuleDef::Module(module))) =
+        from.resolve_hir_path_qualifier(&hir_path)
+    {
+        module
+    } else {
+        return None;
+    };
+
     let module_scope = module.scope(ctx.db(), from.module());
 
     let mut defs = vec![];
diff --git a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
index d62e06b4a8..4bcdae7ba0 100644
--- a/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
+++ b/crates/assists/src/handlers/extract_struct_from_enum_variant.rs
@@ -106,12 +106,7 @@ fn insert_import(
     if let Some(mut mod_path) = mod_path {
         mod_path.segments.pop();
         mod_path.segments.push(variant_hir_name.clone());
-        insert_use_statement(
-            path.syntax(),
-            &mod_path.to_string(),
-            ctx,
-            builder.text_edit_builder(),
-        );
+        insert_use_statement(path.syntax(), &mod_path, ctx, builder.text_edit_builder());
     }
     Some(())
 }
diff --git a/crates/assists/src/handlers/replace_qualified_name_with_use.rs b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
index 470e5f8ff7..011bf1106d 100644
--- a/crates/assists/src/handlers/replace_qualified_name_with_use.rs
+++ b/crates/assists/src/handlers/replace_qualified_name_with_use.rs
@@ -1,5 +1,5 @@
-use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SyntaxNode, TextRange};
-use test_utils::mark;
+use hir;
+use syntax::{algo::SyntaxRewriter, ast, match_ast, AstNode, SmolStr, SyntaxNode};
 
 use crate::{
     utils::{find_insert_use_container, insert_use_statement},
@@ -28,19 +28,12 @@ pub(crate) fn replace_qualified_name_with_use(
     if path.syntax().ancestors().find_map(ast::Use::cast).is_some() {
         return None;
     }
-    if path.qualifier().is_none() {
-        mark::hit!(dont_import_trivial_paths);
+
+    let hir_path = ctx.sema.lower_path(&path)?;
+    let segments = collect_hir_path_segments(&hir_path)?;
+    if segments.len() < 2 {
         return None;
     }
-    let path_to_import = path.to_string().clone();
-    let path_to_import = match path.segment()?.generic_arg_list() {
-        Some(generic_args) => {
-            let generic_args_start =
-                generic_args.syntax().text_range().start() - path.syntax().text_range().start();
-            &path_to_import[TextRange::up_to(generic_args_start)]
-        }
-        None => path_to_import.as_str(),
-    };
 
     let target = path.syntax().text_range();
     acc.add(
@@ -48,16 +41,12 @@ pub(crate) fn replace_qualified_name_with_use(
         "Replace qualified path with use",
         target,
         |builder| {
+            let path_to_import = hir_path.mod_path().clone();
             let container = match find_insert_use_container(path.syntax(), ctx) {
                 Some(c) => c,
                 None => return,
             };
-            insert_use_statement(
-                path.syntax(),
-                &path_to_import.to_string(),
-                ctx,
-                builder.text_edit_builder(),
-            );
+            insert_use_statement(path.syntax(), &path_to_import, ctx, builder.text_edit_builder());
 
             // Now that we've brought the name into scope, re-qualify all paths that could be
             // affected (that is, all paths inside the node we added the `use` to).
@@ -69,6 +58,26 @@ pub(crate) fn replace_qualified_name_with_use(
     )
 }
 
+fn collect_hir_path_segments(path: &hir::Path) -> Option<Vec<SmolStr>> {
+    let mut ps = Vec::<SmolStr>::with_capacity(10);
+    match path.kind() {
+        hir::PathKind::Abs => ps.push("".into()),
+        hir::PathKind::Crate => ps.push("crate".into()),
+        hir::PathKind::Plain => {}
+        hir::PathKind::Super(0) => ps.push("self".into()),
+        hir::PathKind::Super(lvl) => {
+            let mut chain = "super".to_string();
+            for _ in 0..*lvl {
+                chain += "::super";
+            }
+            ps.push(chain.into());
+        }
+        hir::PathKind::DollarCrate(_) => return None,
+    }
+    ps.extend(path.segments().iter().map(|it| it.name.to_string().into()));
+    Some(ps)
+}
+
 /// Adds replacements to `re` that shorten `path` in all descendants of `node`.
 fn shorten_paths(rewriter: &mut SyntaxRewriter<'static>, node: SyntaxNode, path: ast::Path) {
     for child in node.children() {
@@ -458,8 +467,7 @@ impl Debug for Foo {
     }
 
     #[test]
-    fn dont_import_trivial_paths() {
-        mark::check!(dont_import_trivial_paths);
+    fn test_replace_not_applicable_one_segment() {
         check_assist_not_applicable(
             replace_qualified_name_with_use,
             r"
diff --git a/crates/assists/src/utils/insert_use.rs b/crates/assists/src/utils/insert_use.rs
index 49096a67c7..50a62ee829 100644
--- a/crates/assists/src/utils/insert_use.rs
+++ b/crates/assists/src/utils/insert_use.rs
@@ -5,6 +5,7 @@
 use std::iter::successors;
 
 use either::Either;
+use hir::{self, ModPath};
 use syntax::{
     ast::{self, NameOwner, VisibilityOwner},
     AstNode, AstToken, Direction, SmolStr,
@@ -34,11 +35,11 @@ pub(crate) fn find_insert_use_container(
 pub(crate) fn insert_use_statement(
     // Ideally the position of the cursor, used to
     position: &SyntaxNode,
-    path_to_import: &str,
+    path_to_import: &ModPath,
     ctx: &AssistContext,
     builder: &mut TextEditBuilder,
 ) {
-    let target = path_to_import.split("::").map(SmolStr::new).collect::<Vec<_>>();
+    let target = path_to_import.to_string().split("::").map(SmolStr::new).collect::<Vec<_>>();
     let container = find_insert_use_container(position, ctx);
 
     if let Some(container) = container {
diff --git a/crates/hir/src/code_model.rs b/crates/hir/src/code_model.rs
index c442654dd0..5dc3ae3b19 100644
--- a/crates/hir/src/code_model.rs
+++ b/crates/hir/src/code_model.rs
@@ -12,7 +12,6 @@ use hir_def::{
     docs::Documentation,
     expr::{BindingAnnotation, Pat, PatId},
     import_map,
-    path::ModPath,
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
     src::HasSource as _,
@@ -345,7 +344,11 @@ impl Module {
 
     /// Finds a path that can be used to refer to the given item from within
     /// this module, if possible.
-    pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
+    pub fn find_use_path(
+        self,
+        db: &dyn DefDatabase,
+        item: impl Into<ItemInNs>,
+    ) -> Option<hir_def::path::ModPath> {
         hir_def::find_path::find_path(db, item.into(), self.into())
     }
 }
@@ -1123,7 +1126,7 @@ impl ImplDef {
             .value
             .attrs()
             .filter_map(|it| {
-                let path = ModPath::from_src(it.path()?, &hygenic)?;
+                let path = hir_def::path::ModPath::from_src(it.path()?, &hygenic)?;
                 if path.as_ident()?.to_string() == "derive" {
                     Some(it)
                 } else {
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index 8961ba8fd6..4ae2bd0855 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -48,7 +48,7 @@ pub use hir_def::{
     builtin_type::BuiltinType,
     docs::Documentation,
     nameres::ModuleSource,
-    path::ModPath,
+    path::{ModPath, Path, PathKind},
     type_ref::{Mutability, TypeRef},
 };
 pub use hir_expand::{
@@ -60,7 +60,4 @@ pub use hir_ty::display::HirDisplay;
 // These are negative re-exports: pub using these names is forbidden, they
 // should remain private to hir internals.
 #[allow(unused)]
-use {
-    hir_def::path::{Path, PathKind},
-    hir_expand::hygiene::Hygiene,
-};
+use hir_expand::hygiene::Hygiene;
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index c693176fa4..d8beac98a6 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -6,7 +6,7 @@ use std::{cell::RefCell, fmt, iter::successors};
 
 use base_db::{FileId, FileRange};
 use hir_def::{
-    resolver::{self, HasResolver, Resolver, TypeNs},
+    resolver::{self, HasResolver, Resolver},
     AsMacroCall, FunctionId, TraitId, VariantId,
 };
 use hir_expand::{hygiene::Hygiene, name::AsName, ExpansionInfo};
@@ -22,11 +22,12 @@ use crate::{
     db::HirDatabase,
     diagnostics::Diagnostic,
     semantics::source_to_def::{ChildContainer, SourceToDefCache, SourceToDefCtx},
-    source_analyzer::{resolve_hir_path, SourceAnalyzer},
+    source_analyzer::{resolve_hir_path, resolve_hir_path_qualifier, SourceAnalyzer},
     AssocItem, Callable, Crate, Field, Function, HirFileId, ImplDef, InFile, Local, MacroDef,
     Module, ModuleDef, Name, Origin, Path, ScopeDef, Trait, Type, TypeAlias, TypeParam, TypeRef,
     VariantDef,
 };
+use resolver::TypeNs;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum PathResolution {
@@ -111,13 +112,14 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
     pub fn expand(&self, macro_call: &ast::MacroCall) -> Option<SyntaxNode> {
         self.imp.expand(macro_call)
     }
-    pub fn speculative_expand(
+
+    pub fn expand_hypothetical(
         &self,
         actual_macro_call: &ast::MacroCall,
         hypothetical_args: &ast::TokenTree,
         token_to_map: SyntaxToken,
     ) -> Option<(SyntaxNode, SyntaxToken)> {
-        self.imp.speculative_expand(actual_macro_call, hypothetical_args, token_to_map)
+        self.imp.expand_hypothetical(actual_macro_call, hypothetical_args, token_to_map)
     }
 
     pub fn descend_into_macros(&self, token: SyntaxToken) -> SyntaxToken {
@@ -227,6 +229,10 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
         self.imp.resolve_variant(record_lit).map(VariantDef::from)
     }
 
+    pub fn lower_path(&self, path: &ast::Path) -> Option<Path> {
+        self.imp.lower_path(path)
+    }
+
     pub fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
         self.imp.resolve_bind_pat_to_const(pat)
     }
@@ -305,7 +311,7 @@ impl<'db> SemanticsImpl<'db> {
         Some(node)
     }
 
-    fn speculative_expand(
+    fn expand_hypothetical(
         &self,
         actual_macro_call: &ast::MacroCall,
         hypothetical_args: &ast::TokenTree,
@@ -462,6 +468,11 @@ impl<'db> SemanticsImpl<'db> {
         self.analyze(record_lit.syntax()).resolve_variant(self.db, record_lit)
     }
 
+    fn lower_path(&self, path: &ast::Path) -> Option<Path> {
+        let src = self.find_file(path.syntax().clone());
+        Path::from_src(path.clone(), &Hygiene::new(self.db.upcast(), src.file_id.into()))
+    }
+
     fn resolve_bind_pat_to_const(&self, pat: &ast::IdentPat) -> Option<ModuleDef> {
         self.analyze(pat.syntax()).resolve_bind_pat_to_const(self.db, pat)
     }
@@ -745,10 +756,31 @@ impl<'a> SemanticsScope<'a> {
 
     /// Resolve a path as-if it was written at the given scope. This is
     /// necessary a heuristic, as it doesn't take hygiene into account.
-    pub fn speculative_resolve(&self, path: &ast::Path) -> Option<PathResolution> {
+    pub fn resolve_hypothetical(&self, path: &ast::Path) -> Option<PathResolution> {
         let hygiene = Hygiene::new(self.db.upcast(), self.file_id);
         let path = Path::from_src(path.clone(), &hygiene)?;
-        resolve_hir_path(self.db, &self.resolver, &path)
+        self.resolve_hir_path(&path)
+    }
+
+    pub fn resolve_hir_path(&self, path: &Path) -> Option<PathResolution> {
+        resolve_hir_path(self.db, &self.resolver, path)
+    }
+
+    /// Resolves a path where we know it is a qualifier of another path.
+    ///
+    /// For example, if we have:
+    /// ```
+    /// mod my {
+    ///     pub mod foo {
+    ///         struct Bar;
+    ///     }
+    ///
+    ///     pub fn foo() {}
+    /// }
+    /// ```
+    /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
+    pub fn resolve_hir_path_qualifier(&self, path: &Path) -> Option<PathResolution> {
+        resolve_hir_path_qualifier(self.db, &self.resolver, path)
     }
 }
 
diff --git a/crates/hir/src/source_analyzer.rs b/crates/hir/src/source_analyzer.rs
index 1d13c4f1d3..8750584f94 100644
--- a/crates/hir/src/source_analyzer.rs
+++ b/crates/hir/src/source_analyzer.rs
@@ -13,7 +13,6 @@ use hir_def::{
         Body, BodySourceMap,
     },
     expr::{ExprId, Pat, PatId},
-    path::{ModPath, Path, PathKind},
     resolver::{resolver_for_scope, Resolver, TypeNs, ValueNs},
     AsMacroCall, DefWithBodyId, FieldId, FunctionId, LocalFieldId, VariantId,
 };
@@ -29,7 +28,8 @@ use syntax::{
 
 use crate::{
     db::HirDatabase, semantics::PathResolution, Adt, Const, EnumVariant, Field, Function, Local,
-    MacroDef, ModuleDef, Static, Struct, Trait, Type, TypeAlias, TypeParam,
+    MacroDef, ModPath, ModuleDef, Path, PathKind, Static, Struct, Trait, Type, TypeAlias,
+    TypeParam,
 };
 use base_db::CrateId;
 
@@ -508,7 +508,7 @@ pub(crate) fn resolve_hir_path(
 /// }
 /// ```
 /// then we know that `foo` in `my::foo::Bar` refers to the module, not the function.
-fn resolve_hir_path_qualifier(
+pub(crate) fn resolve_hir_path_qualifier(
     db: &dyn HirDatabase,
     resolver: &Resolver,
     path: &Path,
diff --git a/crates/hir_def/src/path.rs b/crates/hir_def/src/path.rs
index 99395667de..74d26f08b3 100644
--- a/crates/hir_def/src/path.rs
+++ b/crates/hir_def/src/path.rs
@@ -153,6 +153,12 @@ pub enum GenericArg {
 }
 
 impl Path {
+    /// Converts an `ast::Path` to `Path`. Works with use trees.
+    #[deprecated = "Doesn't handle hygiene, don't add new calls, remove old ones"]
+    pub fn from_ast(path: ast::Path) -> Option<Path> {
+        lower::lower_path(path, &Hygiene::new_unhygienic())
+    }
+
     /// Converts an `ast::Path` to `Path`. Works with use trees.
     /// It correctly handles `$crate` based path from macro call.
     pub fn from_src(path: ast::Path, hygiene: &Hygiene) -> Option<Path> {
diff --git a/crates/hir_ty/Cargo.toml b/crates/hir_ty/Cargo.toml
index a319b0ce8e..83b5013a90 100644
--- a/crates/hir_ty/Cargo.toml
+++ b/crates/hir_ty/Cargo.toml
@@ -16,9 +16,9 @@ ena = "0.14.0"
 log = "0.4.8"
 rustc-hash = "1.1.0"
 scoped-tls = "1"
-chalk-solve = { version = "0.23.0" }
-chalk-ir = { version = "0.23.0" }
-chalk-recursive = { version = "0.23.0" }
+chalk-solve = { version = "0.21.0" }
+chalk-ir = { version = "0.21.0" }
+chalk-recursive = { version = "0.21.0" }
 
 stdx = { path = "../stdx" }
 hir_def = { path = "../hir_def" }
diff --git a/crates/hir_ty/src/traits.rs b/crates/hir_ty/src/traits.rs
index 14cd3a2b44..2553237170 100644
--- a/crates/hir_ty/src/traits.rs
+++ b/crates/hir_ty/src/traits.rs
@@ -3,7 +3,7 @@ use std::sync::Arc;
 
 use base_db::CrateId;
 use chalk_ir::cast::Cast;
-use chalk_solve::{logging_db::LoggingRustIrDatabase, Solver};
+use chalk_solve::Solver;
 use hir_def::{lang_item::LangItemTarget, TraitId};
 
 use crate::{db::HirDatabase, DebruijnIndex, Substs};
@@ -166,25 +166,16 @@ fn solve(
         }
         remaining > 0
     };
-
     let mut solve = || {
-        if is_chalk_print() {
-            let logging_db = LoggingRustIrDatabase::new(context);
-            let solution = solver.solve_limited(&logging_db, goal, &should_continue);
-            log::debug!("chalk program:\n{}", logging_db);
-            solution
-        } else {
-            solver.solve_limited(&context, goal, &should_continue)
-        }
+        let solution = solver.solve_limited(&context, goal, should_continue);
+        log::debug!("solve({:?}) => {:?}", goal, solution);
+        solution
     };
-
     // don't set the TLS for Chalk unless Chalk debugging is active, to make
     // extra sure we only use it for debugging
     let solution =
         if is_chalk_debug() { chalk::tls::set_current_program(db, solve) } else { solve() };
 
-    log::debug!("solve({:?}) => {:?}", goal, solution);
-
     solution
 }
 
@@ -192,10 +183,6 @@ fn is_chalk_debug() -> bool {
     std::env::var("CHALK_DEBUG").is_ok()
 }
 
-fn is_chalk_print() -> bool {
-    std::env::var("CHALK_PRINT").is_ok()
-}
-
 fn solution_from_chalk(
     db: &dyn HirDatabase,
     solution: chalk_solve::Solution<Interner>,
diff --git a/crates/hir_ty/src/traits/chalk.rs b/crates/hir_ty/src/traits/chalk.rs
index 17c83b6a46..b336534179 100644
--- a/crates/hir_ty/src/traits/chalk.rs
+++ b/crates/hir_ty/src/traits/chalk.rs
@@ -240,23 +240,20 @@ impl<'a> chalk_solve::RustIrDatabase<Interner> for ChalkContext<'a> {
         Substs::empty().to_chalk(self.db)
     }
 
-    fn trait_name(&self, trait_id: chalk_ir::TraitId<Interner>) -> String {
-        let id = from_chalk(self.db, trait_id);
-        self.db.trait_data(id).name.to_string()
+    fn trait_name(&self, _trait_id: chalk_ir::TraitId<Interner>) -> String {
+        unimplemented!()
     }
-    // FIXME: lookup names
-    fn adt_name(&self, struct_id: chalk_ir::AdtId<Interner>) -> String {
-        let datum = self.db.struct_datum(self.krate, struct_id);
-        format!("{:?}", datum.name(&Interner))
+    fn adt_name(&self, _struct_id: chalk_ir::AdtId<Interner>) -> String {
+        unimplemented!()
     }
-    fn assoc_type_name(&self, assoc_ty_id: chalk_ir::AssocTypeId<Interner>) -> String {
-        format!("Assoc_{}", assoc_ty_id.0)
+    fn assoc_type_name(&self, _assoc_ty_id: chalk_ir::AssocTypeId<Interner>) -> String {
+        unimplemented!()
     }
-    fn opaque_type_name(&self, opaque_ty_id: chalk_ir::OpaqueTyId<Interner>) -> String {
-        format!("Opaque_{}", opaque_ty_id.0)
+    fn opaque_type_name(&self, _opaque_ty_id: chalk_ir::OpaqueTyId<Interner>) -> String {
+        unimplemented!()
     }
-    fn fn_def_name(&self, fn_def_id: chalk_ir::FnDefId<Interner>) -> String {
-        format!("fn_{}", fn_def_id.0)
+    fn fn_def_name(&self, _fn_def_id: chalk_ir::FnDefId<Interner>) -> String {
+        unimplemented!()
     }
 }
 
diff --git a/crates/ide/src/completion/completion_context.rs b/crates/ide/src/completion/completion_context.rs
index 85456a66f5..3857dce672 100644
--- a/crates/ide/src/completion/completion_context.rs
+++ b/crates/ide/src/completion/completion_context.rs
@@ -185,7 +185,7 @@ impl<'a> CompletionContext<'a> {
             };
             if let (Some(actual_expansion), Some(hypothetical_expansion)) = (
                 ctx.sema.expand(&actual_macro_call),
-                ctx.sema.speculative_expand(
+                ctx.sema.expand_hypothetical(
                     &actual_macro_call,
                     &hypothetical_args,
                     fake_ident_token,
diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs
index 596bc872db..002adf9159 100644
--- a/crates/ide/src/inlay_hints.rs
+++ b/crates/ide/src/inlay_hints.rs
@@ -43,7 +43,7 @@ pub struct InlayHint {
 // rust-analyzer shows additional information inline with the source code.
 // Editors usually render this using read-only virtual text snippets interspersed with code.
 //
-// rust-analyzer shows hints for
+// rust-analyzer shows hits for
 //
 // * types of local variables
 // * names of function arguments
diff --git a/crates/proc_macro_api/src/lib.rs b/crates/proc_macro_api/src/lib.rs
index d5e87cf7d5..15db57eb28 100644
--- a/crates/proc_macro_api/src/lib.rs
+++ b/crates/proc_macro_api/src/lib.rs
@@ -89,8 +89,9 @@ impl ProcMacroClient {
                 macros
                     .into_iter()
                     .filter_map(|(name, kind)| {
+                        // FIXME: Support custom derive only for now.
                         match kind {
-                            ProcMacroKind::CustomDerive | ProcMacroKind::FuncLike => {
+                            ProcMacroKind::CustomDerive => {
                                 let name = SmolStr::new(&name);
                                 let expander: Arc<dyn tt::TokenExpander> =
                                     Arc::new(ProcMacroProcessExpander {
@@ -100,8 +101,7 @@ impl ProcMacroClient {
                                     });
                                 Some((name, expander))
                             }
-                            // FIXME: Attribute macro are currently unsupported.
-                            ProcMacroKind::Attr => None,
+                            _ => None,
                         }
                     })
                     .collect()
diff --git a/crates/proc_macro_srv/Cargo.toml b/crates/proc_macro_srv/Cargo.toml
index a468b55609..7171f08084 100644
--- a/crates/proc_macro_srv/Cargo.toml
+++ b/crates/proc_macro_srv/Cargo.toml
@@ -21,9 +21,7 @@ test_utils = { path = "../test_utils" }
 [dev-dependencies]
 cargo_metadata = "0.11.1"
 difference = "2.0.0"
-
-# used as proc macro test targets
+# used as proc macro test target
 serde_derive = "1.0.106"
-proc_macro_test = { path = "../proc_macro_test" }
 
 toolchain = { path = "../toolchain" }
diff --git a/crates/ssr/src/resolving.rs b/crates/ssr/src/resolving.rs
index b932132d5b..4441fb426a 100644
--- a/crates/ssr/src/resolving.rs
+++ b/crates/ssr/src/resolving.rs
@@ -212,13 +212,13 @@ impl<'db> ResolutionScope<'db> {
         // First try resolving the whole path. This will work for things like
         // `std::collections::HashMap`, but will fail for things like
         // `std::collections::HashMap::new`.
-        if let Some(resolution) = self.scope.speculative_resolve(&path) {
+        if let Some(resolution) = self.scope.resolve_hypothetical(&path) {
             return Some(resolution);
         }
         // Resolution failed, try resolving the qualifier (e.g. `std::collections::HashMap` and if
         // that succeeds, then iterate through the candidates on the resolved type with the provided
         // name.
-        let resolved_qualifier = self.scope.speculative_resolve(&path.qualifier()?)?;
+        let resolved_qualifier = self.scope.resolve_hypothetical(&path.qualifier()?)?;
         if let hir::PathResolution::Def(hir::ModuleDef::Adt(adt)) = resolved_qualifier {
             let name = path.segment()?.name_ref()?;
             adt.ty(self.scope.db).iterate_path_candidates(
diff --git a/crates/syntax/Cargo.toml b/crates/syntax/Cargo.toml
index ec3132da8d..47e351f9d1 100644
--- a/crates/syntax/Cargo.toml
+++ b/crates/syntax/Cargo.toml
@@ -13,7 +13,7 @@ doctest = false
 [dependencies]
 itertools = "0.9.0"
 rowan = "0.10.0"
-rustc_lexer = { version = "673.0.0", package = "rustc-ap-rustc_lexer" }
+rustc_lexer = { version = "671.0.0", package = "rustc-ap-rustc_lexer" }
 rustc-hash = "1.1.0"
 arrayvec = "0.5.1"
 once_cell = "1.3.1"
diff --git a/crates/syntax/src/ast/edit.rs b/crates/syntax/src/ast/edit.rs
index 190746e09e..060b209663 100644
--- a/crates/syntax/src/ast/edit.rs
+++ b/crates/syntax/src/ast/edit.rs
@@ -91,29 +91,52 @@ impl ast::AssocItemList {
             res = make_multiline(res);
         }
         items.into_iter().for_each(|it| res = res.append_item(it));
-        res
+        res.fixup_trailing_whitespace().unwrap_or(res)
     }
 
     #[must_use]
     pub fn append_item(&self, item: ast::AssocItem) -> ast::AssocItemList {
-        let (indent, position) = match self.assoc_items().last() {
+        let (indent, position, whitespace) = match self.assoc_items().last() {
             Some(it) => (
                 leading_indent(it.syntax()).unwrap_or_default().to_string(),
                 InsertPosition::After(it.syntax().clone().into()),
+                "\n\n",
             ),
             None => match self.l_curly_token() {
                 Some(it) => (
                     "    ".to_string() + &leading_indent(self.syntax()).unwrap_or_default(),
                     InsertPosition::After(it.into()),
+                    "\n",
                 ),
                 None => return self.clone(),
             },
         };
-        let ws = tokens::WsBuilder::new(&format!("\n{}", indent));
+        let ws = tokens::WsBuilder::new(&format!("{}{}", whitespace, indent));
         let to_insert: ArrayVec<[SyntaxElement; 2]> =
             [ws.ws().into(), item.syntax().clone().into()].into();
         self.insert_children(position, to_insert)
     }
+
+    /// Remove extra whitespace between last item and closing curly brace.
+    fn fixup_trailing_whitespace(&self) -> Option<ast::AssocItemList> {
+        let first_token_after_items =
+            self.assoc_items().last()?.syntax().next_sibling_or_token()?;
+        let last_token_before_curly = self.r_curly_token()?.prev_sibling_or_token()?;
+        if last_token_before_curly != first_token_after_items {
+            // there is something more between last item and
+            // right curly than just whitespace - bail out
+            return None;
+        }
+        let whitespace =
+            last_token_before_curly.clone().into_token().and_then(ast::Whitespace::cast)?;
+        let text = whitespace.syntax().text();
+        let newline = text.rfind("\n")?;
+        let keep = tokens::WsBuilder::new(&text[newline..]);
+        Some(self.replace_children(
+            first_token_after_items..=last_token_before_curly,
+            std::iter::once(keep.ws().into()),
+        ))
+    }
 }
 
 impl ast::RecordExprFieldList {
diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml
index e9edbdd10b..1a1140b04e 100644
--- a/xtask/Cargo.toml
+++ b/xtask/Cargo.toml
@@ -18,4 +18,3 @@ quote = "1.0.2"
 ungrammar = "1.1.1"
 walkdir = "2.3.1"
 write-json =  "0.1.0"
-# Avoid adding more dependencies to this crate


Test patch:
diff --git a/crates/assists/src/tests/generated.rs b/crates/assists/src/tests/generated.rs
index d16e6fb0a6..1735670037 100644
--- a/crates/assists/src/tests/generated.rs
+++ b/crates/assists/src/tests/generated.rs
@@ -82,8 +82,8 @@ trait Trait {
 impl Trait for () {
     Type X = ();
     fn foo(&self) {}
-    $0fn bar(&self) {}
 
+    $0fn bar(&self) {}
 }
 "#####,
     )
@@ -115,7 +115,6 @@ impl Trait<u32> for () {
     fn foo(&self) -> u32 {
         ${0:todo!()}
     }
-
 }
 "#####,
     )
diff --git a/crates/proc_macro_srv/src/tests/mod.rs b/crates/proc_macro_srv/src/tests/mod.rs
index 1a827cbd76..8e6f28abdf 100644
--- a/crates/proc_macro_srv/src/tests/mod.rs
+++ b/crates/proc_macro_srv/src/tests/mod.rs
@@ -35,7 +35,7 @@ SUBTREE $
 
 #[test]
 fn test_derive_proc_macro_list() {
-    let res = list("serde_derive", "1").join("\n");
+    let res = list("serde_derive", "1.0").join("\n");
 
     assert_eq_text!(
         &res,
@@ -43,16 +43,3 @@ fn test_derive_proc_macro_list() {
 Deserialize [CustomDerive]"#
     );
 }
-
-/// Tests that we find and classify non-derive macros correctly.
-#[test]
-fn list_test_macros() {
-    let res = list("proc_macro_test", "0.0.0").join("\n");
-
-    assert_eq_text!(
-        &res,
-        r#"function_like_macro [FuncLike]
-attribute_macro [Attr]
-DummyTrait [CustomDerive]"#
-    );
-}
diff --git a/crates/proc_macro_srv/src/tests/utils.rs b/crates/proc_macro_srv/src/tests/utils.rs
index 36942147d9..5828512d6e 100644
--- a/crates/proc_macro_srv/src/tests/utils.rs
+++ b/crates/proc_macro_srv/src/tests/utils.rs
@@ -13,7 +13,7 @@ mod fixtures {
     // Use current project metadata to get the proc-macro dylib path
     pub fn dylib_path(crate_name: &str, version: &str) -> std::path::PathBuf {
         let command = Command::new(toolchain::cargo())
-            .args(&["check", "--tests", "--message-format", "json"])
+            .args(&["check", "--message-format", "json"])
             .output()
             .unwrap()
             .stdout;
diff --git a/crates/proc_macro_test/Cargo.toml b/crates/proc_macro_test/Cargo.toml
deleted file mode 100644
index 7b0f64f318..0000000000
--- a/crates/proc_macro_test/Cargo.toml
+++ /dev/null
@@ -1,10 +0,0 @@
-[package]
-name = "proc_macro_test"
-version = "0.0.0"
-license = "MIT OR Apache-2.0"
-authors = ["rust-analyzer developers"]
-edition = "2018"
-
-[lib]
-doctest = false
-proc-macro = true
diff --git a/crates/proc_macro_test/src/lib.rs b/crates/proc_macro_test/src/lib.rs
deleted file mode 100644
index ec2a114a35..0000000000
--- a/crates/proc_macro_test/src/lib.rs
+++ /dev/null
@@ -1,18 +0,0 @@
-//! Exports a few trivial procedural macros for testing.
-
-use proc_macro::TokenStream;
-
-#[proc_macro]
-pub fn function_like_macro(args: TokenStream) -> TokenStream {
-    args
-}
-
-#[proc_macro_attribute]
-pub fn attribute_macro(_args: TokenStream, item: TokenStream) -> TokenStream {
-    item
-}
-
-#[proc_macro_derive(DummyTrait)]
-pub fn derive_macro(_item: TokenStream) -> TokenStream {
-    TokenStream::new()
-}