3 files changed, 84 insertions, 32 deletions

diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs
index 52c7e2bdb..9901fe931 100644
--- a/crates/ra_lsp_server/src/main_loop.rs
+++ b/crates/ra_lsp_server/src/main_loop.rs
@@ -5,7 +5,14 @@ mod handlers;
 mod subscriptions;
 pub(crate) mod pending_requests;
 
-use std::{error::Error, fmt, panic, path::PathBuf, sync::Arc, time::Instant};
+use std::{
+    env,
+    error::Error,
+    fmt, panic,
+    path::PathBuf,
+    sync::Arc,
+    time::{Duration, Instant},
+};
 
 use crossbeam_channel::{select, unbounded, RecvError, Sender};
 use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response};
@@ -425,6 +432,19 @@ fn loop_turn(
             loop_state.subscriptions.subscriptions(),
         )
     }
+
+    let loop_duration = loop_start.elapsed();
+    if loop_duration > Duration::from_millis(10) {
+        log::error!("overly long loop turn: {:?}", loop_duration);
+        if env::var("RA_PROFILE").is_ok() {
+            show_message(
+                req::MessageType::Error,
+                format!("overly long loop turn: {:?}", loop_duration),
+                &connection.sender,
+            );
+        }
+    }
+
     Ok(())
 }
 
@@ -452,7 +472,7 @@ fn on_request(
     world: &mut WorldState,
     pending_requests: &mut PendingRequests,
     pool: &ThreadPool,
-    sender: &Sender<Task>,
+    task_sender: &Sender<Task>,
     msg_sender: &Sender<Message>,
     request_received: Instant,
     req: Request,
@@ -461,7 +481,7 @@ fn on_request(
         req: Some(req),
         pool,
         world,
-        sender,
+        task_sender,
         msg_sender,
         pending_requests,
         request_received,
@@ -661,7 +681,7 @@ struct PoolDispatcher<'a> {
     world: &'a mut WorldState,
     pending_requests: &'a mut PendingRequests,
     msg_sender: &'a Sender<Message>,
-    sender: &'a Sender<Task>,
+    task_sender: &'a Sender<Task>,
     request_received: Instant,
 }
 
@@ -708,7 +728,7 @@ impl<'a> PoolDispatcher<'a> {
 
         self.pool.execute({
             let world = self.world.snapshot();
-            let sender = self.sender.clone();
+            let sender = self.task_sender.clone();
             move || {
                 let result = f(world, params);
                 let task = result_to_task::<R>(id, result);
@@ -786,7 +806,7 @@ fn update_file_notifications_on_threadpool(
     pool: &ThreadPool,
     world: WorldSnapshot,
     publish_decorations: bool,
-    sender: Sender<Task>,
+    task_sender: Sender<Task>,
     subscriptions: Vec<FileId>,
 ) {
     log::trace!("updating notifications for {:?}", subscriptions);
@@ -802,7 +822,7 @@ fn update_file_notifications_on_threadpool(
                     }
                     Ok(params) => {
                         let not = notification_new::<req::PublishDiagnostics>(params);
-                        sender.send(Task::Notify(not)).unwrap();
+                        task_sender.send(Task::Notify(not)).unwrap();
                     }
                 }
             }
@@ -815,7 +835,7 @@ fn update_file_notifications_on_threadpool(
                     }
                     Ok(params) => {
                         let not = notification_new::<req::PublishDecorations>(params);
-                        sender.send(Task::Notify(not)).unwrap();
+                        task_sender.send(Task::Notify(not)).unwrap();
                     }
                 }
             }
diff --git a/crates/test_utils/src/lib.rs b/crates/test_utils/src/lib.rs
index 659f77b71..265fcf8da 100644
--- a/crates/test_utils/src/lib.rs
+++ b/crates/test_utils/src/lib.rs
@@ -21,6 +21,12 @@ pub use difference::Changeset as __Changeset;
 
 pub const CURSOR_MARKER: &str = "<|>";
 
+/// Asserts that two strings are equal, otherwise displays a rich diff between them.
+///
+/// The diff shows changes from the "original" left string to the "actual" right string.
+///
+/// All arguments starting from and including the 3rd one are passed to
+/// `eprintln!()` macro in case of text inequality.
 #[macro_export]
 macro_rules! assert_eq_text {
     ($left:expr, $right:expr) => {
@@ -42,6 +48,7 @@ macro_rules! assert_eq_text {
     }};
 }
 
+/// Infallible version of `try_extract_offset()`.
 pub fn extract_offset(text: &str) -> (TextUnit, String) {
     match try_extract_offset(text) {
         None => panic!("text should contain cursor marker"),
@@ -49,6 +56,8 @@ pub fn extract_offset(text: &str) -> (TextUnit, String) {
     }
 }
 
+/// Returns the offset of the first occurence of `<|>` marker and the copy of `text`
+/// without the marker.
 fn try_extract_offset(text: &str) -> Option<(TextUnit, String)> {
     let cursor_pos = text.find(CURSOR_MARKER)?;
     let mut new_text = String::with_capacity(text.len() - CURSOR_MARKER.len());
@@ -58,6 +67,7 @@ fn try_extract_offset(text: &str) -> Option<(TextUnit, String)> {
     Some((cursor_pos, new_text))
 }
 
+/// Infallible version of `try_extract_range()`.
 pub fn extract_range(text: &str) -> (TextRange, String) {
     match try_extract_range(text) {
         None => panic!("text should contain cursor marker"),
@@ -65,6 +75,8 @@ pub fn extract_range(text: &str) -> (TextRange, String) {
     }
 }
 
+/// Returns `TextRange` between the first two markers `<|>...<|>` and the copy
+/// of `text` without both of these markers.
 fn try_extract_range(text: &str) -> Option<(TextRange, String)> {
     let (start, text) = try_extract_offset(text)?;
     let (end, text) = try_extract_offset(&text)?;
@@ -85,6 +97,11 @@ impl From<RangeOrOffset> for TextRange {
     }
 }
 
+/// Extracts `TextRange` or `TextUnit` depending on the amount of `<|>` markers
+/// found in `text`.
+///
+/// # Panics
+/// Panics if no `<|>` marker is present in the `text`.
 pub fn extract_range_or_offset(text: &str) -> (RangeOrOffset, String) {
     if let Some((range, text)) = try_extract_range(text) {
         return (RangeOrOffset::Range(range), text);
@@ -93,7 +110,7 @@ pub fn extract_range_or_offset(text: &str) -> (RangeOrOffset, String) {
     (RangeOrOffset::Offset(offset), text)
 }
 
-/// Extracts ranges, marked with `<tag> </tag>` paris from the `text`
+/// Extracts ranges, marked with `<tag> </tag>` pairs from the `text`
 pub fn extract_ranges(mut text: &str, tag: &str) -> (Vec<TextRange>, String) {
     let open = format!("<{}>", tag);
     let close = format!("</{}>", tag);
@@ -127,9 +144,9 @@ pub fn extract_ranges(mut text: &str, tag: &str) -> (Vec<TextRange>, String) {
     (ranges, res)
 }
 
+/// Inserts `<|>` marker into the `text` at `offset`.
 pub fn add_cursor(text: &str, offset: TextUnit) -> String {
-    let offset: u32 = offset.into();
-    let offset: usize = offset as usize;
+    let offset: usize = offset.to_usize();
     let mut res = String::new();
     res.push_str(&text[..offset]);
     res.push_str("<|>");
@@ -152,19 +169,6 @@ pub struct FixtureEntry {
 ///  // - other meta
 ///  ```
 pub fn parse_fixture(fixture: &str) -> Vec<FixtureEntry> {
-    let mut res = Vec::new();
-    let mut buf = String::new();
-    let mut meta: Option<&str> = None;
-
-    macro_rules! flush {
-        () => {
-            if let Some(meta) = meta {
-                res.push(FixtureEntry { meta: meta.to_string(), text: buf.clone() });
-                buf.clear();
-            }
-        };
-    };
-
     let margin = fixture
         .lines()
         .filter(|it| it.trim_start().starts_with("//-"))
@@ -184,6 +188,19 @@ pub fn parse_fixture(fixture: &str) -> Vec<FixtureEntry> {
             }
         });
 
+    let mut res = Vec::new();
+    let mut buf = String::new();
+    let mut meta: Option<&str> = None;
+
+    macro_rules! flush {
+        () => {
+            if let Some(meta) = meta {
+                res.push(FixtureEntry { meta: meta.to_string(), text: buf.clone() });
+                buf.clear();
+            }
+        };
+    };
+
     for line in lines {
         if line.starts_with("//-") {
             flush!();
@@ -236,11 +253,10 @@ fn lines_match_works() {
     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.
+/// 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. 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) {
@@ -286,6 +302,14 @@ pub fn find_mismatch<'a>(expected: &'a Value, actual: &'a Value) -> Option<(&'a
     }
 }
 
+/// Calls callback `f` with input code and file paths of all `.rs` files from `test_data_dir`
+/// subdirectories defined by `paths`.
+///
+/// If the content of the matching `.txt` file differs from the output of `f()`
+/// the test will fail.
+///
+/// If there is no matching `.txt` file it will be created and filled with the
+/// output of `f()`, but the test will fail.
 pub fn dir_tests<F>(test_data_dir: &Path, paths: &[&str], f: F)
 where
     F: Fn(&str, &Path) -> String,
@@ -307,6 +331,7 @@ where
     }
 }
 
+/// Collects all `.rs` files from `test_data_dir` subdirectories defined by `paths`.
 pub fn collect_tests(test_data_dir: &Path, paths: &[&str]) -> Vec<(PathBuf, String)> {
     paths
         .iter()
@@ -321,6 +346,7 @@ pub fn collect_tests(test_data_dir: &Path, paths: &[&str]) -> Vec<(PathBuf, Stri
         .collect()
 }
 
+/// Collects paths to all `.rs` files from `dir` in a sorted `Vec<PathBuf>`.
 fn test_from_dir(dir: &Path) -> Vec<PathBuf> {
     let mut acc = Vec::new();
     for file in fs::read_dir(&dir).unwrap() {
@@ -334,6 +360,7 @@ fn test_from_dir(dir: &Path) -> Vec<PathBuf> {
     acc
 }
 
+/// Returns the path to the root directory of `rust-analyzer` project.
 pub fn project_dir() -> PathBuf {
     let dir = env!("CARGO_MANIFEST_DIR");
     PathBuf::from(dir).parent().unwrap().parent().unwrap().to_owned()
@@ -356,6 +383,9 @@ pub fn read_text(path: &Path) -> String {
         .replace("\r\n", "\n")
 }
 
+/// Returns `false` if slow tests should not run, otherwise returns `true` and
+/// also creates a file at `./target/.slow_tests_cookie` which serves as a flag
+/// that slow tests did run.
 pub fn skip_slow_tests() -> bool {
     let should_skip = std::env::var("CI").is_err() && std::env::var("RUN_SLOW_TESTS").is_err();
     if should_skip {
@@ -367,8 +397,9 @@ pub fn skip_slow_tests() -> bool {
     should_skip
 }
 
-const REWRITE: bool = false;
-
+/// Asserts that `expected` and `actual` strings are equal. If they differ only
+/// in trailing or leading whitespace the test won't fail and
+/// the contents of `actual` will be written to the file located at `path`.
 fn assert_equal_text(expected: &str, actual: &str, path: &Path) {
     if expected == actual {
         return;
@@ -381,6 +412,7 @@ fn assert_equal_text(expected: &str, actual: &str, path: &Path) {
         fs::write(path, actual).unwrap();
         return;
     }
+    const REWRITE: bool = false;
     if REWRITE {
         println!("rewriting {}", pretty_path.display());
         fs::write(path, actual).unwrap();
diff --git a/crates/test_utils/src/marks.rs b/crates/test_utils/src/marks.rs
index fe1813947..f8fabfaff 100644
--- a/crates/test_utils/src/marks.rs
+++ b/crates/test_utils/src/marks.rs
@@ -1,4 +1,4 @@
-//! This module implements manually tracked test coverage, which useful for
+//! This module implements manually tracked test coverage, which is useful for
 //! quickly finding a test responsible for testing a particular bit of code.
 //!
 //! See <https://matklad.github.io/2018/06/18/a-trick-for-test-maintenance.html>