Merge #261

261: Add heavy test for code actions r=matklad a=flodiebold

Here's the test for the code actions; I didn't find anything fitting on crates.io ([assert-json-diff](https://crates.io/crates/assert-json-diff) looks kind of nice, but doesn't have anything like the wildcards), so I copied the cargo code as you suggested.

Co-authored-by: Florian Diebold <[email protected]>

6 files changed, 177 insertions, 19 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 42a962cf6..4f46e26fb 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1075,6 +1075,7 @@ version = "0.1.0"
 dependencies = [
  "difference 2.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "itertools 0.7.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "serde_json 1.0.32 (registry+https://github.com/rust-lang/crates.io-index)",
  "text_unit 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
diff --git a/crates/ra_lsp_server/tests/heavy_tests/main.rs b/crates/ra_lsp_server/tests/heavy_tests/main.rs
index cbc0c8844..26f5e3f20 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/main.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/main.rs
@@ -1,6 +1,10 @@
 mod support;
 
-use ra_lsp_server::req::{Runnables, RunnablesParams};
+use serde_json::json;
+
+use ra_lsp_server::req::{Runnables, RunnablesParams, CodeActionRequest, CodeActionParams};
+
+use languageserver_types::{Position, Range, CodeActionContext};
 
 use crate::support::project;
 
@@ -21,7 +25,7 @@ fn foo() {
             text_document: server.doc_id("lib.rs"),
             position: None,
         },
-        r#"[
+        json!([
           {
             "args": [ "test", "--", "foo", "--nocapture" ],
             "bin": "cargo",
@@ -51,7 +55,7 @@ fn foo() {
               }
             }
           }
-        ]"#,
+        ]),
     );
 }
 
@@ -78,7 +82,7 @@ fn test_eggs() {}
             text_document: server.doc_id("tests/spam.rs"),
             position: None,
         },
-        r#"[
+        json!([
           {
             "args": [ "test", "--package", "foo", "--test", "spam", "--", "test_eggs", "--nocapture" ],
             "bin": "cargo",
@@ -111,6 +115,63 @@ fn test_eggs() {}
               }
             }
           }
-        ]"#
+        ])
+    );
+}
+
+#[test]
+fn test_missing_module_code_action() {
+    let server = project(
+        r#"
+//- Cargo.toml
+[package]
+name = "foo"
+version = "0.0.0"
+
+//- src/lib.rs
+mod bar;
+
+fn main() {}
+"#,
+    );
+    server.wait_for_feedback("workspace loaded");
+    let empty_context = || CodeActionContext {
+        diagnostics: Vec::new(),
+        only: None,
+    };
+    server.request::<CodeActionRequest>(
+        CodeActionParams {
+            text_document: server.doc_id("src/lib.rs"),
+            range: Range::new(Position::new(0, 0), Position::new(0, 7)),
+            context: empty_context(),
+        },
+        json!([
+            {
+              "arguments": [
+                {
+                  "cursorPosition": null,
+                  "fileSystemEdits": [
+                    {
+                        "type": "createFile",
+                        "uri": "file:///[..]/src/bar.rs"
+                    }
+                  ],
+                  "label": "create module",
+                  "sourceFileEdits": []
+                }
+              ],
+              "command": "ra-lsp.applySourceChange",
+              "title": "create module"
+            }
+        ]),
+    );
+
+    server.request::<CodeActionRequest>(
+        CodeActionParams {
+            text_document: server.doc_id("src/lib.rs"),
+            range: Range::new(Position::new(2, 0), Position::new(2, 7)),
+            context: empty_context(),
+        },
+        json!([]),
     );
 }
diff --git a/crates/ra_lsp_server/tests/heavy_tests/support.rs b/crates/ra_lsp_server/tests/heavy_tests/support.rs
index 019048a3a..4b75be3ee 100644
--- a/crates/ra_lsp_server/tests/heavy_tests/support.rs
+++ b/crates/ra_lsp_server/tests/heavy_tests/support.rs
@@ -15,9 +15,9 @@ use languageserver_types::{
     DidOpenTextDocumentParams, TextDocumentIdentifier, TextDocumentItem, Url,
 };
 use serde::Serialize;
-use serde_json::{from_str, to_string_pretty, Value};
+use serde_json::{to_string_pretty, Value};
 use tempdir::TempDir;
-use test_utils::parse_fixture;
+use test_utils::{parse_fixture, find_mismatch};
 
 use ra_lsp_server::{
     main_loop, req,
@@ -88,23 +88,24 @@ impl Server {
         }
     }
 
-    pub fn request<R>(&self, params: R::Params, expected_resp: &str)
+    pub fn request<R>(&self, params: R::Params, expected_resp: Value)
     where
         R: Request,
         R::Params: Serialize,
     {
         let id = self.req_id.get();
         self.req_id.set(id + 1);
-        let expected_resp: Value = from_str(expected_resp).unwrap();
         let actual = self.send_request::<R>(id, params);
-        assert_eq!(
-            expected_resp,
-            actual,
-            "Expected:\n{}\n\
-             Actual:\n{}\n",
-            to_string_pretty(&expected_resp).unwrap(),
-            to_string_pretty(&actual).unwrap(),
-        );
+        match find_mismatch(&expected_resp, &actual) {
+            Some((expected_part, actual_part)) => panic!(
+                "JSON mismatch\nExpected:\n{}\nWas:\n{}\nExpected part:\n{}\nActual part:\n{}\n",
+                to_string_pretty(&expected_resp).unwrap(),
+                to_string_pretty(&actual).unwrap(),
+                to_string_pretty(expected_part).unwrap(),
+                to_string_pretty(actual_part).unwrap(),
+            ),
+            None => {}
+        }
     }
 
     fn send_request<R>(&self, id: u64, params: R::Params) -> Value
@@ -139,7 +140,7 @@ impl Server {
     pub fn wait_for_feedback_n(&self, feedback: &str, n: usize) {
         let f = |msg: &RawMessage| match msg {
             RawMessage::Notification(n) if n.method == "internalFeedback" => {
-                return n.clone().cast::<req::InternalFeedback>().unwrap() == feedback
+                return n.clone().cast::<req::InternalFeedback>().unwrap() == feedback;
             }
             _ => false,
         };
diff --git a/crates/ra_syntax/src/grammar/items/mod.rs b/crates/ra_syntax/src/grammar/items/mod.rs
index 682266908..4473c2fab 100644
--- a/crates/ra_syntax/src/grammar/items/mod.rs
+++ b/crates/ra_syntax/src/grammar/items/mod.rs
@@ -159,7 +159,7 @@ pub(super) fn maybe_item(p: &mut Parser, flavor: ItemFlavor) -> MaybeItem {
                 MaybeItem::Modifiers
             } else {
                 MaybeItem::None
-            }
+            };
         }
     };
 
diff --git a/crates/test_utils/Cargo.toml b/crates/test_utils/Cargo.toml
index fe0998ab8..8c8fcd7ae 100644
--- a/crates/test_utils/Cargo.toml
+++ b/crates/test_utils/Cargo.toml
@@ -8,3 +8,4 @@ authors = ["Aleksey Kladov <[email protected]>"]
 difference = "2.0.0"
 itertools = "0.7.8"
 text_unit = "0.1.2"
+serde_json = "1.0.24"
diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs
index e72ec9c47..0a94adb74 100644
--- a/crates/test_utils/src/lib.rs
+++ b/crates/test_utils/src/lib.rs
@@ -2,6 +2,7 @@ use std::fmt;
 
 use itertools::Itertools;
 use text_unit::{TextRange, TextUnit};
+use serde_json::Value;
 
 pub use difference::Changeset as __Changeset;
 
@@ -145,3 +146,96 @@ pub fn parse_fixture(fixture: &str) -> Vec<FixtureEntry> {
     flush!();
     res
 }
+
+// Comparison functionality borrowed from cargo:
+
+/// Compare a line with an expected pattern.
+/// - Use `[..]` as a wildcard to match 0 or more characters on the same line
+///   (similar to `.*` in a regex).
+pub fn lines_match(expected: &str, actual: &str) -> bool {
+    // Let's not deal with / vs \ (windows...)
+    // First replace backslash-escaped backslashes with forward slashes
+    // which can occur in, for example, JSON output
+    let expected = expected.replace("\\\\", "/").replace("\\", "/");
+    let mut actual: &str = &actual.replace("\\\\", "/").replace("\\", "/");
+    for (i, part) in expected.split("[..]").enumerate() {
+        match actual.find(part) {
+            Some(j) => {
+                if i == 0 && j != 0 {
+                    return false;
+                }
+                actual = &actual[j + part.len()..];
+            }
+            None => return false,
+        }
+    }
+    actual.is_empty() || expected.ends_with("[..]")
+}
+
+#[test]
+fn lines_match_works() {
+    assert!(lines_match("a b", "a b"));
+    assert!(lines_match("a[..]b", "a b"));
+    assert!(lines_match("a[..]", "a b"));
+    assert!(lines_match("[..]", "a b"));
+    assert!(lines_match("[..]b", "a b"));
+
+    assert!(!lines_match("[..]b", "c"));
+    assert!(!lines_match("b", "c"));
+    assert!(!lines_match("b", "cb"));
+}
+
+// Compares JSON object for approximate equality.
+// You can use `[..]` wildcard in strings (useful for OS dependent things such
+// as paths).  You can use a `"{...}"` string literal as a wildcard for
+// arbitrary nested JSON (useful for parts of object emitted by other programs
+// (e.g. rustc) rather than Cargo itself).  Arrays are sorted before comparison.
+pub fn find_mismatch<'a>(expected: &'a Value, actual: &'a Value) -> Option<(&'a Value, &'a Value)> {
+    use serde_json::Value::*;
+    match (expected, actual) {
+        (&Number(ref l), &Number(ref r)) if l == r => None,
+        (&Bool(l), &Bool(r)) if l == r => None,
+        (&String(ref l), &String(ref r)) if lines_match(l, r) => None,
+        (&Array(ref l), &Array(ref r)) => {
+            if l.len() != r.len() {
+                return Some((expected, actual));
+            }
+
+            let mut l = l.iter().collect::<Vec<_>>();
+            let mut r = r.iter().collect::<Vec<_>>();
+
+            l.retain(
+                |l| match r.iter().position(|r| find_mismatch(l, r).is_none()) {
+                    Some(i) => {
+                        r.remove(i);
+                        false
+                    }
+                    None => true,
+                },
+            );
+
+            if !l.is_empty() {
+                assert!(!r.is_empty());
+                Some((&l[0], &r[0]))
+            } else {
+                assert_eq!(r.len(), 0);
+                None
+            }
+        }
+        (&Object(ref l), &Object(ref r)) => {
+            let same_keys = l.len() == r.len() && l.keys().all(|k| r.contains_key(k));
+            if !same_keys {
+                return Some((expected, actual));
+            }
+
+            l.values()
+                .zip(r.values())
+                .filter_map(|(l, r)| find_mismatch(l, r))
+                .nth(0)
+        }
+        (&Null, &Null) => None,
+        // magic string literal "{...}" acts as wildcard for any sub-JSON
+        (&String(ref l), _) if l == "{...}" => None,
+        _ => Some((expected, actual)),
+    }
+}