diff --git a/rust/kcl-lib/src/errors.rs b/rust/kcl-lib/src/errors.rs index ec0da1ccc..4fd038ee6 100644 --- a/rust/kcl-lib/src/errors.rs +++ b/rust/kcl-lib/src/errors.rs @@ -1,4 +1,5 @@ use indexmap::IndexMap; +use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use thiserror::Error; use tower_lsp::lsp_types::{Diagnostic, DiagnosticSeverity}; @@ -228,13 +229,10 @@ impl IntoDiagnostic for KclErrorWithOutputs { fn to_lsp_diagnostics(&self, code: &str) -> Vec { let message = self.error.get_message(); let source_ranges = self.error.source_ranges(); - println!("self: {:?}", self); source_ranges .into_iter() .map(|source_range| { - println!("source_range: {:?}", source_range); - println!("filenames: {:?}", self.filenames); let source = self .source_files .get(&source_range.module_id()) @@ -658,10 +656,19 @@ pub enum Tag { None, } -#[derive(Debug, Clone, Serialize, Deserialize, ts_rs::TS)] +#[derive(Debug, Clone, Serialize, Deserialize, ts_rs::TS, PartialEq, Eq, JsonSchema)] #[ts(export)] pub struct Suggestion { pub title: String, pub insert: String, pub source_range: SourceRange, } + +pub type LspSuggestion = (Suggestion, tower_lsp::lsp_types::Range); + +impl Suggestion { + pub fn to_lsp_edit(&self, code: &str) -> LspSuggestion { + let range = self.source_range.to_lsp_range(code); + (self.clone(), range) + } +} diff --git a/rust/kcl-lib/src/lint/checks/camel_case.rs b/rust/kcl-lib/src/lint/checks/camel_case.rs index 366da912a..6336e58f8 100644 --- a/rust/kcl-lib/src/lint/checks/camel_case.rs +++ b/rust/kcl-lib/src/lint/checks/camel_case.rs @@ -32,6 +32,7 @@ fn lint_lower_camel_case_var(decl: &VariableDeclarator) -> Result Result startProfile(at = [pipeLargeDia - (thickness / 2), 38]) - |> line([thickness, 0], %) - |> line([0, -1], %) + |> line(end = [thickness, 0]) + |> line(end = [0, -1]) |> angledLine(angle = 60, endAbsoluteX = pipeSmallDia + thickness) - |> line([0, -pipeLength], %) + |> line(end = [0, -pipeLength]) |> angledLine(angle = -60, endAbsoluteX = pipeLargeDia + thickness) - |> line([0, -1], %) - |> line([-thickness, 0], %) - |> line([0, 1], %) + |> line(end = [0, -1]) + |> line(end = [-thickness, 0]) + |> line(end = [0, 1]) |> angledLine(angle = 120, endAbsoluteX = pipeSmallDia) - |> line([0, pipeLength], %) + |> line(end = [0, pipeLength]) |> angledLine(angle = 60, endAbsoluteX = pipeLargeDia) |> close() - |> revolve({ axis = Y }, %) -" + |> revolve(axis = Y) +", + "found 'Part001'", + None ); test_no_finding!( @@ -130,21 +158,21 @@ const pipeLargeDia = 20 const thickness = 0.5 // Create the sketch to be revolved around the y-axis. Use the small diameter, large diameter, length, and thickness to define the sketch. -const part001 = startSketchOn('XY') +const part001 = startSketchOn(XY) |> startProfile(at = [pipeLargeDia - (thickness / 2), 38]) - |> line([thickness, 0], %) - |> line([0, -1], %) + |> line(end = [thickness, 0]) + |> line(end = [0, -1]) |> angledLine(angle = 60, endAbsoluteX = pipeSmallDia + thickness) - |> line([0, -pipeLength], %) + |> line(end = [0, -pipeLength]) |> angledLine(angle = -60, endAbsoluteX = pipeLargeDia + thickness) - |> line([0, -1], %) - |> line([-thickness, 0], %) - |> line([0, 1], %) + |> line(end = [0, -1]) + |> line(end = [-thickness, 0]) + |> line(end = [0, 1]) |> angledLine(angle = 120, endAbsoluteX = pipeSmallDia) - |> line([0, pipeLength], %) + |> line(end = [0, pipeLength]) |> angledLine(angle = 60, endAbsoluteX = pipeLargeDia) |> close() - |> revolve({ axis = Y }, %) + |> revolve(axis = Y) " ); @@ -153,7 +181,9 @@ const part001 = startSketchOn('XY') lint_object_properties, Z0001, "\ -let circ = {angle_start: 0, angle_end: 360, radius: radius} -" +let circ = {angle_start = 0, angle_end = 360, radius = 5} +", + "found 'angle_start'", + None ); } diff --git a/rust/kcl-lib/src/lint/checks/mod.rs b/rust/kcl-lib/src/lint/checks/mod.rs index f3cc11135..0bd629c3c 100644 --- a/rust/kcl-lib/src/lint/checks/mod.rs +++ b/rust/kcl-lib/src/lint/checks/mod.rs @@ -1,7 +1,5 @@ mod camel_case; mod offset_plane; -mod std_lib_args; pub use camel_case::{lint_object_properties, lint_variables, Z0001}; pub use offset_plane::{lint_should_be_offset_plane, Z0003}; -pub use std_lib_args::{lint_call_expressions, Z0002}; diff --git a/rust/kcl-lib/src/lint/checks/offset_plane.rs b/rust/kcl-lib/src/lint/checks/offset_plane.rs index f83d33bbc..f9df2d4fb 100644 --- a/rust/kcl-lib/src/lint/checks/offset_plane.rs +++ b/rust/kcl-lib/src/lint/checks/offset_plane.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use anyhow::Result; use crate::{ + errors::Suggestion, lint::rule::{def_finding, Discovered, Finding}, parsing::ast::types::{BinaryPart, Expr, LiteralValue, ObjectExpression, UnaryOperator}, walk::Node, @@ -45,19 +46,11 @@ pub fn lint_should_be_offset_plane(node: Node) -> Result> { return Ok(vec![]); }; - let Some(plane) = arg.inner.properties.iter().find(|v| v.key.inner.name == "plane") else { - return Ok(vec![]); - }; - - let Expr::ObjectExpression(ref plane) = plane.inner.value else { - return Ok(vec![]); - }; - let mut origin: Option<(f64, f64, f64)> = None; let mut x_vec: Option<(f64, f64, f64)> = None; let mut y_vec: Option<(f64, f64, f64)> = None; - for property in &plane.inner.properties { + for property in &arg.inner.properties { let Expr::ObjectExpression(ref point) = property.inner.value else { return Ok(vec![]); }; @@ -146,13 +139,25 @@ pub fn lint_should_be_offset_plane(node: Node) -> Result> { return Ok(vec![]); }; - let call_source_range = SourceRange::new(call.start, call.end, call.module_id); + let call_source_range = SourceRange::new( + call.arguments[0].start(), + call.arguments[0].end(), + call.arguments[0].module_id(), + ); + + let offset = get_offset(origin, x_vec, y_vec); + let suggestion = offset.map(|offset| Suggestion { + title: "use offsetPlane instead".to_owned(), + insert: format!("offsetPlane({}, offset = {})", plane_name, offset), + source_range: call_source_range, + }); Ok(vec![Z0003.at( format!( "custom plane in startSketchOn; offsetPlane from {} would work here", plane_name ), call_source_range, + suggestion, )]) } @@ -200,6 +205,35 @@ fn get_xyz(point: &ObjectExpression) -> Option<(f64, f64, f64)> { Some((x?, y?, z?)) } +fn get_offset(origin: (f64, f64, f64), x_axis: (f64, f64, f64), y_axis: (f64, f64, f64)) -> Option { + // Check which number is not a 1 or -1, or zero. + // Return that back out since that is the offset. + + // This is a bit of a hack, but it works for now. + // We can do better later. + if origin.0 != 1.0 && origin.0 != -1.0 && origin.0 != 0.0 { + return Some(origin.0); + } else if origin.1 != 1.0 && origin.1 != -1.0 && origin.1 != 0.0 { + return Some(origin.1); + } else if origin.2 != 1.0 && origin.2 != -1.0 && origin.2 != 0.0 { + return Some(origin.2); + } else if x_axis.0 != 1.0 && x_axis.0 != -1.0 && x_axis.0 != 0.0 { + return Some(x_axis.0); + } else if x_axis.1 != 1.0 && x_axis.1 != -1.0 && x_axis.1 != 0.0 { + return Some(x_axis.1); + } else if x_axis.2 != 1.0 && x_axis.2 != -1.0 && x_axis.2 != 0.0 { + return Some(x_axis.2); + } else if y_axis.0 != 1.0 && y_axis.0 != -1.0 && y_axis.0 != 0.0 { + return Some(y_axis.0); + } else if y_axis.1 != 1.0 && y_axis.1 != -1.0 && y_axis.1 != 0.0 { + return Some(y_axis.1); + } else if y_axis.2 != 1.0 && y_axis.2 != -1.0 && y_axis.2 != 0.0 { + return Some(y_axis.2); + } + + None +} + #[cfg(test)] mod tests { use super::{lint_should_be_offset_plane, Z0003}; @@ -211,14 +245,14 @@ mod tests { Z0003, "\ startSketchOn({ - plane: { - origin: { x: 0, y: -14.3, z: 0 }, - xAxis: { x: 1, y: 0, z: 0 }, - yAxis: { x: 0, y: 0, z: 1 }, - zAxis: { x: 0, y: -1, z: 0 } - } + origin = { x = 0, y = -14.3, z = 0 }, + xAxis = { x = 1, y = 0, z = 0 }, + yAxis = { x = 0, y = 0, z = 1 }, }) -" +|> startProfile(at = [0, 0]) +", + "custom plane in startSketchOn; offsetPlane from XZ would work here", + Some("offsetPlane(XZ, offset = -14.3)".to_string()) ); test_no_finding!( @@ -227,12 +261,9 @@ startSketchOn({ Z0003, "\ startSketchOn({ - plane: { - origin: { x: 10, y: -14.3, z: 0 }, - xAxis: { x: 1, y: 0, z: 0 }, - yAxis: { x: 0, y: 0, z: 1 }, - zAxis: { x: 0, y: -1, z: 0 } - } + origin = { x = 10, y = -14.3, z = 0 }, + xAxis = { x = 1, y = 0, z = 0 }, + yAxis = { x = 0, y = 0, z = 1 }, }) " ); diff --git a/rust/kcl-lib/src/lint/checks/std_lib_args.rs b/rust/kcl-lib/src/lint/checks/std_lib_args.rs deleted file mode 100644 index c9cb5171e..000000000 --- a/rust/kcl-lib/src/lint/checks/std_lib_args.rs +++ /dev/null @@ -1,169 +0,0 @@ -use std::sync::Arc; - -use anyhow::Result; - -use crate::{ - docs::StdLibFn, - lint::rule::{def_finding, Discovered, Finding}, - parsing::ast::types::{CallExpression, NodeRef}, - std::{FunctionKind, StdLib}, - walk::Node, - SourceRange, -}; - -def_finding!( - Z0002, - "Too many arguments to stdlib function", - "\ -Previously, we have not been failing when too many arguments are passed to a stdlib function. This is a problem because it can lead to unexpected behavior. We will in the future fail when too many arguments are passed to a function. So fix your code now." -); - -fn lint_too_many_args_std_lib_function( - f: Box, - exp: NodeRef<'_, CallExpression>, -) -> Result> { - let mut findings = vec![]; - - if f.name() == "pow" { - if exp.arguments.len() != 2 { - findings.push(Z0002.at( - format!("expected 2 arguments, found {}", exp.arguments.len()), - SourceRange::new(exp.start, exp.end, exp.module_id), - )); - } - return Ok(findings); - } - - if f.name() == "max" || f.name() == "min" { - if exp.arguments.len() < 2 { - findings.push(Z0002.at( - format!("expected at least 2 arguments, found {}", exp.arguments.len()), - SourceRange::new(exp.start, exp.end, exp.module_id), - )); - } - return Ok(findings); - } - - let fn_args_len = f.args(false).len(); - if exp.arguments.len() > fn_args_len { - findings.push(Z0002.at( - format!("expected {} arguments, found {}", fn_args_len, exp.arguments.len()), - SourceRange::new(exp.start, exp.end, exp.module_id), - )); - } - - Ok(findings) -} - -pub fn lint_call_expressions(exp: Node) -> Result> { - // Yes this is dumb but its only for a temporary amount of time so its fine. - let stdlib = Arc::new(StdLib::new()); - let Node::CallExpression(exp) = exp else { - return Ok(vec![]); - }; - - match stdlib.get_either(&exp.callee) { - FunctionKind::Core(func) => lint_too_many_args_std_lib_function(func, exp), - _ => Ok(vec![]), - } -} - -#[cfg(test)] -mod tests { - use super::{lint_call_expressions, Z0002}; - use crate::lint::rule::{test_finding, test_no_finding}; - - test_finding!( - z0002_full_bad, - lint_call_expressions, - Z0002, - "\ -// Shelf Bracket -// This is a shelf bracket made out of 6061-T6 aluminum sheet metal. The required thickness is calculated based on a point load of 300 lbs applied to the end of the shelf. There are two brackets holding up the shelf, so the moment experienced is divided by 2. The shelf is 1 foot long from the wall. - -// Define our bracket feet lengths -const shelfMountL = 8 // The length of the bracket holding up the shelf is 6 inches -const wallMountL = 6 // the length of the bracket - -// Define constants required to calculate the thickness needed to support 300 lbs -const sigmaAllow = 35000 // psi -const width = 6 // inch -const p = 300 // Force on shelf - lbs -const shelfLength = 12 // inches -const moment = shelfLength * p / 2 // Moment experienced at fixed end of bracket -const factorOfSafety = 2 // Factor of safety of 2 to be conservative - -// Calculate the thickness off the bending stress and factor of safety -const thickness = sqrt(6 * moment * factorOfSafety / (width * sigmaAllow)) - -// 0.25 inch fillet radius -const filletR = 0.25 - -// Sketch the bracket and extrude with fillets -const bracket = startSketchOn('XY') - |> startProfile(at = [0, 0]) - |> line([0, wallMountL], %, $outerEdge) - |> line([-shelfMountL, 0], %) - |> line([0, -thickness], %) - |> line([shelfMountL - thickness, 0], %, $innerEdge) - |> line([0, -wallMountL + thickness], %) - |> close() - |> extrude(width, %) - |> fillet( - radius = filletR, - tags = [getPreviousAdjacentEdge(innerEdge, %)] - ) - |> fillet( - radius = filletR + thickness, - tags = [getPreviousAdjacentEdge(outerEdge, %)] - ) -" - ); - - test_no_finding!( - z0002_full_good, - lint_call_expressions, - Z0002, - "\ -// Shelf Bracket -// This is a shelf bracket made out of 6061-T6 aluminum sheet metal. The required thickness is calculated based on a point load of 300 lbs applied to the end of the shelf. There are two brackets holding up the shelf, so the moment experienced is divided by 2. The shelf is 1 foot long from the wall. - -// Define our bracket feet lengths -const shelfMountL = 8 // The length of the bracket holding up the shelf is 6 inches -const wallMountL = 6 // the length of the bracket - -// Define constants required to calculate the thickness needed to support 300 lbs -const sigmaAllow = 35000 // psi -const width = 6 // inch -const p = 300 // Force on shelf - lbs -const shelfLength = 12 // inches -const moment = shelfLength * p / 2 // Moment experienced at fixed end of bracket -const factorOfSafety = 2 // Factor of safety of 2 to be conservative - -// Calculate the thickness off the bending stress and factor of safety -const thickness = sqrt(6 * moment * factorOfSafety / (width * sigmaAllow)) - -// 0.25 inch fillet radius -const filletR = 0.25 - -// Sketch the bracket and extrude with fillets -const bracket = startSketchOn('XY') - |> startProfile(at = [0, 0]) - |> line([0, wallMountL], %, $outerEdge) - |> line([-shelfMountL, 0], %) - |> line([0, -thickness], %) - |> line([shelfMountL - thickness, 0], %, $innerEdge) - |> line([0, -wallMountL + thickness], %) - |> close() - |> extrude(width, %) - |> fillet( - radius = filletR, - tags = [getPreviousAdjacentEdge(innerEdge)] - ) - |> fillet( - radius = filletR + thickness, - tags = [getPreviousAdjacentEdge(outerEdge)] - ) -" - ); -} diff --git a/rust/kcl-lib/src/lint/rule.rs b/rust/kcl-lib/src/lint/rule.rs index f9fcba6fd..9d729f4aa 100644 --- a/rust/kcl-lib/src/lint/rule.rs +++ b/rust/kcl-lib/src/lint/rule.rs @@ -3,7 +3,7 @@ use schemars::JsonSchema; use serde::Serialize; use tower_lsp::lsp_types::{Diagnostic, DiagnosticSeverity}; -use crate::{lsp::IntoDiagnostic, walk::Node, SourceRange}; +use crate::{errors::Suggestion, lsp::IntoDiagnostic, walk::Node, SourceRange}; /// Check the provided AST for any found rule violations. /// @@ -40,6 +40,22 @@ pub struct Discovered { /// Is this discovered issue overridden by the programmer? pub overridden: bool, + + /// Suggestion to fix the issue. + pub suggestion: Option, +} + +impl Discovered { + #[cfg(test)] + pub fn apply_suggestion(&self, src: &str) -> Option { + let suggestion = self.suggestion.as_ref()?; + Some(format!( + "{}{}{}", + &src[0..suggestion.source_range.start()], + suggestion.insert, + &src[suggestion.source_range.end()..] + )) + } } #[cfg(feature = "pyo3")] @@ -80,6 +96,7 @@ impl IntoDiagnostic for &Discovered { fn to_lsp_diagnostics(&self, code: &str) -> Vec { let message = self.finding.title.to_owned(); let source_range = self.pos; + let edit = self.suggestion.as_ref().map(|s| s.to_lsp_edit(code)); vec![Diagnostic { range: source_range.to_lsp_range(code), @@ -91,7 +108,7 @@ impl IntoDiagnostic for &Discovered { message, related_information: None, tags: None, - data: None, + data: edit.map(|e| serde_json::to_value(e).unwrap()), }] } @@ -121,12 +138,13 @@ pub struct Finding { impl Finding { /// Create a new Discovered finding at the specific Position. - pub fn at(&self, description: String, pos: SourceRange) -> Discovered { + pub fn at(&self, description: String, pos: SourceRange, suggestion: Option) -> Discovered { Discovered { description, finding: self.clone(), pos, overridden: false, + suggestion, } } } @@ -182,7 +200,11 @@ mod test { macro_rules! assert_no_finding { ( $check:expr, $finding:expr, $kcl:expr ) => { - let prog = $crate::parsing::top_level_parse($kcl).unwrap(); + let prog = $crate::Program::parse_no_errs($kcl).unwrap(); + + // Ensure the code still works. + $crate::execution::parse_execute($kcl).await.unwrap(); + for discovered_finding in prog.lint($check).unwrap() { if discovered_finding.finding == $finding { assert!(false, "Finding {:?} was emitted", $finding.code); @@ -192,11 +214,28 @@ mod test { } macro_rules! assert_finding { - ( $check:expr, $finding:expr, $kcl:expr ) => { - let prog = $crate::parsing::top_level_parse($kcl).unwrap(); + ( $check:expr, $finding:expr, $kcl:expr, $output:expr, $suggestion:expr ) => { + let prog = $crate::Program::parse_no_errs($kcl).unwrap(); + + // Ensure the code still works. + $crate::execution::parse_execute($kcl).await.unwrap(); for discovered_finding in prog.lint($check).unwrap() { + pretty_assertions::assert_eq!(discovered_finding.description, $output,); + if discovered_finding.finding == $finding { + pretty_assertions::assert_eq!( + discovered_finding.suggestion.clone().map(|s| s.insert), + $suggestion, + ); + + if discovered_finding.suggestion.is_some() { + // Apply the suggestion to the source code. + let code = discovered_finding.apply_suggestion($kcl).unwrap(); + + // Ensure the code still works. + $crate::execution::parse_execute(&code).await.unwrap(); + } return; } } @@ -205,18 +244,18 @@ mod test { } macro_rules! test_finding { - ( $name:ident, $check:expr, $finding:expr, $kcl:expr ) => { - #[test] - fn $name() { - $crate::lint::rule::assert_finding!($check, $finding, $kcl); + ( $name:ident, $check:expr, $finding:expr, $kcl:expr, $output:expr, $suggestion:expr ) => { + #[tokio::test] + async fn $name() { + $crate::lint::rule::assert_finding!($check, $finding, $kcl, $output, $suggestion); } }; } macro_rules! test_no_finding { ( $name:ident, $check:expr, $finding:expr, $kcl:expr ) => { - #[test] - fn $name() { + #[tokio::test] + async fn $name() { $crate::lint::rule::assert_no_finding!($check, $finding, $kcl); } }; diff --git a/rust/kcl-lib/src/lsp/kcl/mod.rs b/rust/kcl-lib/src/lsp/kcl/mod.rs index 27fc5f4fb..757dc015e 100644 --- a/rust/kcl-lib/src/lsp/kcl/mod.rs +++ b/rust/kcl-lib/src/lsp/kcl/mod.rs @@ -17,30 +17,31 @@ use tokio::sync::RwLock; use tower_lsp::{ jsonrpc::Result as RpcResult, lsp_types::{ - CodeAction, CodeActionKind, CodeActionOrCommand, CodeActionParams, CodeActionResponse, CompletionItem, - CompletionItemKind, CompletionOptions, CompletionParams, CompletionResponse, CreateFilesParams, - DeleteFilesParams, Diagnostic, DiagnosticOptions, DiagnosticServerCapabilities, DiagnosticSeverity, - DidChangeConfigurationParams, DidChangeTextDocumentParams, DidChangeWatchedFilesParams, - DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, - DidSaveTextDocumentParams, DocumentDiagnosticParams, DocumentDiagnosticReport, DocumentDiagnosticReportResult, - DocumentFilter, DocumentFormattingParams, DocumentSymbol, DocumentSymbolParams, DocumentSymbolResponse, - Documentation, FoldingRange, FoldingRangeParams, FoldingRangeProviderCapability, FullDocumentDiagnosticReport, - Hover as LspHover, HoverContents, HoverParams, HoverProviderCapability, InitializeParams, InitializeResult, - InitializedParams, InlayHint, InlayHintParams, InsertTextFormat, MarkupContent, MarkupKind, MessageType, OneOf, - Position, RelatedFullDocumentDiagnosticReport, RenameFilesParams, RenameParams, SemanticToken, - SemanticTokenModifier, SemanticTokenType, SemanticTokens, SemanticTokensFullOptions, SemanticTokensLegend, - SemanticTokensOptions, SemanticTokensParams, SemanticTokensRegistrationOptions, SemanticTokensResult, - SemanticTokensServerCapabilities, ServerCapabilities, SignatureHelp, SignatureHelpOptions, SignatureHelpParams, - StaticRegistrationOptions, TextDocumentItem, TextDocumentRegistrationOptions, TextDocumentSyncCapability, - TextDocumentSyncKind, TextDocumentSyncOptions, TextEdit, WorkDoneProgressOptions, WorkspaceEdit, - WorkspaceFolder, WorkspaceFoldersServerCapabilities, WorkspaceServerCapabilities, + CodeAction, CodeActionKind, CodeActionOptions, CodeActionOrCommand, CodeActionParams, + CodeActionProviderCapability, CodeActionResponse, CompletionItem, CompletionItemKind, CompletionOptions, + CompletionParams, CompletionResponse, CreateFilesParams, DeleteFilesParams, Diagnostic, DiagnosticOptions, + DiagnosticServerCapabilities, DiagnosticSeverity, DidChangeConfigurationParams, DidChangeTextDocumentParams, + DidChangeWatchedFilesParams, DidChangeWorkspaceFoldersParams, DidCloseTextDocumentParams, + DidOpenTextDocumentParams, DidSaveTextDocumentParams, DocumentDiagnosticParams, DocumentDiagnosticReport, + DocumentDiagnosticReportResult, DocumentFilter, DocumentFormattingParams, DocumentSymbol, DocumentSymbolParams, + DocumentSymbolResponse, Documentation, FoldingRange, FoldingRangeParams, FoldingRangeProviderCapability, + FullDocumentDiagnosticReport, Hover as LspHover, HoverContents, HoverParams, HoverProviderCapability, + InitializeParams, InitializeResult, InitializedParams, InlayHint, InlayHintParams, InsertTextFormat, + MarkupContent, MarkupKind, MessageType, OneOf, Position, RelatedFullDocumentDiagnosticReport, + RenameFilesParams, RenameParams, SemanticToken, SemanticTokenModifier, SemanticTokenType, SemanticTokens, + SemanticTokensFullOptions, SemanticTokensLegend, SemanticTokensOptions, SemanticTokensParams, + SemanticTokensRegistrationOptions, SemanticTokensResult, SemanticTokensServerCapabilities, ServerCapabilities, + SignatureHelp, SignatureHelpOptions, SignatureHelpParams, StaticRegistrationOptions, TextDocumentItem, + TextDocumentRegistrationOptions, TextDocumentSyncCapability, TextDocumentSyncKind, TextDocumentSyncOptions, + TextEdit, WorkDoneProgressOptions, WorkspaceEdit, WorkspaceFolder, WorkspaceFoldersServerCapabilities, + WorkspaceServerCapabilities, }, Client, LanguageServer, }; use crate::{ docs::kcl_doc::DocData, - errors::Suggestion, + errors::LspSuggestion, exec::KclValue, execution::{cache, kcl_value::FunctionSource}, lsp::{ @@ -837,6 +838,11 @@ impl LanguageServer for Backend { Ok(InitializeResult { capabilities: ServerCapabilities { + code_action_provider: Some(CodeActionProviderCapability::Options(CodeActionOptions { + code_action_kinds: Some(vec![CodeActionKind::QUICKFIX]), + resolve_provider: Some(false), + work_done_progress_options: WorkDoneProgressOptions::default(), + })), completion_provider: Some(CompletionOptions { resolve_provider: Some(false), trigger_characters: Some(vec![".".to_string()]), @@ -1452,14 +1458,18 @@ impl LanguageServer for Backend { .diagnostics .into_iter() .filter_map(|diagnostic| { - let (suggestion, range) = diagnostic.data.as_ref().and_then(|data| { - serde_json::from_value::<(Suggestion, tower_lsp::lsp_types::Range)>(data.clone()).ok() - })?; + let (suggestion, range) = diagnostic + .data + .as_ref() + .and_then(|data| serde_json::from_value::(data.clone()).ok())?; let edit = TextEdit { range, new_text: suggestion.insert, }; let changes = HashMap::from([(params.text_document.uri.clone(), vec![edit])]); + + // If you add more code action kinds, make sure you add it to the server + // capabilities on initialization! Some(CodeActionOrCommand::CodeAction(CodeAction { title: suggestion.title, kind: Some(CodeActionKind::QUICKFIX), diff --git a/rust/kcl-lib/src/lsp/mod.rs b/rust/kcl-lib/src/lsp/mod.rs index 0ff613d7f..2058305a7 100644 --- a/rust/kcl-lib/src/lsp/mod.rs +++ b/rust/kcl-lib/src/lsp/mod.rs @@ -19,10 +19,7 @@ use crate::{ impl IntoDiagnostic for CompilationError { fn to_lsp_diagnostics(&self, code: &str) -> Vec { - let edit = self.suggestion.as_ref().map(|s| { - let range = s.source_range.to_lsp_range(code); - serde_json::to_value((s, range)).unwrap() - }); + let edit = self.suggestion.as_ref().map(|s| s.to_lsp_edit(code)); vec![Diagnostic { range: self.source_range.to_lsp_range(code), @@ -33,7 +30,7 @@ impl IntoDiagnostic for CompilationError { message: self.message.clone(), related_information: None, tags: self.tag.to_lsp_tags(), - data: edit, + data: edit.map(|e| serde_json::to_value(e).unwrap()), }] } diff --git a/rust/kcl-lib/src/lsp/tests.rs b/rust/kcl-lib/src/lsp/tests.rs index 717f22ba7..6c4902d17 100644 --- a/rust/kcl-lib/src/lsp/tests.rs +++ b/rust/kcl-lib/src/lsp/tests.rs @@ -1,14 +1,19 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use pretty_assertions::assert_eq; use tower_lsp::{ - lsp_types::{Diagnostic, SemanticTokenModifier, SemanticTokenType}, + lsp_types::{ + CodeActionKind, CodeActionOrCommand, Diagnostic, SemanticTokenModifier, SemanticTokenType, TextEdit, + WorkspaceEdit, + }, LanguageServer, }; use crate::{ + errors::{LspSuggestion, Suggestion}, lsp::test_util::{copilot_lsp_server, kcl_lsp_server}, parsing::ast::types::{Node, Program}, + SourceRange, }; #[track_caller] @@ -3549,3 +3554,129 @@ startSketchOn(XY) server.executor_ctx().await.clone().unwrap().close().await; } + +#[tokio::test(flavor = "multi_thread")] +async fn kcl_test_kcl_lsp_code_actions_lint_offset_planes() { + let server = kcl_lsp_server(false).await.unwrap(); + + // Send open file. + server + .did_open(tower_lsp::lsp_types::DidOpenTextDocumentParams { + text_document: tower_lsp::lsp_types::TextDocumentItem { + uri: "file:///testlint.kcl".try_into().unwrap(), + language_id: "kcl".to_string(), + version: 1, + text: r#"startSketchOn({ + origin = { x = 0, y = -14.3, z = 0 }, + xAxis = { x = 1, y = 0, z = 0 }, + yAxis = { x = 0, y = 0, z = 1 }, +}) +|> startProfile(at = [0, 0])"# + .to_string(), + }, + }) + .await; + + // Send diagnostics request. + let diagnostics = server + .diagnostic(tower_lsp::lsp_types::DocumentDiagnosticParams { + text_document: tower_lsp::lsp_types::TextDocumentIdentifier { + uri: "file:///testlint.kcl".try_into().unwrap(), + }, + partial_result_params: Default::default(), + work_done_progress_params: Default::default(), + identifier: None, + previous_result_id: None, + }) + .await + .unwrap(); + + // Check the diagnostics. + let tower_lsp::lsp_types::DocumentDiagnosticReportResult::Report(diagnostics) = diagnostics else { + panic!("Expected diagnostics"); + }; + + let tower_lsp::lsp_types::DocumentDiagnosticReport::Full(diagnostics) = diagnostics else { + panic!("Expected full diagnostics"); + }; + assert_eq!(diagnostics.full_document_diagnostic_report.items.len(), 1); + assert_eq!( + diagnostics.full_document_diagnostic_report.items[0].message, + "offsetPlane should be used to define a new plane offset from the origin" + ); + + // Make sure we get the suggestion data. + assert_eq!( + diagnostics.full_document_diagnostic_report.items[0] + .data + .clone() + .map(|d| serde_json::from_value::(d).unwrap()), + Some(( + Suggestion { + insert: "offsetPlane(XZ, offset = -14.3)".to_string(), + source_range: SourceRange::new(14, 133, Default::default()), + title: "use offsetPlane instead".to_string(), + }, + tower_lsp::lsp_types::Range { + start: tower_lsp::lsp_types::Position { line: 0, character: 14 }, + end: tower_lsp::lsp_types::Position { line: 4, character: 1 }, + } + )) + ); + + let diagnostic = diagnostics.full_document_diagnostic_report.items[0].clone(); + + // Run a code action. + let code_action = server + .code_action(tower_lsp::lsp_types::CodeActionParams { + text_document: tower_lsp::lsp_types::TextDocumentIdentifier { + uri: "file:///testlint.kcl".try_into().unwrap(), + }, + range: tower_lsp::lsp_types::Range { + start: tower_lsp::lsp_types::Position { line: 0, character: 14 }, + end: tower_lsp::lsp_types::Position { line: 4, character: 1 }, + }, + context: tower_lsp::lsp_types::CodeActionContext { + diagnostics: vec![diagnostic.clone()], + only: None, + trigger_kind: Default::default(), + }, + work_done_progress_params: Default::default(), + partial_result_params: Default::default(), + }) + .await + .unwrap(); + + assert!(code_action.is_some()); + + let code_action = code_action.unwrap(); + + assert_eq!(code_action.len(), 1); + + assert_eq!( + code_action[0], + CodeActionOrCommand::CodeAction(tower_lsp::lsp_types::CodeAction { + title: "use offsetPlane instead".to_string(), + kind: Some(CodeActionKind::QUICKFIX), + diagnostics: Some(vec![diagnostic]), + edit: Some(WorkspaceEdit { + changes: Some(HashMap::from_iter(vec![( + "file:///testlint.kcl".try_into().unwrap(), + vec![TextEdit { + range: tower_lsp::lsp_types::Range { + start: tower_lsp::lsp_types::Position { line: 0, character: 14 }, + end: tower_lsp::lsp_types::Position { line: 4, character: 1 }, + }, + new_text: "offsetPlane(XZ, offset = -14.3)".to_string(), + }], + )])), + document_changes: None, + change_annotations: None, + }), + command: None, + is_preferred: Some(true), + disabled: None, + data: None, + }) + ); +} diff --git a/rust/kcl-lib/src/parsing/ast/types/mod.rs b/rust/kcl-lib/src/parsing/ast/types/mod.rs index a209380f5..957463ccd 100644 --- a/rust/kcl-lib/src/parsing/ast/types/mod.rs +++ b/rust/kcl-lib/src/parsing/ast/types/mod.rs @@ -323,7 +323,6 @@ impl Node { let rules = vec![ crate::lint::checks::lint_variables, crate::lint::checks::lint_object_properties, - crate::lint::checks::lint_call_expressions, crate::lint::checks::lint_should_be_offset_plane, ];