buck2_error -> lsp diagnostics

app/buck2_error/src/context_value.rs

@@ -12,6 +12,7 @@ use std::fmt;
 use std::sync::Arc;
 
 use smallvec::SmallVec;
+use starlark_syntax::call_stack::CallStack;
 use starlark_syntax::codemap::FileSpan;
 use starlark_syntax::span_display::span_display;
 
@@ -106,22 +107,24 @@ impl<T: TypedContext> From<T> for ContextValue {
 
 #[derive(Clone, allocative::Allocative, Debug, PartialEq, Eq, Hash)]
 pub struct StarlarkContext {
-    // TODO(minglunli): We could take in the CallStack type and do some magic to make it look nicer
-    // but I think just concatenating the string form and it's accurate and look good enough
-    pub call_stack: String,
+    pub call_stack: CallStack,
     pub error_msg: String,
     pub span: Option<FileSpan>,
 }
 
 impl StarlarkContext {
     pub fn concat(&self, other: Option<Self>) -> Self {
         if let Some(ctx) = other {
-            let trimmed = ctx
+            let frames = self
                 .call_stack
-                .trim_start_matches(starlark_syntax::call_stack::CALL_STACK_TRACEBACK_PREFIX);
-            let call_stack = format!("{}{}", self.call_stack, trimmed);
+                .frames
+                .iter()
+                .chain(ctx.call_stack.frames.iter())
+                .cloned()
+                .collect();
+
             Self {
-                call_stack,
+                call_stack: CallStack { frames },
                 error_msg: ctx.error_msg.clone(),
                 span: ctx.span.clone(),
             }

app/buck2_error/src/starlark_error.rs

@@ -118,7 +118,7 @@ fn from_starlark_impl(
 ) -> crate::Error {
     let starlark_context = if e.has_diagnostic() && !skip_stacktrace {
         Some(StarlarkContext {
-            call_stack: format!("{}", e.call_stack()),
+            call_stack: e.call_stack().clone(),
             error_msg: format!("{}", e.without_diagnostic()),
             span: e.span().cloned(),
         })
@@ -232,6 +232,8 @@ mod tests {
     use std::sync::Arc;
 
     use allocative::Allocative;
+    use starlark_syntax::call_stack::CallStack;
+    use starlark_syntax::frame::Frame;
 
     use crate::any::ProvidableMetadata;
     use crate::buck2_error;
@@ -272,6 +274,15 @@ mod tests {
         }
     }
 
+    fn example_call_stack() -> CallStack {
+        CallStack {
+            frames: vec![Frame {
+                name: "frame".to_owned(),
+                location: None,
+            }],
+        }
+    }
+
     #[test]
     fn test_roundtrip_starlark() {
         // Tests buck2_error->starlark->buck2_error
@@ -312,7 +323,7 @@ mod tests {
         let context_key = "Some context key";
         let error_tag = crate::ErrorTag::IoWindowsSharingViolation;
         let starlark_context = StarlarkContext {
-            call_stack: "Some call stack".to_owned(),
+            call_stack: example_call_stack(),
             error_msg: "Some error message".to_owned(),
             span: None,
         };
@@ -333,9 +344,9 @@ mod tests {
     fn test_pop_base_error_from_context() {
         let base_error = "Some base error";
         let starlark_error = "Starlark error message";
-        let starlark_call_stack = "Some call stack";
+        let starlark_call_stack = example_call_stack();
         let starlark_context = StarlarkContext {
-            call_stack: starlark_call_stack.to_owned(),
+            call_stack: starlark_call_stack.clone(),
             error_msg: starlark_error.to_owned(),
             span: None,
         };
@@ -345,7 +356,11 @@ mod tests {
         let base_replaced = error_with_starlark_context(&e, starlark_context);
 
         assert!(!base_replaced.to_string().contains(base_error));
-        assert!(base_replaced.to_string().contains(starlark_call_stack));
+        assert!(
+            base_replaced
+                .to_string()
+                .contains(&starlark_call_stack.to_string())
+        );
         assert!(base_replaced.to_string().contains(starlark_error));
     }
 
@@ -358,18 +373,18 @@ mod tests {
             || FullMetadataError,
         );
 
-        let starlark_call_stack = "Some call stack";
+        let starlark_call_stack = example_call_stack();
         let starlark_error_msg = "Starlark error message";
         let starlark_context = StarlarkContext {
-            call_stack: starlark_call_stack.to_owned(),
+            call_stack: starlark_call_stack.clone(),
             error_msg: starlark_error_msg.to_owned(),
             span: None,
         };
 
         let starlark_err = error_with_starlark_context(&e, starlark_context);
         let starlark_err_string = format!("{starlark_err:#}");
 
-        assert!(starlark_err_string.contains(starlark_call_stack));
+        assert!(starlark_err_string.contains(&starlark_call_stack.to_string()));
         assert!(starlark_err_string.contains(starlark_error_msg));
         // Root error shouldn't be lost
         assert!(starlark_err_string.contains(base_error));

app/buck2_error_tests/src/golden/test_starlark_callstack_backtrace.golden

@@ -6,8 +6,7 @@ Traceback (most recent call last):
       outer_fail()
   * outer_import.bzl:4, in outer_fail
       outer_rust_failure()
-
-  * assert.bzl:3, in <module>
+  * assert.bzl:3, in outer_rust_failure
       should_fail()
   * imported.bzl:9, in should_fail
       rust_failure()

starlark-rust/starlark_syntax/src/call_stack.rs

@@ -28,12 +28,14 @@ use std::fmt;
 use std::fmt::Debug;
 use std::fmt::Display;
 
+use allocative::Allocative;
+
 use crate::frame::Frame;
 
 pub const CALL_STACK_TRACEBACK_PREFIX: &str = "Traceback (most recent call last):";
 
 /// Owned call stack.
-#[derive(Debug, Default, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Default, Clone, PartialEq, Eq, Hash, Allocative)]
 pub struct CallStack {
     /// The frames.
     pub frames: Vec<Frame>,

starlark-rust/starlark_syntax/src/frame.rs

@@ -19,12 +19,14 @@ use std::fmt;
 use std::fmt::Display;
 use std::fmt::Formatter;
 
+use allocative::Allocative;
+
 use crate::codemap::FileSpan;
 use crate::fast_string;
 use crate::fast_string::CharIndex;
 
 /// A frame of the call-stack.
-#[derive(Debug, PartialEq, Eq, Hash, Clone)]
+#[derive(Debug, PartialEq, Eq, Hash, Clone, Allocative)]
 pub struct Frame {
     /// The name of the entry on the call-stack.
     pub name: String,