Merge pull request #1512 from Xanewok/refactor-arc-handling

Clean up a bit

Author: Xanewok

rls-vfs/src/lib.rs

@@ -50,6 +50,7 @@ pub enum VfsSpan {
     Utf16CodeUnit(SpanData),
 }
 
+#[allow(clippy::len_without_is_empty)]
 impl VfsSpan {
     pub fn from_usv(span: span::Span<span::ZeroIndexed>, len: Option<u64>) -> VfsSpan {
         VfsSpan::UnicodeScalarValue(SpanData { span, len })

rls/src/actions/hover.rs

@@ -255,12 +255,11 @@ fn tooltip_local_variable_usage(
     doc_url: Option<String>,
 ) -> Vec<MarkedString> {
     debug!("tooltip_local_variable_usage: {}", def.name);
-    let vfs = ctx.vfs.clone();
 
     let the_type = def.value.trim().into();
     let mut context = String::new();
     if ctx.config.lock().unwrap().show_hover_context {
-        match vfs.load_line(&def.span.file, def.span.range.row_start) {
+        match ctx.vfs.load_line(&def.span.file, def.span.range.row_start) {
             Ok(line) => {
                 context.push_str(line.trim());
             }
@@ -282,7 +281,7 @@ fn tooltip_local_variable_usage(
 fn tooltip_type(ctx: &InitActionContext, def: &Def, doc_url: Option<String>) -> Vec<MarkedString> {
     debug!("tooltip_type: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
 
     let the_type = || def.value.trim().into();
     let the_type = def_decl(def, &vfs, the_type);
@@ -299,7 +298,7 @@ fn tooltip_field_or_variant(
 ) -> Vec<MarkedString> {
     debug!("tooltip_field_or_variant: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
 
     let the_type = def.value.trim().into();
     let docs = def_docs(def, &vfs);
@@ -315,7 +314,7 @@ fn tooltip_struct_enum_union_trait(
 ) -> Vec<MarkedString> {
     debug!("tooltip_struct_enum_union_trait: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
     let fmt_config = ctx.fmt_config();
     // We hover often, so use the in-process one to speed things up.
     let fmt = Rustfmt::Internal;
@@ -341,7 +340,7 @@ fn tooltip_struct_enum_union_trait(
 fn tooltip_mod(ctx: &InitActionContext, def: &Def, doc_url: Option<String>) -> Vec<MarkedString> {
     debug!("tooltip_mod: name: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
 
     let the_type = def.value.trim();
     let the_type = the_type.replace("\\\\", "/");
@@ -370,7 +369,7 @@ fn tooltip_function_method(
 ) -> Vec<MarkedString> {
     debug!("tooltip_function_method: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
     let fmt_config = ctx.fmt_config();
     // We hover often, so use the in-process one to speed things up.
     let fmt = Rustfmt::Internal;
@@ -441,11 +440,9 @@ fn tooltip_static_const_decl(
 ) -> Vec<MarkedString> {
     debug!("tooltip_static_const_decl: {}", def.name);
 
-    let vfs = ctx.vfs.clone();
+    let vfs = &ctx.vfs;
 
-    let the_type = def.value.trim().into();
-
-    let the_type = def_decl(def, &vfs, || the_type);
+    let the_type = def_decl(def, &vfs, || def.value.trim().into());
     let docs = def_docs(def, &vfs);
     let context = None;
 
@@ -684,15 +681,14 @@ fn racer_match_to_def(ctx: &InitActionContext, m: &racer::Match) -> Option<Def>
 /// Uses racer to synthesize a `Def` for the given `span`. If no appropriate
 /// match is found with coordinates, `None` is returned.
 fn racer_def(ctx: &InitActionContext, span: &Span<ZeroIndexed>) -> Option<Def> {
-    let vfs = ctx.vfs.clone();
     let file_path = &span.file;
 
     if !file_path.as_path().exists() {
         error!("racer_def: skipping non-existant file: {:?}", file_path);
         return None;
     }
 
-    let name = vfs.load_line(file_path.as_path(), span.range.row_start).ok().and_then(|line| {
+    let name = ctx.vfs.load_line(file_path.as_path(), span.range.row_start).ok().and_then(|line| {
         let col_start = span.range.col_start.0 as usize;
         let col_end = span.range.col_end.0 as usize;
         line.get(col_start..col_end).map(ToOwned::to_owned)

rls/src/actions/mod.rs

@@ -86,9 +86,9 @@ impl ActionContext {
         let ctx = match *self {
             ActionContext::Uninit(ref uninit) => {
                 let ctx = InitActionContext::new(
-                    uninit.analysis.clone(),
-                    uninit.vfs.clone(),
-                    uninit.config.clone(),
+                    Arc::clone(&uninit.analysis),
+                    Arc::clone(&uninit.vfs),
+                    Arc::clone(&uninit.config),
                     client_capabilities,
                     current_project,
                     uninit.pid,
@@ -189,7 +189,7 @@ impl InitActionContext {
         pid: u32,
         client_supports_cmd_run: bool,
     ) -> InitActionContext {
-        let build_queue = BuildQueue::new(vfs.clone(), config.clone());
+        let build_queue = BuildQueue::new(Arc::clone(&vfs), Arc::clone(&config));
         let analysis_queue = Arc::new(AnalysisQueue::init());
         InitActionContext {
             analysis,
@@ -226,7 +226,7 @@ impl InitActionContext {
                 info!("loading cargo project model");
                 let pm = ProjectModel::load(&self.current_project.join("Cargo.toml"), &self.vfs)?;
                 let pm = Arc::new(pm);
-                *self.project_model.lock().unwrap() = Some(pm.clone());
+                *self.project_model.lock().unwrap() = Some(Arc::clone(&pm));
                 Ok(pm)
             }
         }
@@ -245,7 +245,7 @@ impl InitActionContext {
                 }
             }
         }
-        racer::FileCache::new(RacerVfs(self.vfs.clone()))
+        racer::FileCache::new(RacerVfs(Arc::clone(&self.vfs)))
     }
 
     pub fn racer_session<'c>(&self, cache: &'c racer::FileCache) -> racer::Session<'c> {
@@ -326,15 +326,15 @@ impl InitActionContext {
         let pbh = {
             let config = self.config.lock().unwrap();
             PostBuildHandler {
-                analysis: self.analysis.clone(),
-                analysis_queue: self.analysis_queue.clone(),
-                previous_build_results: self.previous_build_results.clone(),
-                file_to_crates: self.file_to_crates.clone(),
+                analysis: Arc::clone(&self.analysis),
+                analysis_queue: Arc::clone(&self.analysis_queue),
+                previous_build_results: Arc::clone(&self.previous_build_results),
+                file_to_crates: Arc::clone(&self.file_to_crates),
                 project_path: project_path.to_owned(),
                 show_warnings: config.show_warnings,
                 related_information_support: self.client_capabilities.related_information_support,
-                shown_cargo_error: self.shown_cargo_error.clone(),
-                active_build_count: self.active_build_count.clone(),
+                shown_cargo_error: Arc::clone(&self.shown_cargo_error),
+                active_build_count: Arc::clone(&self.active_build_count),
                 crate_blacklist: config.crate_blacklist.as_ref().clone(),
                 notifier: Box::new(BuildDiagnosticsNotifier::new(out.clone())),
                 blocked_threads: vec![],

rls/src/actions/notifications.rs

@@ -5,6 +5,7 @@ use crate::Span;
 use log::{debug, trace, warn};
 use rls_vfs::{Change, VfsSpan};
 use std::sync::atomic::Ordering;
+use std::sync::Arc;
 
 use crate::build::*;
 use crate::lsp_data::request::{RangeFormatting, RegisterCapability, UnregisterCapability};
@@ -170,7 +171,7 @@ impl BlockingNotificationAction for DidChangeConfiguration {
 
             if needs_inference {
                 let project_dir = ctx.current_project.clone();
-                let config = ctx.config.clone();
+                let config = Arc::clone(&ctx.config);
                 // Will lock and access Config just outside the current scope
                 thread::spawn(move || {
                     let mut config = config.lock().unwrap();

rls/src/actions/post_build.rs

@@ -54,7 +54,7 @@ impl PostBuildHandler {
 
                 // Emit appropriate diagnostics using the ones from build.
                 self.handle_messages(&cwd, &messages);
-                let analysis_queue = self.analysis_queue.clone();
+                let analysis_queue = Arc::clone(&self.analysis_queue);
 
                 {
                     let mut files_to_crates = self.file_to_crates.lock().unwrap();
@@ -232,10 +232,13 @@ pub struct AnalysisQueue {
 impl AnalysisQueue {
     // Create a new queue and start the worker thread.
     pub fn init() -> AnalysisQueue {
-        let queue = Arc::new(Mutex::new(Vec::new()));
-        let queue_clone = queue.clone();
-        let worker_thread =
-            thread::spawn(move || AnalysisQueue::run_worker_thread(queue_clone)).thread().clone();
+        let queue = Arc::default();
+        let worker_thread = thread::spawn({
+            let queue = Arc::clone(&queue);
+            || AnalysisQueue::run_worker_thread(queue)
+        })
+        .thread()
+        .clone();
 
         AnalysisQueue { cur_cwd: Mutex::new(None), queue, worker_thread }
     }
@@ -293,6 +296,7 @@ impl Drop for AnalysisQueue {
     }
 }
 
+#[allow(clippy::large_enum_variant)]
 enum QueuedJob {
     Job(Job),
     Terminate,

rls/src/actions/requests.rs

@@ -183,9 +183,8 @@ impl RequestAction for Definition {
         // Save-analysis thread.
         let file_path = parse_file_path!(&params.text_document.uri, "goto_def")?;
         let span = ctx.convert_pos_to_span(file_path.clone(), params.position);
-        let analysis = ctx.analysis.clone();
 
-        if let Ok(out) = analysis.goto_def(&span) {
+        if let Ok(out) = ctx.analysis.goto_def(&span) {
             let result = vec![ls_util::rls_to_location(&out)];
             trace!("goto_def (compiler): {:?}", result);
             Ok(result)

rls/src/build/cargo.rs

@@ -40,48 +40,50 @@ pub(super) fn cargo(
     package_arg: PackageArg,
     progress_sender: Sender<ProgressUpdate>,
 ) -> BuildResult {
-    let compilation_cx = internals.compilation_cx.clone();
-    let config = internals.config.clone();
-    let vfs = internals.vfs.clone();
-    let env_lock = internals.env_lock.clone();
-
-    let diagnostics = Arc::new(Mutex::new(vec![]));
-    let diagnostics_clone = diagnostics.clone();
-    let analysis = Arc::new(Mutex::new(vec![]));
-    let analysis_clone = analysis.clone();
-    let input_files = Arc::new(Mutex::new(HashMap::new()));
-    let input_files_clone = input_files.clone();
-    let out = Arc::new(Mutex::new(vec![]));
-    let out_clone = out.clone();
+    let compilation_cx = Arc::clone(&internals.compilation_cx);
+    let config = Arc::clone(&internals.config);
+    let vfs = Arc::clone(&internals.vfs);
+    let env_lock = Arc::clone(&internals.env_lock);
+
+    let diagnostics = Arc::default();
+    let analysis = Arc::default();
+    let input_files = Arc::default();
+    let out = Arc::default();
 
     // Cargo may or may not spawn threads to run the various builds, since
     // we may be in separate threads we need to block and wait our thread.
     // However, if Cargo doesn't run a separate thread, then we'll just wait
     // forever. Therefore, we spawn an extra thread here to be safe.
-    let handle = thread::spawn(move || {
-        run_cargo(
-            compilation_cx,
-            package_arg,
-            config,
-            vfs,
-            env_lock,
-            diagnostics,
-            analysis,
-            input_files,
-            out,
-            progress_sender,
-        )
+    let handle = thread::spawn({
+        let diagnostics = Arc::clone(&diagnostics);
+        let analysis = Arc::clone(&analysis);
+        let input_files = Arc::clone(&input_files);
+        let out = Arc::clone(&out);
+        || {
+            run_cargo(
+                compilation_cx,
+                package_arg,
+                config,
+                vfs,
+                env_lock,
+                diagnostics,
+                analysis,
+                input_files,
+                out,
+                progress_sender,
+            )
+        }
     });
 
     match handle.join().map_err(|_| failure::err_msg("thread panicked")).and_then(|res| res) {
         Ok(ref cwd) => {
-            let diagnostics = Arc::try_unwrap(diagnostics_clone).unwrap().into_inner().unwrap();
-            let analysis = Arc::try_unwrap(analysis_clone).unwrap().into_inner().unwrap();
-            let input_files = Arc::try_unwrap(input_files_clone).unwrap().into_inner().unwrap();
+            let diagnostics = Arc::try_unwrap(diagnostics).unwrap().into_inner().unwrap();
+            let analysis = Arc::try_unwrap(analysis).unwrap().into_inner().unwrap();
+            let input_files = Arc::try_unwrap(input_files).unwrap().into_inner().unwrap();
             BuildResult::Success(cwd.clone(), diagnostics, analysis, input_files, true)
         }
         Err(error) => {
-            let stdout = String::from_utf8(out_clone.lock().unwrap().to_owned()).unwrap();
+            let stdout = String::from_utf8(out.lock().unwrap().to_owned()).unwrap();
 
             let (manifest_path, manifest_error_range) = {
                 let mae = error.downcast_ref::<ManifestAwareError>();
@@ -257,7 +259,7 @@ fn run_cargo_ws(
         analysis,
         input_files,
         progress_sender,
-        reached_primary.clone(),
+        Arc::clone(&reached_primary),
     );
 
     let exec = Arc::new(exec) as Arc<dyn Executor>;
@@ -363,9 +365,9 @@ impl Executor for RlsExecutor {
             .as_cargo_mut()
             .expect("build plan should be properly initialized before running Cargo");
 
-        let only_primary = |unit: &Unit<'_>| self.is_primary_package(unit.pkg.package_id());
+        let only_primary = |unit: Unit<'_>| self.is_primary_package(unit.pkg.package_id());
 
-        plan.emplace_dep_with_filter(unit, cx, &only_primary);
+        plan.emplace_dep_with_filter(*unit, cx, &only_primary);
     }
 
     fn force_rebuild(&self, unit: &Unit<'_>) -> bool {