Various hover improvements (#5617)

* Show more info on hover for variables

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Move hover impls to lsp module

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Make hover work on names inside calls, fix doc line breaking, trim docs in tool tips

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Test the new hovers; fix signature syntax

Signed-off-by: Nick Cameron <nrc@ncameron.org>

* Hover tips for kwargs

Signed-off-by: Nick Cameron <nrc@ncameron.org>

---------

Signed-off-by: Nick Cameron <nrc@ncameron.org>
This commit is contained in:
Nick Cameron
2025-03-04 22:53:31 +13:00
committed by GitHub
parent 6e57a80c13
commit df278c7e6a
200 changed files with 7888 additions and 4459 deletions

View File

@ -225,10 +225,10 @@ fn create_start_sketch_on(
"line",
None,
vec![LabeledArg {
label: super::types::Identifier {
label: Node::no_src(super::types::Identifier {
name: "end".to_owned(),
digest: None,
},
}),
arg: expr,
}],
)?;
@ -264,10 +264,10 @@ fn create_start_sketch_on(
None,
vec![LabeledArg {
arg: expr,
label: Identifier {
label: Node::no_src(Identifier {
name: "end".to_owned(),
digest: None,
},
}),
}],
)?;
pipe_body.push(line.into());

View File

@ -1,7 +1,7 @@
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use super::{BoxNode, ConstraintLevel, Digest, Expr, Hover, Node, NodeList};
use super::{BoxNode, ConstraintLevel, Digest, Expr, Node, NodeList};
use crate::SourceRange;
// TODO: This should be its own type, similar to Program,
@ -58,18 +58,6 @@ impl Node<IfExpression> {
}
impl IfExpression {
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
self.cond
.get_hover_value_for_position(pos, code)
.or_else(|| self.then_val.get_hover_value_for_position(pos, code))
.or_else(|| {
self.else_ifs
.iter()
.find_map(|else_if| else_if.get_hover_value_for_position(pos, code))
})
.or_else(|| self.final_else.get_hover_value_for_position(pos, code))
}
/// Rename all identifiers that have the old name to the new given name.
pub fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.cond.rename_identifiers(old_name, new_name);
@ -89,11 +77,6 @@ impl IfExpression {
}
impl ElseIf {
fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
self.cond
.get_hover_value_for_position(pos, code)
.or_else(|| self.then_val.get_hover_value_for_position(pos, code))
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.cond.rename_identifiers(old_name, new_name);

View File

@ -14,7 +14,7 @@ use parse_display::{Display, FromStr};
use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use tower_lsp::lsp_types::{
CompletionItem, CompletionItemKind, DocumentSymbol, FoldingRange, FoldingRangeKind, Range as LspRange, SymbolKind,
CompletionItem, CompletionItemKind, DocumentSymbol, FoldingRange, FoldingRangeKind, SymbolKind,
};
pub use crate::parsing::ast::types::{
@ -309,23 +309,6 @@ impl Program {
matches!(last, BodyItem::ExpressionStatement(_))
}
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
// Check if we are in shebang.
if let Some(node) = &self.shebang {
if node.contains(pos) {
let source_range: SourceRange = node.into();
return Some(Hover::Comment {
value: r#"The `#!` at the start of a script, known as a shebang, specifies the path to the interpreter that should execute the script. This line is not necessary for your `kcl` to run in the modeling-app. You can safely delete it. If you wish to learn more about what you _can_ do with a shebang, read this doc: [zoo.dev/docs/faq/shebang](https://zoo.dev/docs/faq/shebang)."#.to_string(),
range: source_range.to_lsp_range(code),
});
}
}
let value = self.get_expr_for_position(pos)?;
value.get_hover_value_for_position(pos, code)
}
/// Returns the body item that includes the given character position.
pub fn get_body_item_for_position(&self, pos: usize) -> Option<&BodyItem> {
for item in &self.body {
@ -798,37 +781,6 @@ impl Expr {
}
}
/// Returns a hover value that includes the given character position.
/// This is really recursive so keep that in mind.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
match self {
Expr::BinaryExpression(binary_expression) => binary_expression.get_hover_value_for_position(pos, code),
Expr::FunctionExpression(function_expression) => {
function_expression.get_hover_value_for_position(pos, code)
}
Expr::CallExpression(call_expression) => call_expression.get_hover_value_for_position(pos, code),
Expr::CallExpressionKw(call_expression) => call_expression.get_hover_value_for_position(pos, code),
Expr::PipeExpression(pipe_expression) => pipe_expression.get_hover_value_for_position(pos, code),
Expr::ArrayExpression(array_expression) => array_expression.get_hover_value_for_position(pos, code),
Expr::ArrayRangeExpression(array_range) => array_range.get_hover_value_for_position(pos, code),
Expr::ObjectExpression(object_expression) => object_expression.get_hover_value_for_position(pos, code),
Expr::MemberExpression(member_expression) => member_expression.get_hover_value_for_position(pos, code),
Expr::UnaryExpression(unary_expression) => unary_expression.get_hover_value_for_position(pos, code),
Expr::IfExpression(expr) => expr.get_hover_value_for_position(pos, code),
// TODO: LSP hover information for values/types. https://github.com/KittyCAD/modeling-app/issues/1126
Expr::None(_) => None,
Expr::Literal(_) => None,
Expr::Identifier(_) => None,
Expr::TagDeclarator(_) => None,
// TODO LSP hover info for tag
Expr::LabelledExpression(expr) => expr.expr.get_hover_value_for_position(pos, code),
// TODO LSP hover info for type
Expr::AscribedExpression(expr) => expr.expr.get_hover_value_for_position(pos, code),
// TODO: LSP hover information for symbols. https://github.com/KittyCAD/modeling-app/issues/1127
Expr::PipeSubstitution(_) => None,
}
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
match self {
@ -1114,24 +1066,6 @@ impl BinaryPart {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
match self {
BinaryPart::Literal(_literal) => None,
BinaryPart::Identifier(_identifier) => None,
BinaryPart::BinaryExpression(binary_expression) => {
binary_expression.get_hover_value_for_position(pos, code)
}
BinaryPart::CallExpression(call_expression) => call_expression.get_hover_value_for_position(pos, code),
BinaryPart::CallExpressionKw(call_expression) => call_expression.get_hover_value_for_position(pos, code),
BinaryPart::UnaryExpression(unary_expression) => unary_expression.get_hover_value_for_position(pos, code),
BinaryPart::IfExpression(e) => e.get_hover_value_for_position(pos, code),
BinaryPart::MemberExpression(member_expression) => {
member_expression.get_hover_value_for_position(pos, code)
}
}
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
match self {
@ -1631,7 +1565,7 @@ pub struct CallExpressionKw {
#[ts(export)]
#[serde(tag = "type")]
pub struct LabeledArg {
pub label: Identifier,
pub label: Node<Identifier>,
pub arg: Expr,
}
@ -1707,30 +1641,6 @@ impl CallExpression {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let callee_source_range: SourceRange = self.callee.clone().into();
if callee_source_range.contains(pos) {
return Some(Hover::Function {
name: self.callee.name.clone(),
range: callee_source_range.to_lsp_range(code),
});
}
for (index, arg) in self.arguments.iter().enumerate() {
let source_range: SourceRange = arg.into();
if source_range.contains(pos) {
return Some(Hover::Signature {
name: self.callee.name.clone(),
parameter_index: index as u32,
range: source_range.to_lsp_range(code),
});
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.callee.rename(old_name, new_name);
@ -1753,8 +1663,11 @@ impl CallExpressionKw {
}
/// Iterate over all arguments (labeled or not)
pub fn iter_arguments(&self) -> impl Iterator<Item = &Expr> {
self.unlabeled.iter().chain(self.arguments.iter().map(|arg| &arg.arg))
pub fn iter_arguments(&self) -> impl Iterator<Item = (Option<&Node<Identifier>>, &Expr)> {
self.unlabeled
.iter()
.map(|e| (None, e))
.chain(self.arguments.iter().map(|arg| (Some(&arg.label), &arg.arg)))
}
/// Is at least one argument the '%' i.e. the substitution operator?
@ -1770,30 +1683,6 @@ impl CallExpressionKw {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let callee_source_range: SourceRange = self.callee.clone().into();
if callee_source_range.contains(pos) {
return Some(Hover::Function {
name: self.callee.name.clone(),
range: callee_source_range.to_lsp_range(code),
});
}
for (index, arg) in self.iter_arguments().enumerate() {
let source_range: SourceRange = arg.into();
if source_range.contains(pos) {
return Some(Hover::Signature {
name: self.callee.name.clone(),
parameter_index: index as u32,
range: source_range.to_lsp_range(code),
});
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.callee.rename(old_name, new_name);
@ -2340,18 +2229,6 @@ impl ArrayExpression {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
for element in &self.elements {
let element_source_range: SourceRange = element.into();
if element_source_range.contains(pos) {
return element.get_hover_value_for_position(pos, code);
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
for element in &mut self.elements {
@ -2405,18 +2282,6 @@ impl ArrayRangeExpression {
self.end_element.replace_value(source_range, new_value.clone());
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
for element in [&self.start_element, &self.end_element] {
let element_source_range: SourceRange = element.into();
if element_source_range.contains(pos) {
return element.get_hover_value_for_position(pos, code);
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.start_element.rename_identifiers(old_name, new_name);
@ -2469,18 +2334,6 @@ impl ObjectExpression {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
for property in &self.properties {
let property_source_range: SourceRange = property.into();
if property_source_range.contains(pos) {
return property.get_hover_value_for_position(pos, code);
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
for property in &mut self.properties {
@ -2529,16 +2382,6 @@ impl ObjectProperty {
digest: None,
})
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let value_source_range: SourceRange = self.value.clone().into();
if value_source_range.contains(pos) {
return self.value.get_hover_value_for_position(pos, code);
}
None
}
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
@ -2550,16 +2393,6 @@ pub enum MemberObject {
}
impl MemberObject {
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
match self {
MemberObject::MemberExpression(member_expression) => {
member_expression.get_hover_value_for_position(pos, code)
}
MemberObject::Identifier(_identifier) => None,
}
}
pub fn start(&self) -> usize {
match self {
MemberObject::MemberExpression(member_expression) => member_expression.start,
@ -2647,16 +2480,6 @@ impl Node<MemberExpression> {
}
impl MemberExpression {
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let object_source_range: SourceRange = self.object.clone().into();
if object_source_range.contains(pos) {
return self.object.get_hover_value_for_position(pos, code);
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
match &mut self.object {
@ -2725,22 +2548,6 @@ impl BinaryExpression {
self.operator.precedence()
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let left_source_range: SourceRange = self.left.clone().into();
let right_source_range: SourceRange = self.right.clone().into();
if left_source_range.contains(pos) {
return self.left.get_hover_value_for_position(pos, code);
}
if right_source_range.contains(pos) {
return self.right.get_hover_value_for_position(pos, code);
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.left.rename_identifiers(old_name, new_name);
@ -2902,16 +2709,6 @@ impl UnaryExpression {
self.argument.get_constraint_level()
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
let argument_source_range: SourceRange = self.argument.clone().into();
if argument_source_range.contains(pos) {
return self.argument.get_hover_value_for_position(pos, code);
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
self.argument.rename_identifiers(old_name, new_name);
@ -2996,18 +2793,6 @@ impl PipeExpression {
}
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
for b in &self.body {
let b_source_range: SourceRange = b.into();
if b_source_range.contains(pos) {
return b.get_hover_value_for_position(pos, code);
}
}
None
}
/// Rename all identifiers that have the old name to the new given name.
fn rename_identifiers(&mut self, old_name: &str, new_name: &str) {
for statement in &mut self.body {
@ -3275,13 +3060,31 @@ impl FunctionExpression {
self.body.replace_value(source_range, new_value);
}
/// Returns a hover value that includes the given character position.
pub fn get_hover_value_for_position(&self, pos: usize, code: &str) -> Option<Hover> {
if let Some(value) = self.body.get_expr_for_position(pos) {
return value.get_hover_value_for_position(pos, code);
pub fn signature(&self) -> String {
let mut signature = String::new();
if self.params.is_empty() {
signature.push_str("()");
} else if self.params.len() == 1 {
signature.push('(');
signature.push_str(&self.params[0].recast(&FormatOptions::default(), 0));
signature.push(')');
} else {
signature.push('(');
for a in &self.params {
signature.push_str("\n ");
signature.push_str(&a.recast(&FormatOptions::default(), 0));
signature.push(',');
}
signature.push('\n');
signature.push(')');
}
None
if let Some(ty) = &self.return_type {
signature.push_str(&format!(": {ty}"));
}
signature
}
#[cfg(test)]
@ -3311,25 +3114,6 @@ pub struct ReturnStatement {
pub digest: Option<Digest>,
}
/// Describes information about a hover.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
#[serde(rename_all = "camelCase")]
pub enum Hover {
Function {
name: String,
range: LspRange,
},
Signature {
name: String,
parameter_index: u32,
range: LspRange,
},
Comment {
value: String,
range: LspRange,
},
}
/// Format options.
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
#[ts(export)]

View File

@ -2498,10 +2498,7 @@ fn labeled_argument(i: &mut TokenSlice) -> PResult<LabeledArg> {
terminated(one_of((TokenType::Operator, "=")), opt(whitespace)),
expression,
)
.map(|(label, arg)| LabeledArg {
label: label.inner,
arg,
})
.map(|(label, arg)| LabeledArg { label, arg })
.parse_next(i)
}

View File

@ -255,8 +255,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "length"
"end": 166,
"name": "length",
"start": 160,
"type": "Identifier"
},
"arg": {
"end": 169,

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -11,8 +11,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 16,
"name": "endAbsolute",
"start": 5,
"type": "Identifier"
},
"arg": {
"elements": [

View File

@ -91,8 +91,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 92,
"name": "endAbsolute",
"start": 81,
"type": "Identifier"
},
"arg": {
"elements": [
@ -128,8 +130,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "tag"
"end": 106,
"name": "tag",
"start": 103,
"type": "Identifier"
},
"arg": {
"end": 116,
@ -157,8 +161,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 145,
"name": "endAbsolute",
"start": 134,
"type": "Identifier"
},
"arg": {
"elements": [
@ -209,8 +215,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 183,
"name": "endAbsolute",
"start": 172,
"type": "Identifier"
},
"arg": {
"elements": [
@ -246,8 +254,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "tag"
"end": 197,
"name": "tag",
"start": 194,
"type": "Identifier"
},
"arg": {
"end": 210,

View File

@ -91,8 +91,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 76,
"name": "endAbsolute",
"start": 65,
"type": "Identifier"
},
"arg": {
"elements": [

View File

@ -68,8 +68,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "end"
"end": 61,
"name": "end",
"start": 58,
"type": "Identifier"
},
"arg": {
"elements": [

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -11,8 +11,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "endAbsolute"
"end": 16,
"name": "endAbsolute",
"start": 5,
"type": "Identifier"
},
"arg": {
"elements": [

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -42,8 +42,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "center"
"end": 57,
"name": "center",
"start": 51,
"type": "Identifier"
},
"arg": {
"elements": [
@ -79,8 +81,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "radius"
"end": 73,
"name": "radius",
"start": 67,
"type": "Identifier"
},
"arg": {
"end": 77,
@ -112,8 +116,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "length"
"end": 100,
"name": "length",
"start": 94,
"type": "Identifier"
},
"arg": {
"end": 105,

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -18,8 +18,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "x"
"end": 11,
"name": "x",
"start": 10,
"type": "Identifier"
},
"arg": {
"end": 15,
@ -32,8 +34,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "y"
"end": 18,
"name": "y",
"start": 17,
"type": "Identifier"
},
"arg": {
"end": 22,

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -31,8 +31,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "arg"
"end": 16,
"name": "arg",
"start": 13,
"type": "Identifier"
},
"arg": {
"end": 20,

View File

@ -1,7 +1,6 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
snapshot_kind: text
---
{
"body": [
@ -19,8 +18,10 @@ snapshot_kind: text
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "arg"
"end": 25,
"name": "arg",
"start": 22,
"type": "Identifier"
},
"arg": {
"end": 29,
@ -33,8 +34,10 @@ snapshot_kind: text
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "foo"
"end": 47,
"name": "foo",
"start": 44,
"type": "Identifier"
},
"arg": {
"end": 51,
@ -47,8 +50,10 @@ snapshot_kind: text
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "bar"
"end": 69,
"name": "bar",
"start": 66,
"type": "Identifier"
},
"arg": {
"end": 73,

View File

@ -1,7 +1,6 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
snapshot_kind: text
---
{
"body": [
@ -19,8 +18,10 @@ snapshot_kind: text
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "arg"
"end": 25,
"name": "arg",
"start": 22,
"type": "Identifier"
},
"arg": {
"end": 29,
@ -33,8 +34,10 @@ snapshot_kind: text
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "bar"
"end": 72,
"name": "bar",
"start": 69,
"type": "Identifier"
},
"arg": {
"end": 76,

View File

@ -1,5 +1,5 @@
---
source: kcl/src/parsing/parser.rs
source: kcl-lib/src/parsing/parser.rs
expression: actual
---
{
@ -18,8 +18,10 @@ expression: actual
{
"type": "LabeledArg",
"label": {
"type": "Identifier",
"name": "y"
"end": 14,
"name": "y",
"start": 13,
"type": "Identifier"
},
"arg": {
"end": 18,