Codemirror lsp enhance (#6580)

* codemirror side Signed-off-by: Jess Frazelle <github@jessfraz.com> * codemirror actions Signed-off-by: Jess Frazelle <github@jessfraz.com> * codemirror actions Signed-off-by: Jess Frazelle <github@jessfraz.com> * code mirror now shows lint suggestions Signed-off-by: Jess Frazelle <github@jessfraz.com> * fix hanging params with test Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates for signature help Signed-off-by: Jess Frazelle <github@jessfraz.com> * fix clone Signed-off-by: Jess Frazelle <github@jessfraz.com> * add tests Signed-off-by: Jess Frazelle <github@jessfraz.com> * add tests Signed-off-by: Jess Frazelle <github@jessfraz.com> * clippy Signed-off-by: Jess Frazelle <github@jessfraz.com> * clippy Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates Signed-off-by: Jess Frazelle <github@jessfraz.com> * Update packages/codemirror-lsp-client/src/plugin/lsp.ts Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com> * z-index Signed-off-by: Jess Frazelle <github@jessfraz.com> * playwright tests Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates Signed-off-by: Jess Frazelle <github@jessfraz.com> --------- Signed-off-by: Jess Frazelle <github@jessfraz.com> Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
2025-04-29 17:57:02 -07:00
parent 844f229b5a
commit 29b8a442c2
35 changed files with 6746 additions and 80 deletions
--- a/rust/kcl-lib/src/lsp/kcl/mod.rs
+++ b/rust/kcl-lib/src/lsp/kcl/mod.rs
@ -1287,6 +1287,69 @@ impl LanguageServer for Backend {

        let pos = position_to_char_index(params.text_document_position_params.position, current_code);

+        // Get the character at the position.
+        let Some(ch) = current_code.chars().nth(pos) else {
+            return Ok(None);
+        };
+
+        let check_char = |ch: char| {
+            // If  we are on a (, then get the string in front of the (
+            // and try to get the signature.
+            // We do these before the ast check because we might not have a valid ast.
+            if ch == '(' {
+                // If the current character is not a " " then get the next space after
+                // our position so we can split on that.
+                // Find the next space after the current position.
+                let next_space = if ch != ' ' {
+                    if let Some(next_space) = current_code[pos..].find(' ') {
+                        pos + next_space
+                    } else if let Some(next_space) = current_code[pos..].find('(') {
+                        pos + next_space
+                    } else {
+                        pos
+                    }
+                } else {
+                    pos
+                };
+                let p2 = std::cmp::max(pos, next_space);
+
+                let last_word = current_code[..p2].split_whitespace().last()?;
+
+                // Get the function name.
+                return self.stdlib_signatures.get(last_word);
+            } else if ch == ',' {
+                // If we have a comma, then get the string in front of
+                // the closest ( and try to get the signature.
+
+                // Find the last ( before the comma.
+                let last_paren = current_code[..pos].rfind('(')?;
+                // Get the string in front of the (.
+                let last_word = current_code[..last_paren].split_whitespace().last()?;
+                // Get the function name.
+                return self.stdlib_signatures.get(last_word);
+            }
+
+            None
+        };
+
+        if let Some(signature) = check_char(ch) {
+            return Ok(Some(signature.clone()));
+        }
+
+        // Check if we have context.
+        if let Some(context) = params.context {
+            if let Some(character) = context.trigger_character {
+                for character in character.chars() {
+                    // Check if we are on a ( or a ,.
+                    if character == '(' || character == ',' {
+                        if let Some(signature) = check_char(character) {
+                            return Ok(Some(signature.clone()));
+                        }
+                    }
+                }
+            }
+        }
+
        // Let's iterate over the AST and find the node that contains the cursor.
        let Some(ast) = self.ast_map.get(&filename) else {
            return Ok(None);
@ -1419,7 +1482,7 @@ impl LanguageServer for Backend {
        ast.rename_symbol(&params.new_name, pos);
        // Now recast it.
        let recast = ast.recast(&Default::default(), 0);
-        let source_range = SourceRange::new(0, current_code.len() - 1, module_id);
+        let source_range = SourceRange::new(0, current_code.len(), module_id);
        let range = source_range.to_lsp_range(current_code);
        Ok(Some(WorkspaceEdit {
            changes: Some(HashMap::from([(
@ -1590,7 +1653,7 @@ fn position_to_char_index(position: Position, code: &str) -> usize {
        }
    }

-    char_position
+    std::cmp::min(char_position, code.len() - 1)
 }

 async fn with_cached_var<T>(name: &str, f: impl Fn(&KclValue) -> T) -> Option<T> {
--- a/rust/kcl-lib/src/lsp/tests.rs
+++ b/rust/kcl-lib/src/lsp/tests.rs
@ -1119,6 +1119,348 @@ async fn test_kcl_lsp_signature_help() {
    }
 }

+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_signature_help_on_parens_trigger() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    // Send open file.
+    // We do this to trigger a valid ast.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send open file.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude("
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send signature help request.
+    let signature_help = server
+        .signature_help(tower_lsp::lsp_types::SignatureHelpParams {
+            text_document_position_params: tower_lsp::lsp_types::TextDocumentPositionParams {
+                text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                    uri: "file:///test.kcl".try_into().unwrap(),
+                },
+                position: tower_lsp::lsp_types::Position { line: 9, character: 14 },
+            },
+            context: None,
+            work_done_progress_params: Default::default(),
+        })
+        .await
+        .unwrap();
+
+    // Check the signature help.
+    if let Some(signature_help) = signature_help {
+        assert_eq!(
+            signature_help.signatures.len(),
+            1,
+            "Expected one signature, got {:?}",
+            signature_help.signatures
+        );
+        assert_eq!(signature_help.signatures[0].label, "extrude");
+    } else {
+        panic!("Expected signature help");
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_signature_help_on_parens_trigger_on_before() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    // Send open file.
+    // We do this to trigger a valid ast.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send open file.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude("
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send signature help request.
+    let signature_help = server
+        .signature_help(tower_lsp::lsp_types::SignatureHelpParams {
+            text_document_position_params: tower_lsp::lsp_types::TextDocumentPositionParams {
+                text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                    uri: "file:///test.kcl".try_into().unwrap(),
+                },
+                position: tower_lsp::lsp_types::Position { line: 9, character: 10 },
+            },
+            context: Some(tower_lsp::lsp_types::SignatureHelpContext {
+                trigger_kind: tower_lsp::lsp_types::SignatureHelpTriggerKind::INVOKED,
+                trigger_character: Some("(".to_string()),
+                is_retrigger: false,
+                active_signature_help: None,
+            }),
+            work_done_progress_params: Default::default(),
+        })
+        .await
+        .unwrap();
+
+    // Check the signature help.
+    if let Some(signature_help) = signature_help {
+        assert_eq!(
+            signature_help.signatures.len(),
+            1,
+            "Expected one signature, got {:?}",
+            signature_help.signatures
+        );
+        assert_eq!(signature_help.signatures[0].label, "extrude");
+    } else {
+        panic!("Expected signature help");
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_signature_help_on_comma_trigger() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    // Send open file.
+    // We do this to trigger a valid ast.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send update file.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude(length = 10,"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send signature help request.
+    let signature_help = server
+        .signature_help(tower_lsp::lsp_types::SignatureHelpParams {
+            text_document_position_params: tower_lsp::lsp_types::TextDocumentPositionParams {
+                text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                    uri: "file:///test.kcl".try_into().unwrap(),
+                },
+                position: tower_lsp::lsp_types::Position { line: 9, character: 25 },
+            },
+            context: None,
+            work_done_progress_params: Default::default(),
+        })
+        .await
+        .unwrap();
+
+    // Check the signature help.
+    if let Some(signature_help) = signature_help {
+        assert_eq!(
+            signature_help.signatures.len(),
+            1,
+            "Expected one signature, got {:?}",
+            signature_help.signatures
+        );
+        assert_eq!(signature_help.signatures[0].label, "extrude");
+    } else {
+        panic!("Expected signature help");
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_signature_help_on_comma_trigger_on_before() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    // Send open file.
+    // We do this to trigger a valid ast.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send update file.
+    server
+        .did_change(tower_lsp::lsp_types::DidChangeTextDocumentParams {
+            text_document: tower_lsp::lsp_types::VersionedTextDocumentIdentifier {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                version: 1,
+            },
+            content_changes: vec![tower_lsp::lsp_types::TextDocumentContentChangeEvent {
+                range: None,
+                range_length: None,
+                text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude(length = 10,"
+                    .to_string(),
+            }],
+        })
+        .await;
+
+    // Send signature help request.
+    let signature_help = server
+        .signature_help(tower_lsp::lsp_types::SignatureHelpParams {
+            text_document_position_params: tower_lsp::lsp_types::TextDocumentPositionParams {
+                text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                    uri: "file:///test.kcl".try_into().unwrap(),
+                },
+                position: tower_lsp::lsp_types::Position { line: 9, character: 22 },
+            },
+            context: Some(tower_lsp::lsp_types::SignatureHelpContext {
+                trigger_kind: tower_lsp::lsp_types::SignatureHelpTriggerKind::INVOKED,
+                trigger_character: Some(",".to_string()),
+                is_retrigger: false,
+                active_signature_help: None,
+            }),
+            work_done_progress_params: Default::default(),
+        })
+        .await
+        .unwrap();
+
+    // Check the signature help.
+    if let Some(signature_help) = signature_help {
+        assert_eq!(
+            signature_help.signatures.len(),
+            1,
+            "Expected one signature, got {:?}",
+            signature_help.signatures
+        );
+        assert_eq!(signature_help.signatures[0].label, "extrude");
+    } else {
+        panic!("Expected signature help");
+    }
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_kcl_lsp_semantic_tokens() {
    let server = kcl_lsp_server(false).await.unwrap();
@ -1808,13 +2150,91 @@ async fn test_kcl_lsp_rename() {
        vec![tower_lsp::lsp_types::TextEdit {
            range: tower_lsp::lsp_types::Range {
                start: tower_lsp::lsp_types::Position { line: 0, character: 0 },
-                end: tower_lsp::lsp_types::Position { line: 0, character: 7 }
+                end: tower_lsp::lsp_types::Position { line: 0, character: 8 }
            },
            new_text: "newName = 1\n".to_string()
        }]
    );
 }

+#[tokio::test(flavor = "multi_thread")]
+async fn test_kcl_lsp_rename_no_hanging_parens() {
+    let server = kcl_lsp_server(false).await.unwrap();
+
+    let code = r#"myVARName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVARName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude(length = 3.14)"#;
+
+    // Send open file.
+    server
+        .did_open(tower_lsp::lsp_types::DidOpenTextDocumentParams {
+            text_document: tower_lsp::lsp_types::TextDocumentItem {
+                uri: "file:///test.kcl".try_into().unwrap(),
+                language_id: "kcl".to_string(),
+                version: 1,
+                text: code.to_string(),
+            },
+        })
+        .await;
+
+    // Send rename request.
+    let rename = server
+        .rename(tower_lsp::lsp_types::RenameParams {
+            text_document_position: tower_lsp::lsp_types::TextDocumentPositionParams {
+                text_document: tower_lsp::lsp_types::TextDocumentIdentifier {
+                    uri: "file:///test.kcl".try_into().unwrap(),
+                },
+                position: tower_lsp::lsp_types::Position { line: 0, character: 2 },
+            },
+            new_name: "myVarName".to_string(),
+            work_done_progress_params: Default::default(),
+        })
+        .await
+        .unwrap()
+        .unwrap();
+
+    // Check the rename.
+    let changes = rename.changes.unwrap();
+
+    let last_character = 27;
+
+    // Get the last character of the last line of the original code.
+    assert_eq!(code.lines().last().unwrap().chars().count(), last_character);
+
+    let u: tower_lsp::lsp_types::Url = "file:///test.kcl".try_into().unwrap();
+    assert_eq!(
+        changes.get(&u).unwrap().clone(),
+        vec![tower_lsp::lsp_types::TextEdit {
+            range: tower_lsp::lsp_types::Range {
+                start: tower_lsp::lsp_types::Position { line: 0, character: 0 },
+                // Its important we get back the right number here so that we actually replace the whole text!!
+                end: tower_lsp::lsp_types::Position {
+                    line: 9,
+                    character: last_character as u32
+                }
+            },
+            new_text: "myVarName = 100
+
+a1 = startSketchOn(offsetPlane(XY, offset = 10))
+  |> startProfile(at = [0, 0])
+  |> line(end = [myVarName, 0])
+  |> yLine(length = -100.0)
+  |> xLine(length = -100.0)
+  |> yLine(length = 100.0)
+  |> close()
+  |> extrude(length = 3.14)\n"
+                .to_string()
+        }]
+    );
+}
+
 #[tokio::test(flavor = "multi_thread")]
 async fn test_kcl_lsp_diagnostic_no_errors() {
    let server = kcl_lsp_server(false).await.unwrap();