buck2_error -> lsp diagnostics

.buckconfig

@@ -15,7 +15,8 @@ ignore = \
     app_dep_graph_rules, \
     examples, \
     integrations/rust-project/tests, \
-    tests
+    tests, \
+    target
 
 [rust]
 default_edition = 2024

app/buck2_error/src/any.rs

@@ -36,6 +36,7 @@ fn maybe_add_context_from_metadata(mut e: crate::Error, context: &dyn StdError)
     }
 }
 
+/// Walk a StdError (e.g. anyhow::Error) down to its source, and rebuild it as a [struct@crate::Error].
 pub fn recover_crate_error(
     value: &'_ (dyn StdError + 'static),
     source_location: SourceLocation,
@@ -81,6 +82,7 @@ pub fn recover_crate_error(
     // context that is not included in the `base` error yet.
     let mut e = base;
     for context_value in context_stack.into_iter().rev() {
+        // XXX: this seems sus. If the downcast succeeds, we just write N copies of {starlark_err}.
         if let Some(starlark_err) = cur.downcast_ref::<crate::starlark_error::BuckStarlarkError>() {
             e = e.context(format!("{starlark_err}"));
         } else {

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;
 
@@ -76,6 +77,12 @@ impl std::fmt::Display for ContextValue {
     }
 }
 
+impl std::fmt::Debug for ContextValue {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        std::fmt::Display::fmt(&self, f)
+    }
+}
+
 impl From<String> for ContextValue {
     fn from(value: String) -> Self {
         ContextValue::Dyn(value.into())
@@ -106,29 +113,67 @@ 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,
+    /// May be empty when we try to inject some StarlarkContext and there is already a StarlarkContext
+    /// near the top of the error.
     pub error_msg: String,
     pub span: Option<FileSpan>,
+    /// If true, we render the span / call stack in buck output.
+    /// Otherwise, data is only for LSP to fish out.
+    pub show_span_in_buck_output: bool,
+    ///
+    /// We don't render the root error if `replaces_root_error` is set.
+    /// We render this in preference because it has a starlark
+    /// span and call stack.
+    ///
+    /// This is only to be set if error_msg contains a rendering of the root error.
+    ///
+    /// This field enables us to keep an actual StarlarkContext tha can be rendered
+    /// as an LSP span instead of literally replacing the root context with a
+    /// stringified span.
+    ///
+    pub replaces_root_error: bool,
 }
 
 impl StarlarkContext {
-    pub fn concat(&self, other: Option<Self>) -> Self {
-        if let Some(ctx) = other {
-            let trimmed = ctx
+    /// Concatenate call stacks. Keeps self.error_msg, drops inner.error_msg.
+    ///
+    /// Pass the inner context as the argument, i.e. the one closer to the root cause.
+    pub fn concat(&self, inner: Option<Self>) -> Self {
+        if let Some(inner) = inner {
+            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(inner.call_stack.frames.iter())
+                .cloned()
+                .collect();
+
             Self {
-                call_stack,
-                error_msg: ctx.error_msg.clone(),
-                span: ctx.span.clone(),
+                call_stack: CallStack { frames },
+                error_msg: self.error_msg.clone(),
+                span: inner.span.clone(),
+                replaces_root_error: inner.replaces_root_error,
+                show_span_in_buck_output: true,
             }
         } else {
             self.clone()
         }
     }
+
+    pub fn find_span_in_file(&self, filename: &str) -> Option<FileSpan> {
+        self.span
+            .as_ref()
+            .filter(|x| x.filename() == filename)
+            .or_else(|| {
+                self.call_stack
+                    .frames
+                    .iter()
+                    .filter_map(|f| f.location.as_ref())
+                    .find(|span| span.filename() == filename)
+            })
+            .cloned()
+    }
 }
 
 impl std::fmt::Display for StarlarkContext {

app/buck2_error/src/error.rs

@@ -52,9 +52,14 @@ pub struct Error(pub(crate) Arc<ErrorKind>);
 /// Right now, this type can represent an error root, together with a stack of context information.
 #[derive(allocative::Allocative)]
 pub(crate) enum ErrorKind {
+    /// The originating error, before context is added
     Root(Box<ErrorRoot>),
-    /// For now we use untyped context to maximize compatibility with anyhow.
+    /// The output of [`e.context(...)`][Error::context]; recursively contains Error.
+    /// There is eventually a [Self::Root] at the bottom.
     WithContext(ContextValue, Error),
+    /// The output of [`e.mark_emitted()`][Error::mark_emitted]; recursively contains Error.
+    /// There is eventually a [Self::Root] at the bottom.
+    ///
     /// Indicates that the error has been emitted, ie shown to the user.
     // This `Arc` should ideally be a `Box`. However, that doesn't work right now because of the
     // implementation of `into_anyhow_for_format`.
@@ -262,6 +267,16 @@ impl Error {
         })
     }
 
+    /// Useful to check if StarlarkContext got injected properly
+    #[cfg(test)]
+    #[allow(unused)]
+    pub(crate) fn find_starlark_context(&self) -> Option<&StarlarkContext> {
+        self.iter_context().find_map(|kind| match kind {
+            ContextValue::StarlarkError(sc) => Some(sc),
+            _ => None,
+        })
+    }
+
     pub fn string_tags(&self) -> Vec<String> {
         let mut tags: Vec<String> = self
             .iter_context()

app/buck2_error/src/format.rs

@@ -59,11 +59,34 @@ pub(crate) fn into_anyhow_for_format(
     let mut out: anyhow::Error = base.into();
     for context in context_stack.into_iter().rev() {
         if let ContextValue::StarlarkError(ctx) = context {
-            // Because context_stack is reversed, the right ordering would be first error last to preserve stack ordering
-            starlark_error = Some(ctx.concat(starlark_error));
+            if ctx.replaces_root_error {
+                // ignore the root error, treat this as the root
+                out = AnyhowWrapperForFormat::Root(ctx.to_string()).into();
+            } else if !ctx.show_span_in_buck_output {
+                out = out.context(ctx.error_msg.clone())
+            } else {
+                // Because context_stack is reversed, the right ordering is first error
+                // last to preserve stack ordering.
+                // Inner's error message are dropped in concat. But if there is one, we add it as
+                // context first.
+                let mut outer = ctx.clone();
+                if let Some(inner) = &mut starlark_error {
+                    // Inner is getting concatenated to outer. Concat will drop its message, so
+                    // preserve it in one way or another.
+                    //
+                    // Error messages may be empty when inject_starlark_context finds another StarlarkContext.
+                    let inner_message = std::mem::take(&mut inner.error_msg);
+                    if outer.error_msg.is_empty() {
+                        outer.error_msg = inner_message;
+                    } else if !inner_message.is_empty() {
+                        out = out.context(inner_message);
+                    }
+                }
+                starlark_error = Some(outer.concat(starlark_error));
+            }
             continue;
         }
-        if let Some(ctx) = starlark_error {
+        if let Some(ref ctx) = starlark_error {
             out = out.context(format!("{ctx}"));
             starlark_error = None;
         }

app/buck2_error/src/lib.rs

@@ -21,6 +21,7 @@ mod derive_tests;
 mod error;
 mod exit_code;
 mod format;
+pub mod lsp;
 pub mod macros;
 mod root;
 pub mod source_location;