Make "evaluate" request support structured variables

Summary:
This diff updates handling of "evaluate" request based on changes in Starlark api. All heavy weight work is done by variables request handling done in D50312131. Here we just register a value access path after evaluating expression. The downside of this approach is that expression is re-evaluated for each subsequent subvalue. I'm not sure if there is any caching in the evaluator maybe this is not an issue.

## Context
I've noticed expressions are hardcoding `value.to_string()`. The way I'm planning to support this is by introducing
`scope: Expr(String) | Local(String)` instead of name to VariablePath. "inspect_variable" will be able to 1) evaluate scope 2) evaluate access path. And everything stays pretty much the same it is right now

Probably a better implementation would be based on unique identification for Value<'v>. This will require some API in Starlark that would fetch Value<'v> based on id. I don't really know much about Starlark internals to implement this now. However pivoting to this solution is effectively changing "scope" to "id" and the rest is pretty much stays the same

Reviewed By: stepancheg

Differential Revision: D50396039

fbshipit-source-id: bc2b846fd2f553c2142c54ebd5d10de6f4096f4c

starlark/src/debug/adapter/implementation.rs

@@ -29,6 +29,7 @@ use debugserver_types::*;
 use dupe::Dupe;
 use starlark_syntax::slice_vec_ext::SliceExt;
 
+use super::EvaluateExprInfo;
 use super::InspectVariableInfo;
 use super::PathSegment;
 use super::VariablePath;
@@ -321,6 +322,7 @@ impl DapAdapter for DapAdapterImpl {
 
     fn inspect_variable(&self, path: VariablePath) -> anyhow::Result<InspectVariableInfo> {
         let path = path;
+        let state = self.state.dupe();
         self.with_ctx(Box::new(move |_span, eval| {
             let access_path = &path.access_path;
             let mut value = match &path.scope {
@@ -332,7 +334,7 @@ impl DapAdapter for DapAdapterImpl {
                         anyhow::Error::msg(format!("Local variable {} not found", name))
                     })
                 }
-                super::Scope::Expr(_expr) => panic!("Not implemented yet"),
+                super::Scope::Expr(expr) => evaluate_expr(&state, eval, expr.to_owned()),
             }?;
 
             for p in access_path.iter() {
@@ -352,22 +354,14 @@ impl DapAdapter for DapAdapterImpl {
         Ok(())
     }
 
-    fn evaluate(&self, expr: &str) -> anyhow::Result<EvaluateResponseBody> {
+    fn evaluate(&self, expr: &str) -> anyhow::Result<EvaluateExprInfo> {
         let state = self.state.dupe();
         let expression = expr.to_owned();
         self.with_ctx(Box::new(move |_, eval| {
-            let s = match evaluate_expr(&state, eval, expression.clone()) {
-                Err(e) => format!("{:#}", e),
-                Ok(v) => v.to_string(),
-            };
-            Ok(EvaluateResponseBody {
-                indexed_variables: None,
-                named_variables: None,
-                presentation_hint: None,
-                result: s,
-                type_: None,
-                variables_reference: 0.0,
-            })
+            match evaluate_expr(&state, eval, expression.clone()) {
+                Err(e) => Err(e),
+                Ok(v) => Ok(EvaluateExprInfo::from_value(&v)),
+            }
         }))
     }
 }

starlark/src/debug/adapter/mod.rs

@@ -86,7 +86,15 @@ pub struct VariablePath {
 }
 
 impl VariablePath {
-    /// creates new instance of VariablePath
+    /// creates new instance of VariablePath from a given expression
+    pub fn new_expression(expr: impl Into<String>) -> VariablePath {
+        VariablePath {
+            scope: Scope::Expr(expr.into()),
+            access_path: vec![],
+        }
+    }
+
+    /// creates new instance of VariablePath from a given local variable
     pub fn new_local(scope: impl Into<String>) -> VariablePath {
         VariablePath {
             scope: Scope::Local(scope.into()),
@@ -156,12 +164,16 @@ impl Variable {
             name,
             value: v.to_str(),
             type_: v.get_type().to_owned(),
-            has_children: match v.get_type() {
-                "function" | "never" | "NoneType" | "bool" | "int" | "float" | "string" => false,
-                _ => true,
-            },
+            has_children: Self::has_children(v.get_type()),
         }
     }
+
+    pub(crate) fn has_children(value_type: &str) -> bool {
+        !matches!(
+            value_type,
+            "function" | "never" | "NoneType" | "bool" | "int" | "float" | "string"
+        )
+    }
 }
 
 /// The kind of debugger step, used for next/stepin/stepout requests.
@@ -191,6 +203,16 @@ pub struct InspectVariableInfo {
     pub sub_values: Vec<Variable>,
 }
 
+/// Information about expression evaluation result
+pub struct EvaluateExprInfo {
+    /// The value as a String.
+    pub result: String,
+    /// The variables type.
+    pub type_: String,
+    /// Indicates whether there are children available for a given variable.
+    pub has_children: bool,
+}
+
 impl InspectVariableInfo {
     fn try_from_struct_like<'v>(v: &Value<'v>, heap: &'v Heap) -> anyhow::Result<Self> {
         Ok(Self {
@@ -232,6 +254,17 @@ impl InspectVariableInfo {
     }
 }
 
+impl EvaluateExprInfo {
+    /// Creating EvaluateExprInfo from a given starlark value
+    pub fn from_value(v: &Value) -> Self {
+        Self {
+            result: v.to_str(),
+            type_: v.get_type().to_owned(),
+            has_children: Variable::has_children(v.get_type()),
+        }
+    }
+}
+
 /// The DapAdapter accepts DAP requests and updates the hooks in the running evaluator.
 pub trait DapAdapter: Debug + Send + 'static {
     /// Sets multiple breakpoints for a file (and clears existing ones).
@@ -280,7 +313,7 @@ pub trait DapAdapter: Debug + Send + 'static {
     /// Evaluates in expression in the context of the top-most frame.
     ///
     /// See <https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Evaluate>
-    fn evaluate(&self, expr: &str) -> anyhow::Result<EvaluateResponseBody>;
+    fn evaluate(&self, expr: &str) -> anyhow::Result<EvaluateExprInfo>;
 }
 
 #[derive(Debug, Clone, Hash, Eq, PartialEq)]

starlark_bin/bin/dap/mod.rs

@@ -200,7 +200,16 @@ impl DebugServer for Backend {
     }
 
     fn evaluate(&self, x: EvaluateArguments) -> anyhow::Result<EvaluateResponseBody> {
-        self.adapter.evaluate(&x.expression)
+        let expr_result = self.adapter.evaluate(&x.expression)?;
+
+        Ok(EvaluateResponseBody {
+            indexed_variables: None,
+            named_variables: None,
+            presentation_hint: None,
+            result: expr_result.result,
+            type_: Some(expr_result.type_),
+            variables_reference: 0.0,
+        })
     }
 
     fn continue_(&self, _: ContinueArguments) -> anyhow::Result<ContinueResponseBody> {