7 files changed, 241 insertions, 244 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 451ba1f42..d341d1054 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1111,7 +1111,6 @@ dependencies = [
  "expect",
  "fst",
  "indexmap",
- "insta",
  "itertools",
  "log",
  "once_cell",
diff --git a/crates/ra_assists/src/handlers/raw_string.rs b/crates/ra_assists/src/handlers/raw_string.rs
index 6d77dff13..ba1dcb610 100644
--- a/crates/ra_assists/src/handlers/raw_string.rs
+++ b/crates/ra_assists/src/handlers/raw_string.rs
@@ -4,8 +4,9 @@ use ra_syntax::{
     ast::{self, HasQuotes, HasStringValue},
     AstToken,
     SyntaxKind::{RAW_STRING, STRING},
-    TextSize,
+    TextRange, TextSize,
 };
+use test_utils::mark;
 
 use crate::{AssistContext, AssistId, AssistKind, Assists};
 
@@ -33,8 +34,7 @@ pub(crate) fn make_raw_string(acc: &mut Assists, ctx: &AssistContext) -> Option<
         "Rewrite as raw string",
         target,
         |edit| {
-            let max_hash_streak = count_hashes(&value);
-            let hashes = "#".repeat(max_hash_streak + 1);
+            let hashes = "#".repeat(required_hashes(&value).max(1));
             if matches!(value, Cow::Borrowed(_)) {
                 // Avoid replacing the whole string to better position the cursor.
                 edit.insert(token.syntax().text_range().start(), format!("r{}", hashes));
@@ -106,7 +106,7 @@ pub(crate) fn make_usual_string(acc: &mut Assists, ctx: &AssistContext) -> Optio
 pub(crate) fn add_hash(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
     let token = ctx.find_token_at_offset(RAW_STRING)?;
     let target = token.text_range();
-    acc.add(AssistId("add_hash", AssistKind::Refactor), "Add # to raw string", target, |edit| {
+    acc.add(AssistId("add_hash", AssistKind::Refactor), "Add #", target, |edit| {
         edit.insert(token.text_range().start() + TextSize::of('r'), "#");
         edit.insert(token.text_range().end(), "#");
     })
@@ -128,49 +128,58 @@ pub(crate) fn add_hash(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
 // }
 // ```
 pub(crate) fn remove_hash(acc: &mut Assists, ctx: &AssistContext) -> Option<()> {
-    let token = ctx.find_token_at_offset(RAW_STRING)?;
+    let token = ctx.find_token_at_offset(RAW_STRING).and_then(ast::RawString::cast)?;
+
     let text = token.text().as_str();
-    if text.starts_with("r\"") {
-        // no hash to remove
+    if !text.starts_with("r#") && text.ends_with("#") {
         return None;
     }
-    let target = token.text_range();
-    acc.add(
-        AssistId("remove_hash", AssistKind::RefactorRewrite),
-        "Remove hash from raw string",
-        target,
-        |edit| {
-            let result = &text[2..text.len() - 1];
-            let result = if result.starts_with('\"') {
-                // FIXME: this logic is wrong, not only the last has has to handled specially
-                // no more hash, escape
-                let internal_str = &result[1..result.len() - 1];
-                format!("\"{}\"", internal_str.escape_default().to_string())
-            } else {
-                result.to_owned()
-            };
-            edit.replace(token.text_range(), format!("r{}", result));
-        },
-    )
+
+    let existing_hashes = text.chars().skip(1).take_while(|&it| it == '#').count();
+
+    let text_range = token.syntax().text_range();
+    let internal_text = &text[token.text_range_between_quotes()? - text_range.start()];
+
+    if existing_hashes == required_hashes(internal_text) {
+        mark::hit!(cant_remove_required_hash);
+        return None;
+    }
+
+    acc.add(AssistId("remove_hash", AssistKind::RefactorRewrite), "Remove #", text_range, |edit| {
+        edit.delete(TextRange::at(text_range.start() + TextSize::of('r'), TextSize::of('#')));
+        edit.delete(TextRange::new(text_range.end() - TextSize::of('#'), text_range.end()));
+    })
 }
 
-fn count_hashes(s: &str) -> usize {
-    let mut max_hash_streak = 0usize;
-    for idx in s.match_indices("\"#").map(|(i, _)| i) {
+fn required_hashes(s: &str) -> usize {
+    let mut res = 0usize;
+    for idx in s.match_indices('"').map(|(i, _)| i) {
         let (_, sub) = s.split_at(idx + 1);
-        let nb_hash = sub.chars().take_while(|c| *c == '#').count();
-        if nb_hash > max_hash_streak {
-            max_hash_streak = nb_hash;
-        }
+        let n_hashes = sub.chars().take_while(|c| *c == '#').count();
+        res = res.max(n_hashes + 1)
     }
-    max_hash_streak
+    res
+}
+
+#[test]
+fn test_required_hashes() {
+    assert_eq!(0, required_hashes("abc"));
+    assert_eq!(0, required_hashes("###"));
+    assert_eq!(1, required_hashes("\""));
+    assert_eq!(2, required_hashes("\"#abc"));
+    assert_eq!(0, required_hashes("#abc"));
+    assert_eq!(3, required_hashes("#ab\"##c"));
+    assert_eq!(5, required_hashes("#ab\"##\"####c"));
 }
 
 #[cfg(test)]
 mod test {
-    use super::*;
+    use test_utils::mark;
+
     use crate::tests::{check_assist, check_assist_not_applicable, check_assist_target};
 
+    use super::*;
+
     #[test]
     fn make_raw_string_target() {
         check_assist_target(
@@ -372,33 +381,21 @@ string"###;
     fn remove_hash_works() {
         check_assist(
             remove_hash,
-            r##"
-            fn f() {
-                let s = <|>r#"random string"#;
-            }
-            "##,
-            r#"
-            fn f() {
-                let s = r"random string";
-            }
-            "#,
+            r##"fn f() { let s = <|>r#"random string"#; }"##,
+            r#"fn f() { let s = r"random string"; }"#,
         )
     }
 
     #[test]
-    fn remove_hash_with_quote_works() {
-        check_assist(
+    fn cant_remove_required_hash() {
+        mark::check!(cant_remove_required_hash);
+        check_assist_not_applicable(
             remove_hash,
             r##"
             fn f() {
                 let s = <|>r#"random"str"ing"#;
             }
             "##,
-            r#"
-            fn f() {
-                let s = r"random\"str\"ing";
-            }
-            "#,
         )
     }
 
@@ -420,27 +417,13 @@ string"###;
     }
 
     #[test]
-    fn remove_hash_not_works() {
-        check_assist_not_applicable(
-            remove_hash,
-            r#"
-            fn f() {
-                let s = <|>"random string";
-            }
-            "#,
-        );
+    fn remove_hash_doesnt_work() {
+        check_assist_not_applicable(remove_hash, r#"fn f() { let s = <|>"random string"; }"#);
     }
 
     #[test]
-    fn remove_hash_no_hash_not_works() {
-        check_assist_not_applicable(
-            remove_hash,
-            r#"
-            fn f() {
-                let s = <|>r"random string";
-            }
-            "#,
-        );
+    fn remove_hash_no_hash_doesnt_work() {
+        check_assist_not_applicable(remove_hash, r#"fn f() { let s = <|>r"random string"; }"#);
     }
 
     #[test]
@@ -518,14 +501,4 @@ string"###;
             "#,
         );
     }
-
-    #[test]
-    fn count_hashes_test() {
-        assert_eq!(0, count_hashes("abc"));
-        assert_eq!(0, count_hashes("###"));
-        assert_eq!(1, count_hashes("\"#abc"));
-        assert_eq!(0, count_hashes("#abc"));
-        assert_eq!(2, count_hashes("#ab\"##c"));
-        assert_eq!(4, count_hashes("#ab\"##\"####c"));
-    }
 }
diff --git a/crates/ra_hir_def/Cargo.toml b/crates/ra_hir_def/Cargo.toml
index 1ec619b42..d96a86b80 100644
--- a/crates/ra_hir_def/Cargo.toml
+++ b/crates/ra_hir_def/Cargo.toml
@@ -33,5 +33,4 @@ ra_cfg = { path = "../ra_cfg" }
 tt = { path = "../ra_tt", package = "ra_tt" }
 
 [dev-dependencies]
-insta = "0.16.0"
 expect = { path = "../expect" }
diff --git a/crates/ra_hir_def/src/import_map.rs b/crates/ra_hir_def/src/import_map.rs
index 869f3ca5a..9e4c30b1a 100644
--- a/crates/ra_hir_def/src/import_map.rs
+++ b/crates/ra_hir_def/src/import_map.rs
@@ -327,32 +327,14 @@ pub fn search_dependencies<'a>(
 
 #[cfg(test)]
 mod tests {
-    use super::*;
-    use crate::{test_db::TestDB, AssocContainerId, Lookup};
-    use insta::assert_snapshot;
-    use itertools::Itertools;
-    use ra_db::fixture::WithFixture;
-    use ra_db::{SourceDatabase, Upcast};
+    use expect::{expect, Expect};
+    use ra_db::{fixture::WithFixture, SourceDatabase, Upcast};
 
-    fn import_map(ra_fixture: &str) -> String {
-        let db = TestDB::with_files(ra_fixture);
-        let crate_graph = db.crate_graph();
-
-        let s = crate_graph
-            .iter()
-            .filter_map(|krate| {
-                let cdata = &crate_graph[krate];
-                let name = cdata.display_name.as_ref()?;
-
-                let map = db.import_map(krate);
+    use crate::{test_db::TestDB, AssocContainerId, Lookup};
 
-                Some(format!("{}:\n{:?}", name, map))
-            })
-            .join("\n");
-        s
-    }
+    use super::*;
 
-    fn search_dependencies_of(ra_fixture: &str, krate_name: &str, query: Query) -> String {
+    fn check_search(ra_fixture: &str, krate_name: &str, query: Query, expect: Expect) {
         let db = TestDB::with_files(ra_fixture);
         let crate_graph = db.crate_graph();
         let krate = crate_graph
@@ -363,7 +345,7 @@ mod tests {
             })
             .unwrap();
 
-        search_dependencies(db.upcast(), krate, query)
+        let actual = search_dependencies(db.upcast(), krate, query)
             .into_iter()
             .filter_map(|item| {
                 let mark = match item {
@@ -376,14 +358,15 @@ mod tests {
                     let map = db.import_map(krate);
                     let path = map.path_of(item).unwrap();
                     format!(
-                        "{}::{} ({})",
+                        "{}::{} ({})\n",
                         crate_graph[krate].display_name.as_ref().unwrap(),
                         path,
                         mark
                     )
                 })
             })
-            .join("\n")
+            .collect::<String>();
+        expect.assert_eq(&actual)
     }
 
     fn assoc_to_trait(db: &dyn DefDatabase, item: ItemInNs) -> ItemInNs {
@@ -409,9 +392,28 @@ mod tests {
         }
     }
 
+    fn check(ra_fixture: &str, expect: Expect) {
+        let db = TestDB::with_files(ra_fixture);
+        let crate_graph = db.crate_graph();
+
+        let actual = crate_graph
+            .iter()
+            .filter_map(|krate| {
+                let cdata = &crate_graph[krate];
+                let name = cdata.display_name.as_ref()?;
+
+                let map = db.import_map(krate);
+
+                Some(format!("{}:\n{:?}\n", name, map))
+            })
+            .collect::<String>();
+
+        expect.assert_eq(&actual)
+    }
+
     #[test]
     fn smoke() {
-        let map = import_map(
+        check(
             r"
             //- /main.rs crate:main deps:lib
 
@@ -436,24 +438,23 @@ mod tests {
             pub struct Pub2; // t + v
             struct Priv;
         ",
+            expect![[r#"
+                main:
+                - publ1 (t)
+                - real_pu2 (t)
+                - real_pub (t)
+                - real_pub::Pub (t)
+                lib:
+                - Pub (t)
+                - Pub2 (t)
+                - Pub2 (v)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        main:
-        - publ1 (t)
-        - real_pu2 (t)
-        - real_pub (t)
-        - real_pub::Pub (t)
-        lib:
-        - Pub (t)
-        - Pub2 (t)
-        - Pub2 (v)
-        "###);
     }
 
     #[test]
     fn prefers_shortest_path() {
-        let map = import_map(
+        check(
             r"
             //- /main.rs crate:main
 
@@ -465,21 +466,20 @@ mod tests {
                 pub use super::sub::subsub::Def;
             }
         ",
+            expect![[r#"
+                main:
+                - sub (t)
+                - sub::Def (t)
+                - sub::subsub (t)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        main:
-        - sub (t)
-        - sub::Def (t)
-        - sub::subsub (t)
-        "###);
     }
 
     #[test]
     fn type_reexport_cross_crate() {
         // Reexports need to be visible from a crate, even if the original crate exports the item
         // at a shorter path.
-        let map = import_map(
+        check(
             r"
             //- /main.rs crate:main deps:lib
             pub mod m {
@@ -488,22 +488,21 @@ mod tests {
             //- /lib.rs crate:lib
             pub struct S;
         ",
+            expect![[r#"
+                main:
+                - m (t)
+                - m::S (t)
+                - m::S (v)
+                lib:
+                - S (t)
+                - S (v)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        main:
-        - m (t)
-        - m::S (t)
-        - m::S (v)
-        lib:
-        - S (t)
-        - S (v)
-        "###);
     }
 
     #[test]
     fn macro_reexport() {
-        let map = import_map(
+        check(
             r"
             //- /main.rs crate:main deps:lib
             pub mod m {
@@ -515,21 +514,20 @@ mod tests {
                 () => {};
             }
         ",
+            expect![[r#"
+                main:
+                - m (t)
+                - m::pub_macro (m)
+                lib:
+                - pub_macro (m)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        main:
-        - m (t)
-        - m::pub_macro (m)
-        lib:
-        - pub_macro (m)
-        "###);
     }
 
     #[test]
     fn module_reexport() {
         // Reexporting modules from a dependency adds all contents to the import map.
-        let map = import_map(
+        check(
             r"
             //- /main.rs crate:main deps:lib
             pub use lib::module as reexported_module;
@@ -538,24 +536,23 @@ mod tests {
                 pub struct S;
             }
         ",
+            expect![[r#"
+                main:
+                - reexported_module (t)
+                - reexported_module::S (t)
+                - reexported_module::S (v)
+                lib:
+                - module (t)
+                - module::S (t)
+                - module::S (v)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        main:
-        - reexported_module (t)
-        - reexported_module::S (t)
-        - reexported_module::S (v)
-        lib:
-        - module (t)
-        - module::S (t)
-        - module::S (v)
-        "###);
     }
 
     #[test]
     fn cyclic_module_reexport() {
         // A cyclic reexport does not hang.
-        let map = import_map(
+        check(
             r"
             //- /lib.rs crate:lib
             pub mod module {
@@ -567,36 +564,35 @@ mod tests {
                 pub use super::module;
             }
         ",
+            expect![[r#"
+                lib:
+                - module (t)
+                - module::S (t)
+                - module::S (v)
+                - sub (t)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        lib:
-        - module (t)
-        - module::S (t)
-        - module::S (v)
-        - sub (t)
-        "###);
     }
 
     #[test]
     fn private_macro() {
-        let map = import_map(
+        check(
             r"
             //- /lib.rs crate:lib
             macro_rules! private_macro {
                 () => {};
             }
         ",
-        );
+            expect![[r#"
+                lib:
 
-        assert_snapshot!(map, @r###"
-        lib:
-        "###);
+            "#]],
+        );
     }
 
     #[test]
     fn namespacing() {
-        let map = import_map(
+        check(
             r"
             //- /lib.rs crate:lib
             pub struct Thing;     // t + v
@@ -605,16 +601,15 @@ mod tests {
                 () => {};
             }
         ",
+            expect![[r#"
+                lib:
+                - Thing (m)
+                - Thing (t)
+                - Thing (v)
+            "#]],
         );
 
-        assert_snapshot!(map, @r###"
-        lib:
-        - Thing (m)
-        - Thing (t)
-        - Thing (v)
-        "###);
-
-        let map = import_map(
+        check(
             r"
             //- /lib.rs crate:lib
             pub mod Thing {}      // t
@@ -623,13 +618,12 @@ mod tests {
                 () => {};
             }
         ",
+            expect![[r#"
+                lib:
+                - Thing (m)
+                - Thing (t)
+            "#]],
         );
-
-        assert_snapshot!(map, @r###"
-        lib:
-        - Thing (m)
-        - Thing (t)
-        "###);
     }
 
     #[test]
@@ -658,25 +652,33 @@ mod tests {
             }
         "#;
 
-        let res = search_dependencies_of(ra_fixture, "main", Query::new("fmt"));
-        assert_snapshot!(res, @r###"
-        dep::fmt (t)
-        dep::Fmt (t)
-        dep::Fmt (v)
-        dep::Fmt (m)
-        dep::fmt::Display (t)
-        dep::format (v)
-        dep::fmt::Display (t)
-        "###);
-
-        let res = search_dependencies_of(ra_fixture, "main", Query::new("fmt").anchor_end());
-        assert_snapshot!(res, @r###"
-        dep::fmt (t)
-        dep::Fmt (t)
-        dep::Fmt (v)
-        dep::Fmt (m)
-        dep::fmt::Display (t)
-        "###);
+        check_search(
+            ra_fixture,
+            "main",
+            Query::new("fmt"),
+            expect![[r#"
+                dep::fmt (t)
+                dep::Fmt (t)
+                dep::Fmt (v)
+                dep::Fmt (m)
+                dep::fmt::Display (t)
+                dep::format (v)
+                dep::fmt::Display (t)
+            "#]],
+        );
+
+        check_search(
+            ra_fixture,
+            "main",
+            Query::new("fmt").anchor_end(),
+            expect![[r#"
+                dep::fmt (t)
+                dep::Fmt (t)
+                dep::Fmt (v)
+                dep::Fmt (m)
+                dep::fmt::Display (t)
+            "#]],
+        );
     }
 
     #[test]
@@ -689,26 +691,32 @@ mod tests {
             pub struct FMT;
         "#;
 
-        let res = search_dependencies_of(ra_fixture, "main", Query::new("FMT"));
-
-        assert_snapshot!(res, @r###"
-        dep::fmt (t)
-        dep::fmt (v)
-        dep::FMT (t)
-        dep::FMT (v)
-        "###);
-
-        let res = search_dependencies_of(ra_fixture, "main", Query::new("FMT").case_sensitive());
+        check_search(
+            ra_fixture,
+            "main",
+            Query::new("FMT"),
+            expect![[r#"
+                dep::fmt (t)
+                dep::fmt (v)
+                dep::FMT (t)
+                dep::FMT (v)
+            "#]],
+        );
 
-        assert_snapshot!(res, @r###"
-        dep::FMT (t)
-        dep::FMT (v)
-        "###);
+        check_search(
+            ra_fixture,
+            "main",
+            Query::new("FMT").case_sensitive(),
+            expect![[r#"
+                dep::FMT (t)
+                dep::FMT (v)
+            "#]],
+        );
     }
 
     #[test]
     fn search_limit() {
-        let res = search_dependencies_of(
+        check_search(
             r#"
         //- /main.rs crate:main deps:dep
         //- /dep.rs crate:dep
@@ -728,10 +736,10 @@ mod tests {
     "#,
             "main",
             Query::new("").limit(2),
+            expect![[r#"
+                dep::fmt (t)
+                dep::Fmt (t)
+            "#]],
         );
-        assert_snapshot!(res, @r###"
-        dep::fmt (t)
-        dep::Fmt (t)
-        "###);
     }
 }
diff --git a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
index 3da16fbe3..ae58948c4 100644
--- a/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
+++ b/crates/ra_hir_def/src/nameres/tests/mod_resolution.rs
@@ -686,27 +686,25 @@ fn unresolved_module_diagnostics() {
 
     let crate_def_map = db.crate_def_map(krate);
 
-    insta::assert_debug_snapshot!(
-        crate_def_map.diagnostics,
-        @r###"
-    [
-        UnresolvedModule {
-            module: Idx::<ModuleData>(0),
-            declaration: InFile {
-                file_id: HirFileId(
-                    FileId(
+    expect![[r#"
+        [
+            UnresolvedModule {
+                module: Idx::<ModuleData>(0),
+                declaration: InFile {
+                    file_id: HirFileId(
                         FileId(
-                            0,
+                            FileId(
+                                0,
+                            ),
                         ),
                     ),
-                ),
-                value: FileAstId::<ra_syntax::ast::generated::nodes::Module>(1),
+                    value: FileAstId::<ra_syntax::ast::generated::nodes::Module>(1),
+                },
+                candidate: "bar.rs",
             },
-            candidate: "bar.rs",
-        },
-    ]
-    "###
-    );
+        ]
+    "#]]
+    .assert_debug_eq(&crate_def_map.diagnostics);
 }
 
 #[test]
diff --git a/crates/ra_ide/src/call_info.rs b/crates/ra_ide/src/call_info.rs
index 14980afdd..ff602202f 100644
--- a/crates/ra_ide/src/call_info.rs
+++ b/crates/ra_ide/src/call_info.rs
@@ -161,6 +161,10 @@ impl ActiveParameter {
 
         let idx = active_parameter?;
         let mut params = signature.params(sema.db);
+        if !(idx < params.len()) {
+            mark::hit!(too_many_arguments);
+            return None;
+        }
         let (pat, ty) = params.swap_remove(idx);
         let name = pat?.to_string();
         Some(ActiveParameter { ty, name })
diff --git a/crates/ra_ide/src/completion/presentation.rs b/crates/ra_ide/src/completion/presentation.rs
index c7b74e635..9a94ff476 100644
--- a/crates/ra_ide/src/completion/presentation.rs
+++ b/crates/ra_ide/src/completion/presentation.rs
@@ -1160,6 +1160,22 @@ fn go(world: &WorldSnapshot) { go(w<|>) }
     }
 
     #[test]
+    fn too_many_arguments() {
+        mark::check!(too_many_arguments);
+        check_scores(
+            r#"
+struct Foo;
+fn f(foo: &Foo) { f(foo, w<|>) }
+"#,
+            expect![[r#"
+                st Foo []
+                fn f(…) []
+                bn foo []
+            "#]],
+        );
+    }
+
+    #[test]
     fn guesses_macro_braces() {
         check_edit(
             "vec!",