Repository: nushell/nushell (language: rust)

Issue:
The span of `Keyword` does not include its subsidiary expression
### Question

https://github.com/nushell/nushell/blob/23dc1b600a0e09279cef75f0dbf47505c37edeca/crates/nu-protocol/src/ast/keyword.rs#L6-L10

It's kind of weird that the Keyword expression contains at subsidiary expression (like an else block) in it, however the the span of the whole `Keyword` expression only covers the actual keyword syntax string.

https://github.com/nushell/nushell/blob/23dc1b600a0e09279cef75f0dbf47505c37edeca/crates/nu-parser/src/parser.rs#L913-L917

I don't know if this is designed on purpose or not, but it costs some extra attention dealing with those spans. 



Discussion / hints:
Doesn't `expr` have an `expr.span`? You maybe able to use `Span::concat()` to combine them. Or maybe I'm missing your point?
I mean the `Keyword` should be considered as a parent node? If so, a parent node's span should always cover all of its children, right?
I'm sorry, the phrasing should be, for an expression

```rust
Expression {expr: Expr::Keyword(Keyword {keyword: xxx, span: span_a, expr: expr_a} ), span: span_b}
```
where the `span_b` should cover `span_a` as well as the span of `expr_a` imo, however it's not the case,
`span_b` is the same as `span_a` which only covers the keyword string.

I'm referencing the wrong code snippet here, plz ignore that.

Fix patch:
diff --git a/crates/nu-lsp/src/ast.rs b/crates/nu-lsp/src/ast.rs
index 21c6ddc71d724..33e88d2d639af 100644
--- a/crates/nu-lsp/src/ast.rs
+++ b/crates/nu-lsp/src/ast.rs
@@ -444,13 +444,6 @@ fn find_id_in_expr(
 ) -> Option<Vec<(Id, Span)>> {
     // skip the entire expression if the location is not in it
     if !expr.span.contains(*location) {
-        // TODO: the span of Keyword does not include its subsidiary expression
-        // resort to `expr_flat_map` if location found in its expr
-        if let Expr::Keyword(kw) = &expr.expr {
-            if kw.expr.span.contains(*location) {
-                return None;
-            }
-        }
         return Some(Vec::new());
     }
     let span = expr.span;
diff --git a/crates/nu-lsp/src/goto.rs b/crates/nu-lsp/src/goto.rs
index ef1bcb3101536..1c761e4f82724 100644
--- a/crates/nu-lsp/src/goto.rs
+++ b/crates/nu-lsp/src/goto.rs
@@ -475,6 +475,24 @@ mod tests {
             })
         );
 
+        let resp = send_goto_definition_request(&client_connection, script.clone(), 1, 25);
+        let result = if let Message::Response(response) = resp {
+            response.result
+        } else {
+            panic!()
+        };
+
+        assert_json_include!(
+            actual: result,
+            expected: serde_json::json!({
+                "uri": script.to_string().replace("use_module", "module"),
+                "range": {
+                    "start": { "line": 0, "character": 0 },
+                    "end": { "line": 3 }
+                }
+            })
+        );
+
         let resp = send_goto_definition_request(&client_connection, script.clone(), 2, 30);
         let result = if let Message::Response(response) = resp {
             response.result
diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index ceb1b3d129e7c..8744469f5d9bc 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -919,7 +919,7 @@ pub fn parse_multispan_value(
             Expression::new(
                 working_set,
                 Expr::Keyword(Box::new(keyword.clone())),
-                arg_span,
+                keyword.span.merge(keyword.expr.span),
                 keyword.expr.ty,
             )
         }


Test patch: