Skip to content

Commit 94452cc

Browse files
jessfrazjessfraz
authored
ability to set suggestions on lints (KittyCAD#6535)
* fix the lint tests which were not compiling Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates Signed-off-by: Jess Frazelle <github@jessfraz.com> * apply sugggestions for offsetplanes Signed-off-by: Jess Frazelle <github@jessfraz.com> * updates Signed-off-by: Jess Frazelle <github@jessfraz.com> * diagnostics and suggestions for offset planes Signed-off-by: Jess Frazelle <github@jessfraz.com> --------- Signed-off-by: Jess Frazelle <github@jessfraz.com>
1 parent 2e754f2 commit 94452cc

File tree

10 files changed

+339
-266
lines changed

10 files changed

+339
-266
lines changed

‎rust/kcl-lib/src/errors.rs‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use indexmap::IndexMap;
2+
use schemars::JsonSchema;
23
use serde::{Deserialize, Serialize};
34
use thiserror::Error;
45
use tower_lsp::lsp_types::{Diagnostic, DiagnosticSeverity};
@@ -228,13 +229,10 @@ impl IntoDiagnostic for KclErrorWithOutputs {
228229
fn to_lsp_diagnostics(&self, code: &str) -> Vec<Diagnostic> {
229230
let message = self.error.get_message();
230231
let source_ranges = self.error.source_ranges();
231-
println!("self: {:?}", self);
232232

233233
source_ranges
234234
.into_iter()
235235
.map(|source_range| {
236-
println!("source_range: {:?}", source_range);
237-
println!("filenames: {:?}", self.filenames);
238236
let source = self
239237
.source_files
240238
.get(&source_range.module_id())
@@ -658,10 +656,19 @@ pub enum Tag {
658656
None,
659657
}
660658

661-
#[derive(Debug, Clone, Serialize, Deserialize, ts_rs::TS)]
659+
#[derive(Debug, Clone, Serialize, Deserialize, ts_rs::TS, PartialEq, Eq, JsonSchema)]
662660
#[ts(export)]
663661
pub struct Suggestion {
664662
pub title: String,
665663
pub insert: String,
666664
pub source_range: SourceRange,
667665
}
666+
667+
pub type LspSuggestion = (Suggestion, tower_lsp::lsp_types::Range);
668+
669+
impl Suggestion {
670+
pub fn to_lsp_edit(&self, code: &str) -> LspSuggestion {
671+
let range = self.source_range.to_lsp_range(code);
672+
(self.clone(), range)
673+
}
674+
}

‎rust/kcl-lib/src/lint/checks/camel_case.rs‎

Lines changed: 57 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ fn lint_lower_camel_case_var(decl: &VariableDeclarator) -> Result<Vec<Discovered
3232
findings.push(Z0001.at(
3333
format!("found '{}'", name),
3434
SourceRange::new(ident.start, ident.end, ident.module_id),
35+
None,
3536
));
3637
return Ok(findings);
3738
}
@@ -48,6 +49,7 @@ fn lint_lower_camel_case_property(decl: &ObjectProperty) -> Result<Vec<Discovere
4849
findings.push(Z0001.at(
4950
format!("found '{}'", name),
5051
SourceRange::new(ident.start, ident.end, ident.module_id),
52+
None,
5153
));
5254
return Ok(findings);
5355
}
@@ -80,12 +82,36 @@ mod tests {
8082
use super::{lint_object_properties, lint_variables, Z0001};
8183
use crate::lint::rule::{assert_finding, test_finding, test_no_finding};
8284

83-
#[test]
84-
fn z0001_const() {
85-
assert_finding!(lint_variables, Z0001, "const Thickness = 0.5");
86-
assert_finding!(lint_variables, Z0001, "const THICKNESS = 0.5");
87-
assert_finding!(lint_variables, Z0001, "const THICC_NES = 0.5");
88-
assert_finding!(lint_variables, Z0001, "const thicc_nes = 0.5");
85+
#[tokio::test]
86+
async fn z0001_const() {
87+
assert_finding!(
88+
lint_variables,
89+
Z0001,
90+
"const Thickness = 0.5",
91+
"found 'Thickness'",
92+
None
93+
);
94+
assert_finding!(
95+
lint_variables,
96+
Z0001,
97+
"const THICKNESS = 0.5",
98+
"found 'THICKNESS'",
99+
None
100+
);
101+
assert_finding!(
102+
lint_variables,
103+
Z0001,
104+
"const THICC_NES = 0.5",
105+
"found 'THICC_NES'",
106+
None
107+
);
108+
assert_finding!(
109+
lint_variables,
110+
Z0001,
111+
"const thicc_nes = 0.5",
112+
"found 'thicc_nes'",
113+
None
114+
);
89115
}
90116

91117
test_finding!(
@@ -100,22 +126,24 @@ const pipeLargeDia = 20
100126
const thickness = 0.5
101127
102128
// Create the sketch to be revolved around the y-axis. Use the small diameter, large diameter, length, and thickness to define the sketch.
103-
const Part001 = startSketchOn('XY')
129+
const Part001 = startSketchOn(XY)
104130
|> startProfile(at = [pipeLargeDia - (thickness / 2), 38])
105-
|> line([thickness, 0], %)
106-
|> line([0, -1], %)
131+
|> line(end = [thickness, 0])
132+
|> line(end = [0, -1])
107133
|> angledLine(angle = 60, endAbsoluteX = pipeSmallDia + thickness)
108-
|> line([0, -pipeLength], %)
134+
|> line(end = [0, -pipeLength])
109135
|> angledLine(angle = -60, endAbsoluteX = pipeLargeDia + thickness)
110-
|> line([0, -1], %)
111-
|> line([-thickness, 0], %)
112-
|> line([0, 1], %)
136+
|> line(end = [0, -1])
137+
|> line(end = [-thickness, 0])
138+
|> line(end = [0, 1])
113139
|> angledLine(angle = 120, endAbsoluteX = pipeSmallDia)
114-
|> line([0, pipeLength], %)
140+
|> line(end = [0, pipeLength])
115141
|> angledLine(angle = 60, endAbsoluteX = pipeLargeDia)
116142
|> close()
117-
|> revolve({ axis = Y }, %)
118-
"
143+
|> revolve(axis = Y)
144+
",
145+
"found 'Part001'",
146+
None
119147
);
120148

121149
test_no_finding!(
@@ -130,21 +158,21 @@ const pipeLargeDia = 20
130158
const thickness = 0.5
131159
132160
// Create the sketch to be revolved around the y-axis. Use the small diameter, large diameter, length, and thickness to define the sketch.
133-
const part001 = startSketchOn('XY')
161+
const part001 = startSketchOn(XY)
134162
|> startProfile(at = [pipeLargeDia - (thickness / 2), 38])
135-
|> line([thickness, 0], %)
136-
|> line([0, -1], %)
163+
|> line(end = [thickness, 0])
164+
|> line(end = [0, -1])
137165
|> angledLine(angle = 60, endAbsoluteX = pipeSmallDia + thickness)
138-
|> line([0, -pipeLength], %)
166+
|> line(end = [0, -pipeLength])
139167
|> angledLine(angle = -60, endAbsoluteX = pipeLargeDia + thickness)
140-
|> line([0, -1], %)
141-
|> line([-thickness, 0], %)
142-
|> line([0, 1], %)
168+
|> line(end = [0, -1])
169+
|> line(end = [-thickness, 0])
170+
|> line(end = [0, 1])
143171
|> angledLine(angle = 120, endAbsoluteX = pipeSmallDia)
144-
|> line([0, pipeLength], %)
172+
|> line(end = [0, pipeLength])
145173
|> angledLine(angle = 60, endAbsoluteX = pipeLargeDia)
146174
|> close()
147-
|> revolve({ axis = Y }, %)
175+
|> revolve(axis = Y)
148176
"
149177
);
150178

@@ -153,7 +181,9 @@ const part001 = startSketchOn('XY')
153181
lint_object_properties,
154182
Z0001,
155183
"\
156-
let circ = {angle_start: 0, angle_end: 360, radius: radius}
157-
"
184+
let circ = {angle_start = 0, angle_end = 360, radius = 5}
185+
",
186+
"found 'angle_start'",
187+
None
158188
);
159189
}

‎rust/kcl-lib/src/lint/checks/mod.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
mod camel_case;
22
mod offset_plane;
3-
mod std_lib_args;
43

54
pub use camel_case::{lint_object_properties, lint_variables, Z0001};
65
pub use offset_plane::{lint_should_be_offset_plane, Z0003};
7-
pub use std_lib_args::{lint_call_expressions, Z0002};

‎rust/kcl-lib/src/lint/checks/offset_plane.rs‎

Lines changed: 54 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::collections::HashMap;
33
use anyhow::Result;
44

55
use crate::{
6+
errors::Suggestion,
67
lint::rule::{def_finding, Discovered, Finding},
78
parsing::ast::types::{BinaryPart, Expr, LiteralValue, ObjectExpression, UnaryOperator},
89
walk::Node,
@@ -45,19 +46,11 @@ pub fn lint_should_be_offset_plane(node: Node) -> Result<Vec<Discovered>> {
4546
return Ok(vec![]);
4647
};
4748

48-
let Some(plane) = arg.inner.properties.iter().find(|v| v.key.inner.name == "plane") else {
49-
return Ok(vec![]);
50-
};
51-
52-
let Expr::ObjectExpression(ref plane) = plane.inner.value else {
53-
return Ok(vec![]);
54-
};
55-
5649
let mut origin: Option<(f64, f64, f64)> = None;
5750
let mut x_vec: Option<(f64, f64, f64)> = None;
5851
let mut y_vec: Option<(f64, f64, f64)> = None;
5952

60-
for property in &plane.inner.properties {
53+
for property in &arg.inner.properties {
6154
let Expr::ObjectExpression(ref point) = property.inner.value else {
6255
return Ok(vec![]);
6356
};
@@ -146,13 +139,25 @@ pub fn lint_should_be_offset_plane(node: Node) -> Result<Vec<Discovered>> {
146139
return Ok(vec![]);
147140
};
148141

149-
let call_source_range = SourceRange::new(call.start, call.end, call.module_id);
142+
let call_source_range = SourceRange::new(
143+
call.arguments[0].start(),
144+
call.arguments[0].end(),
145+
call.arguments[0].module_id(),
146+
);
147+
148+
let offset = get_offset(origin, x_vec, y_vec);
149+
let suggestion = offset.map(|offset| Suggestion {
150+
title: "use offsetPlane instead".to_owned(),
151+
insert: format!("offsetPlane({}, offset = {})", plane_name, offset),
152+
source_range: call_source_range,
153+
});
150154
Ok(vec![Z0003.at(
151155
format!(
152156
"custom plane in startSketchOn; offsetPlane from {} would work here",
153157
plane_name
154158
),
155159
call_source_range,
160+
suggestion,
156161
)])
157162
}
158163

@@ -200,6 +205,35 @@ fn get_xyz(point: &ObjectExpression) -> Option<(f64, f64, f64)> {
200205
Some((x?, y?, z?))
201206
}
202207

208+
fn get_offset(origin: (f64, f64, f64), x_axis: (f64, f64, f64), y_axis: (f64, f64, f64)) -> Option<f64> {
209+
// Check which number is not a 1 or -1, or zero.
210+
// Return that back out since that is the offset.
211+
212+
// This is a bit of a hack, but it works for now.
213+
// We can do better later.
214+
if origin.0 != 1.0 && origin.0 != -1.0 && origin.0 != 0.0 {
215+
return Some(origin.0);
216+
} else if origin.1 != 1.0 && origin.1 != -1.0 && origin.1 != 0.0 {
217+
return Some(origin.1);
218+
} else if origin.2 != 1.0 && origin.2 != -1.0 && origin.2 != 0.0 {
219+
return Some(origin.2);
220+
} else if x_axis.0 != 1.0 && x_axis.0 != -1.0 && x_axis.0 != 0.0 {
221+
return Some(x_axis.0);
222+
} else if x_axis.1 != 1.0 && x_axis.1 != -1.0 && x_axis.1 != 0.0 {
223+
return Some(x_axis.1);
224+
} else if x_axis.2 != 1.0 && x_axis.2 != -1.0 && x_axis.2 != 0.0 {
225+
return Some(x_axis.2);
226+
} else if y_axis.0 != 1.0 && y_axis.0 != -1.0 && y_axis.0 != 0.0 {
227+
return Some(y_axis.0);
228+
} else if y_axis.1 != 1.0 && y_axis.1 != -1.0 && y_axis.1 != 0.0 {
229+
return Some(y_axis.1);
230+
} else if y_axis.2 != 1.0 && y_axis.2 != -1.0 && y_axis.2 != 0.0 {
231+
return Some(y_axis.2);
232+
}
233+
234+
None
235+
}
236+
203237
#[cfg(test)]
204238
mod tests {
205239
use super::{lint_should_be_offset_plane, Z0003};
@@ -211,14 +245,14 @@ mod tests {
211245
Z0003,
212246
"\
213247
startSketchOn({
214-
plane: {
215-
origin: { x: 0, y: -14.3, z: 0 },
216-
xAxis: { x: 1, y: 0, z: 0 },
217-
yAxis: { x: 0, y: 0, z: 1 },
218-
zAxis: { x: 0, y: -1, z: 0 }
219-
}
248+
origin = { x = 0, y = -14.3, z = 0 },
249+
xAxis = { x = 1, y = 0, z = 0 },
250+
yAxis = { x = 0, y = 0, z = 1 },
220251
})
221-
"
252+
|> startProfile(at = [0, 0])
253+
",
254+
"custom plane in startSketchOn; offsetPlane from XZ would work here",
255+
Some("offsetPlane(XZ, offset = -14.3)".to_string())
222256
);
223257

224258
test_no_finding!(
@@ -227,12 +261,9 @@ startSketchOn({
227261
Z0003,
228262
"\
229263
startSketchOn({
230-
plane: {
231-
origin: { x: 10, y: -14.3, z: 0 },
232-
xAxis: { x: 1, y: 0, z: 0 },
233-
yAxis: { x: 0, y: 0, z: 1 },
234-
zAxis: { x: 0, y: -1, z: 0 }
235-
}
264+
origin = { x = 10, y = -14.3, z = 0 },
265+
xAxis = { x = 1, y = 0, z = 0 },
266+
yAxis = { x = 0, y = 0, z = 1 },
236267
})
237268
"
238269
);

0 commit comments

Comments
 (0)