Use Ark URIs for fallback sources in the debugger (#852)

* Add `RMain::start_debug()` and `RMain::stop_debug()`

* Move creation of fallback sources to `RMain`

* Send Ark vdoc URIs instead of DAP source refs

* Clean up debug vdocs when debug session ends

Author: lionel-

crates/ark/src/dap/dap.rs

@@ -20,7 +20,6 @@ use stdext::log_error;
 use stdext::spawn;
 
 use crate::dap::dap_r_main::FrameInfo;
-use crate::dap::dap_r_main::FrameSource;
 use crate::dap::dap_server;
 use crate::request::RRequest;
 use crate::thread::RThreadSafe;
@@ -62,8 +61,7 @@ pub struct Dap {
     /// associated files (i.e. no `srcref` attribute). The `source` is the key to
     /// ensure that we don't insert the same function multiple times, which would result
     /// in duplicate virtual editors being opened on the client side.
-    pub fallback_sources: HashMap<String, i32>,
-    current_source_reference: i32,
+    pub fallback_sources: HashMap<String, String>,
 
     /// Maps a frame `id` from within the `stack` to a unique
     /// `variables_reference` id, which then allows you to use
@@ -105,7 +103,6 @@ impl Dap {
             backend_events_tx: None,
             stack: None,
             fallback_sources: HashMap::new(),
-            current_source_reference: 1,
             frame_id_to_variables_reference: HashMap::new(),
             variables_reference_to_r_object: HashMap::new(),
             current_variables_reference: 1,
@@ -125,8 +122,14 @@ impl Dap {
         shared
     }
 
-    pub fn start_debug(&mut self, mut stack: Vec<FrameInfo>, preserve_focus: bool) {
-        self.load_fallback_sources(&stack);
+    pub fn start_debug(
+        &mut self,
+        mut stack: Vec<FrameInfo>,
+        preserve_focus: bool,
+        fallback_sources: HashMap<String, String>,
+    ) {
+        self.fallback_sources.extend(fallback_sources);
+
         self.load_variables_references(&mut stack);
         self.stack = Some(stack);
 
@@ -152,7 +155,7 @@ impl Dap {
     pub fn stop_debug(&mut self) {
         // Reset state
         self.stack = None;
-        self.clear_fallback_sources();
+        self.fallback_sources.clear();
         self.clear_variables_reference_maps();
         self.reset_variables_reference_count();
         self.is_debugging = false;
@@ -172,31 +175,6 @@ impl Dap {
         }
     }
 
-    /// Load `fallback_sources` with this stack's text sources
-    fn load_fallback_sources(&mut self, stack: &Vec<FrameInfo>) {
-        for frame in stack.iter() {
-            let source = &frame.source;
-
-            match source {
-                FrameSource::File(_) => continue,
-                FrameSource::Text(source) => {
-                    if self.fallback_sources.contains_key(source) {
-                        // Already in `fallback_sources`, associated with an existing `source_reference`
-                        continue;
-                    }
-                    self.fallback_sources
-                        .insert(source.clone(), self.current_source_reference);
-                    self.current_source_reference += 1;
-                },
-            }
-        }
-    }
-
-    fn clear_fallback_sources(&mut self) {
-        self.fallback_sources.clear();
-        self.current_source_reference = 1;
-    }
-
     fn load_variables_references(&mut self, stack: &mut Vec<FrameInfo>) {
         // Reset the last step's maps. The frontend should never ask for these variable
         // references or variables again (and if it does due to some race condition, we

crates/ark/src/dap/dap_r_main.rs

@@ -5,6 +5,7 @@
 //
 //
 
+use std::collections::HashMap;
 use std::sync::Arc;
 use std::sync::Mutex;
 
@@ -120,16 +121,22 @@ impl RMainDap {
         self.debugging
     }
 
-    pub fn start_debug(&mut self, stack: Vec<FrameInfo>, preserve_focus: bool) {
+    pub fn start_debug(
+        &mut self,
+        stack: Vec<FrameInfo>,
+        preserve_focus: bool,
+        fallback_sources: HashMap<String, String>,
+    ) {
         self.debugging = true;
         let mut dap = self.dap.lock().unwrap();
-        dap.start_debug(stack, preserve_focus)
+        dap.start_debug(stack, preserve_focus, fallback_sources)
     }
 
     pub fn stop_debug(&mut self) {
         let mut dap = self.dap.lock().unwrap();
         dap.stop_debug();
         drop(dap);
+
         self.reset_frame_id();
         self.debugging = false;
     }

crates/ark/src/dap/dap_server.rs

@@ -389,59 +389,13 @@ impl<R: Read, W: Write> DapServer<R, W> {
         self.server.respond(rsp).unwrap();
     }
 
-    fn handle_source(&mut self, req: Request, args: SourceArguments) {
-        // We fully expect a `source` argument to exist, it is only for backwards
-        // compatibility that it could be `None`
-        let Some(source) = args.source else {
-            let message = "Missing `Source` to extract a `source_reference` from.";
-            log::error!("{message}");
-            let rsp = req.error(message);
-            self.server.respond(rsp).unwrap();
-            return;
-        };
-
-        // We expect a `source_reference`. If the client had a `path` then it would
-        // not have asked us for the source content.
-        let Some(source_reference) = source.source_reference else {
-            let message = "Missing `source_reference` to locate content for.";
-            log::error!("{message}");
-            let rsp = req.error(message);
-            self.server.respond(rsp).unwrap();
-            return;
-        };
-
-        // Try to find the source content for this `source_reference`
-        let Some(content) = self.find_source_content(source_reference) else {
-            let message =
-                "Failed to locate source content for `source_reference` {source_reference}.";
-            log::error!("{message}");
-            let rsp = req.error(message);
-            self.server.respond(rsp).unwrap();
-            return;
-        };
-
-        let rsp = req.success(ResponseBody::Source(SourceResponse {
-            content,
-            mime_type: None,
-        }));
-
+    fn handle_source(&mut self, req: Request, _args: SourceArguments) {
+        let message = "Unsupported `source` request: {req:?}";
+        log::error!("{message}");
+        let rsp = req.error(message);
         self.server.respond(rsp).unwrap();
     }
 
-    fn find_source_content(&self, source_reference: i32) -> Option<String> {
-        let state = self.state.lock().unwrap();
-        let fallback_sources = &state.fallback_sources;
-
-        // Match up the requested `source_reference` with one in our `fallback_sources`
-        for (current_source, current_source_reference) in fallback_sources.iter() {
-            if &source_reference == current_source_reference {
-                return Some(current_source.clone());
-            }
-        }
-
-        None
-    }
-
     fn handle_scopes(&mut self, req: Request, args: ScopesArguments) {
         let state = self.state.lock().unwrap();
         let frame_id_to_variables_reference = &state.frame_id_to_variables_reference;
@@ -566,7 +520,7 @@ impl<R: Read, W: Write> DapServer<R, W> {
     }
 }
 
-fn into_dap_frame(frame: &FrameInfo, fallback_sources: &HashMap<String, i32>) -> StackFrame {
+fn into_dap_frame(frame: &FrameInfo, fallback_sources: &HashMap<String, String>) -> StackFrame {
     let id = frame.id;
     let source_name = frame.source_name.clone();
     let frame_name = frame.frame_name.clone();
@@ -579,21 +533,18 @@ fn into_dap_frame(frame: &FrameInfo, fallback_sources: &HashMap<String, i32>) ->
     // Retrieve either `path` or `source_reference` depending on the `source` type.
     // In the `Text` case, a `source_reference` should always exist because we loaded
     // the map with all possible text values in `start_debug()`.
-    let (path, source_reference) = match source {
-        FrameSource::File(path) => (Some(path), None),
-        FrameSource::Text(source) => {
-            let source_reference = fallback_sources.get(&source).cloned().or_else(|| {
-                log::error!("Failed to find a source reference for source text: '{source}'");
-                None
-            });
-            (None, source_reference)
-        },
+    let path = match source {
+        FrameSource::File(path) => Some(path),
+        FrameSource::Text(source) => fallback_sources.get(&source).cloned().or_else(|| {
+            log::error!("Failed to find a source reference for source text: '{source}'");
+            None
+        }),
     };
 
     let src = Source {
         name: Some(source_name),
         path,
-        source_reference,
+        source_reference: None,
         presentation_hint: None,
         origin: None,
         sources: None,