ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Parser ensures that pipe expressions have a % arg (#1933)

Browse Source 
Closes https://github.com/KittyCAD/modeling-app/issues/1411
This commit is contained in:
Adam Chalmers
2024-03-27 13:29:57 -05:00
committed by GitHub
parent 32a2835d0e
commit 868a560e1a
3 changed files with 61 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/wasm-lib/kcl/src/ast/types.rs
View File

@ -986,6 +986,17 @@ impl CallExpression {
})
}
/// Is at least one argument the '%' i.e. the substitution operator?
pub fn has_substitution_arg(&self) -> bool {
self.arguments
.iter()
.any(|arg| matches!(arg, Value::PipeSubstitution(_)))
}
pub fn as_source_ranges(&self) -> Vec<SourceRange> {
vec![SourceRange([self.start, self.end])]
}
pub fn replace_value(&mut self, source_range: SourceRange, new_value: Value) {
for arg in &mut self.arguments {
arg.replace_value(source_range, new_value.clone());

51
src/wasm-lib/kcl/src/parser/parser_impl.rs
View File

@ -139,7 +139,7 @@ fn pipe_expression(i: TokenSlice) -> PResult<PipeExpression> {
let mut values = vec![head];
let value_surrounded_by_comments = (
repeat(0.., preceded(opt(whitespace), non_code_node)), // Before the value
preceded(opt(whitespace), value_allowed_in_pipe_expr), // The value
preceded(opt(whitespace), fn_call), // The value
repeat(0.., noncode_just_after_code), // After the value
);
let tail: Vec<(Vec<_>, _, Vec<_>)> = repeat(
@ -151,7 +151,23 @@ fn pipe_expression(i: TokenSlice) -> PResult<PipeExpression> {
))
.parse_next(i)?;
// All child parsers have been run. Time to structure the return value.
// All child parsers have been run.
// First, ensure they all have a % in their args.
let calls_without_substitution = tail.iter().find_map(|(_nc, call_expr, _nc2)| {
if !call_expr.has_substitution_arg() {
Some(call_expr.as_source_ranges())
} else {
None
}
});
if let Some(source_ranges) = calls_without_substitution {
let err = KclError::Syntax(KclErrorDetails {
source_ranges,
message: "All expressions in a pipeline must use the % (substitution operator)".to_owned(),
});
return Err(ErrMode::Cut(err.into()));
}
// Time to structure the return value.
let mut code_count = 0;
let mut max_noncode_end = 0;
for (noncode_before, code, noncode_after) in tail {
@ -159,7 +175,7 @@ fn pipe_expression(i: TokenSlice) -> PResult<PipeExpression> {
max_noncode_end = nc.end.max(max_noncode_end);
non_code_meta.insert(code_count, nc);
}
values.push(code);
values.push(Value::CallExpression(Box::new(code)));
code_count += 1;
for nc in noncode_after {
max_noncode_end = nc.end.max(max_noncode_end);
@ -1561,7 +1577,7 @@ const mySk1 = startSketchAt([0, 0])"#;
#[test]
fn inline_comment_pipe_expression() {
let test_input = r#"a('XY')
|> b()
|> b(%)
|> c(%) // inline-comment
|> d(%)"#;
@ -1778,10 +1794,10 @@ const mySk1 = startSketchAt([0, 0])"#;
#[test]
fn some_pipe_expr() {
let test_program = r#"x()
|> y() /* this is
|> y(%) /* this is
a comment
spanning a few lines */
|> z()"#;
|> z(%)"#;
let tokens = crate::token::lexer(test_program);
let actual = pipe_expression.parse(&tokens).unwrap();
let n = actual.non_code_meta.non_code_nodes.len();
@ -1795,17 +1811,17 @@ const mySk1 = startSketchAt([0, 0])"#;
fn comments_in_pipe_expr() {
for (i, test_program) in [
r#"y() |> /*hi*/ z(%)"#,
"1 |>/*hi*/ f",
"1 |>/*hi*/ f(%)",
r#"y() |> /*hi*/ z(%)"#,
"1 /*hi*/ |> f",
"1 /*hi*/ |> f(%)",
"1
// Hi
|> f",
|> f(%)",
"1
/* Hi
there
*/
|> f",
|> f(%)",
]
.into_iter()
.enumerate()
@ -2814,6 +2830,17 @@ let myBox = box([0,0], -3, -16, -10)
let parser = crate::parser::Parser::new(tokens);
parser.ast().unwrap();
}
#[test]
fn must_use_percent_in_pipeline_fn() {
let some_program_string = r#"
foo()
|> bar(2)
"#;
let tokens = crate::token::lexer(some_program_string);
let parser = crate::parser::Parser::new(tokens);
let err = parser.ast().unwrap_err();
println!("{err}")
}
}
#[cfg(test)]
@ -2864,7 +2891,7 @@ mod snapshot_tests {
let tokens = crate::token::lexer($test_kcl_program);
let actual = match program.parse(&tokens) {
Ok(x) => x,
Err(_e) => panic!("could not parse test"),
Err(e) => panic!("could not parse test: {e:?}"),
};
insta::assert_json_snapshot!(actual);
}
@ -2972,7 +2999,7 @@ mod snapshot_tests {
"const mySketch = startSketchAt([0,0]) |> lineTo([1, 1], %) |> close(%)"
);
snapshot_test!(ah, "const myBox = startSketchAt(p)");
snapshot_test!(ai, r#"const myBox = f(1) |> g(2)"#);
snapshot_test!(ai, r#"const myBox = f(1) |> g(2, %)"#);
snapshot_test!(aj, r#"const myBox = startSketchAt(p) |> line([0, l], %)"#);
snapshot_test!(ak, "lineTo({ to: [0, 1] })");
snapshot_test!(al, "lineTo({ to: [0, 1], from: [3, 3] })");

16
src/wasm-lib/kcl/src/parser/snapshots/kcl_lib__parser__parser_impl__snapshot_tests__ai.snap
View File

@ -4,18 +4,18 @@ expression: actual
---
{
"start": 0,
"end": 26,
"end": 29,
"body": [
{
"type": "VariableDeclaration",
"type": "VariableDeclaration",
"start": 0,
"end": 26,
"end": 29,
"declarations": [
{
"type": "VariableDeclarator",
"start": 6,
"end": 26,
"end": 29,
"id": {
"type": "Identifier",
"start": 6,
@ -26,7 +26,7 @@ expression: actual
"type": "PipeExpression",
"type": "PipeExpression",
"start": 14,
"end": 26,
"end": 29,
"body": [
{
"type": "CallExpression",
@ -55,7 +55,7 @@ expression: actual
"type": "CallExpression",
"type": "CallExpression",
"start": 22,
"end": 26,
"end": 29,
"callee": {
"type": "Identifier",
"start": 22,
@ -70,6 +70,12 @@ expression: actual
"end": 25,
"value": 2,
"raw": "2"
},
{
"type": "PipeSubstitution",
"type": "PipeSubstitution",
"start": 27,
"end": 28
}
],
"optional": false