Skip to content

Commit ce566fb

Browse files
nrcnrc
authored
Accept idents as KW args (#6644)
Support kw arg/local variable shorthand Signed-off-by: Nick Cameron <[email protected]>
1 parent b23fc9f commit ce566fb

File tree

9 files changed

+232
-191
lines changed

9 files changed

+232
-191
lines changed

‎public/kcl-samples/bench/bench-parts.kcl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,9 @@ fn connectorSketch(@plane, start) {
5757

5858
export fn connector(@plane, length) {
5959
connectorSketch(plane, start = [-12, 8])
60-
|> extrude(length = length)
60+
|> extrude(length)
6161
connectorSketch(plane, start = [16, 8])
62-
|> extrude(length = length)
62+
|> extrude(length)
6363
return 0
6464
}
6565

@@ -79,7 +79,7 @@ fn seatSlatSketch(@plane) {
7979

8080
export fn seatSlats(@plane, length) {
8181
seatSlatSketch(plane)
82-
|> extrude(length = length)
82+
|> extrude(length)
8383
return 0
8484
}
8585

@@ -99,7 +99,7 @@ fn backSlatsSketch(@plane) {
9999

100100
export fn backSlats(@plane, length) {
101101
b = backSlatsSketch(plane)
102-
|> extrude(length = length)
102+
|> extrude(length)
103103
return b
104104
}
105105

‎public/kcl-samples/enclosure/main.kcl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ holeDia = 4
1515
sketch001 = startSketchOn(XY)
1616
|> startProfile(at = [0, 0])
1717
|> angledLine(angle = 0, length = width, tag = $rectangleSegmentA001)
18-
|> angledLine(angle = segAng(rectangleSegmentA001) + 90, length = length, tag = $rectangleSegmentB001)
18+
|> angledLine(angle = segAng(rectangleSegmentA001) + 90, length, tag = $rectangleSegmentB001)
1919
|> angledLine(angle = segAng(rectangleSegmentA001), length = -segLen(rectangleSegmentA001), tag = $rectangleSegmentC001)
2020
|> line(endAbsolute = [profileStartX(%), profileStartY(%)], tag = $rectangleSegmentD001)
2121
|> close()
@@ -74,7 +74,7 @@ function001([
7474
sketch003 = startSketchOn(XY)
7575
|> startProfile(at = [width * 1.2, 0])
7676
|> angledLine(angle = 0, length = width, tag = $rectangleSegmentA002)
77-
|> angledLine(angle = segAng(rectangleSegmentA001) + 90, length = length, tag = $rectangleSegmentB002)
77+
|> angledLine(angle = segAng(rectangleSegmentA001) + 90, length, tag = $rectangleSegmentB002)
7878
|> angledLine(angle = segAng(rectangleSegmentA001), length = -segLen(rectangleSegmentA001), tag = $rectangleSegmentC002)
7979
|> line(endAbsolute = [profileStartX(%), profileStartY(%)], tag = $rectangleSegmentD002)
8080
|> close()

‎rust/kcl-lib/src/execution/exec_ast.rs

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,10 +1311,15 @@ impl Node<CallExpressionKw> {
13111311
Some(l) => {
13121312
fn_args.insert(l.name.clone(), arg);
13131313
}
1314-
None => errors.push(arg),
1314+
None => {
1315+
if let Some(id) = arg_expr.arg.ident_name() {
1316+
fn_args.insert(id.to_owned(), arg);
1317+
} else {
1318+
errors.push(arg);
1319+
}
1320+
}
13151321
}
13161322
}
1317-
let fn_args = fn_args; // remove mutability
13181323

13191324
// Evaluate the unlabeled first param, if any exists.
13201325
let unlabeled = if let Some(ref arg_expr) = self.unlabeled {
@@ -1323,12 +1328,15 @@ impl Node<CallExpressionKw> {
13231328
let value = ctx
13241329
.execute_expr(arg_expr, exec_state, &metadata, &[], StatementKind::Expression)
13251330
.await?;
1326-
Some(Arg::new(value, source_range))
1331+
1332+
let label = arg_expr.ident_name().map(str::to_owned);
1333+
1334+
Some((label, Arg::new(value, source_range)))
13271335
} else {
13281336
None
13291337
};
13301338

1331-
let args = Args::new_kw(
1339+
let mut args = Args::new_kw(
13321340
KwArgs {
13331341
unlabeled,
13341342
labeled: fn_args,
@@ -1347,6 +1355,20 @@ impl Node<CallExpressionKw> {
13471355
));
13481356
}
13491357

1358+
let formals = func.args(false);
1359+
1360+
// If it's possible the input arg was meant to be labelled and we probably don't want to use
1361+
// it as the input arg, then treat it as labelled.
1362+
if let Some((Some(label), _)) = &args.kw_args.unlabeled {
1363+
if (formals.iter().all(|a| a.label_required) || exec_state.pipe_value().is_some())
1364+
&& formals.iter().any(|a| &a.name == label && a.label_required)
1365+
&& !args.kw_args.labeled.contains_key(label)
1366+
{
1367+
let (label, arg) = args.kw_args.unlabeled.take().unwrap();
1368+
args.kw_args.labeled.insert(label.unwrap(), arg);
1369+
}
1370+
}
1371+
13501372
#[cfg(feature = "artifact-graph")]
13511373
let op = if func.feature_tree_operation() {
13521374
let op_labeled_args = args
@@ -1368,7 +1390,6 @@ impl Node<CallExpressionKw> {
13681390
None
13691391
};
13701392

1371-
let formals = func.args(false);
13721393
for (label, arg) in &args.kw_args.labeled {
13731394
match formals.iter().find(|p| &p.name == label) {
13741395
Some(p) => {
@@ -1865,6 +1886,21 @@ fn type_check_params_kw(
18651886
args: &mut KwArgs,
18661887
exec_state: &mut ExecState,
18671888
) -> Result<(), KclError> {
1889+
// If it's possible the input arg was meant to be labelled and we probably don't want to use
1890+
// it as the input arg, then treat it as labelled.
1891+
if let Some((Some(label), _)) = &args.unlabeled {
1892+
if (function_expression.params.iter().all(|p| p.labeled) || exec_state.pipe_value().is_some())
1893+
&& function_expression
1894+
.params
1895+
.iter()
1896+
.any(|p| &p.identifier.name == label && p.labeled)
1897+
&& !args.labeled.contains_key(label)
1898+
{
1899+
let (label, arg) = args.unlabeled.take().unwrap();
1900+
args.labeled.insert(label.unwrap(), arg);
1901+
}
1902+
}
1903+
18681904
for (label, arg) in &mut args.labeled {
18691905
match function_expression.params.iter().find(|p| &p.identifier.name == label) {
18701906
Some(p) => {
@@ -1959,10 +1995,11 @@ fn type_check_params_kw(
19591995
if let Some(arg) = &mut args.unlabeled {
19601996
if let Some(p) = function_expression.params.iter().find(|p| !p.labeled) {
19611997
if let Some(ty) = &p.type_ {
1962-
arg.value = arg
1998+
arg.1.value = arg
1999+
.1
19632000
.value
19642001
.coerce(
1965-
&RuntimeType::from_parsed(ty.inner.clone(), exec_state, arg.source_range)
2002+
&RuntimeType::from_parsed(ty.inner.clone(), exec_state, arg.1.source_range)
19662003
.map_err(|e| KclError::Semantic(e.into()))?,
19672004
exec_state,
19682005
)
@@ -1974,9 +2011,9 @@ fn type_check_params_kw(
19742011
.map(|n| format!("`{}`", n))
19752012
.unwrap_or_else(|| "this function".to_owned()),
19762013
ty.inner,
1977-
arg.value.human_friendly_type()
2014+
arg.1.value.human_friendly_type()
19782015
),
1979-
source_ranges: vec![arg.source_range],
2016+
source_ranges: vec![arg.1.source_range],
19802017
})
19812018
})?;
19822019
}
@@ -2139,10 +2176,11 @@ impl FunctionSource {
21392176
if let Some(arg) = &mut args.kw_args.unlabeled {
21402177
if let Some(p) = ast.params.iter().find(|p| !p.labeled) {
21412178
if let Some(ty) = &p.type_ {
2142-
arg.value = arg
2179+
arg.1.value = arg
2180+
.1
21432181
.value
21442182
.coerce(
2145-
&RuntimeType::from_parsed(ty.inner.clone(), exec_state, arg.source_range)
2183+
&RuntimeType::from_parsed(ty.inner.clone(), exec_state, arg.1.source_range)
21462184
.map_err(|e| KclError::Semantic(e.into()))?,
21472185
exec_state,
21482186
)
@@ -2152,7 +2190,7 @@ impl FunctionSource {
21522190
"The input argument of {} requires a value with type `{}`, but found {}",
21532191
props.name,
21542192
ty.inner,
2155-
arg.value.human_friendly_type(),
2193+
arg.1.value.human_friendly_type(),
21562194
),
21572195
source_ranges: vec![callsite],
21582196
})
@@ -2224,7 +2262,7 @@ impl FunctionSource {
22242262
.kw_args
22252263
.unlabeled
22262264
.as_ref()
2227-
.map(|arg| OpArg::new(OpKclValue::from(&arg.value), arg.source_range)),
2265+
.map(|arg| OpArg::new(OpKclValue::from(&arg.1.value), arg.1.source_range)),
22282266
labeled_args: op_labeled_args,
22292267
},
22302268
source_range: callsite,
@@ -2665,13 +2703,12 @@ a = foo()
26652703

26662704
#[tokio::test(flavor = "multi_thread")]
26672705
async fn test_sensible_error_when_missing_equals_in_kwarg() {
2668-
for (i, call) in ["f(x=1,y)", "f(x=1,3,z)", "f(x=1,y,z=1)", "f(x=1, 3 + 4, z=1)"]
2706+
for (i, call) in ["f(x=1,3,0)", "f(x=1,3,z)", "f(x=1,0,z=1)", "f(x=1, 3 + 4, z)"]
26692707
.into_iter()
26702708
.enumerate()
26712709
{
26722710
let program = format!(
26732711
"fn foo() {{ return 0 }}
2674-
y = 42
26752712
z = 0
26762713
fn f(x, y, z) {{ return 0 }}
26772714
{call}"
@@ -2691,9 +2728,10 @@ fn f(x, y, z) {{ return 0 }}
26912728

26922729
#[tokio::test(flavor = "multi_thread")]
26932730
async fn default_param_for_unlabeled() {
2694-
// Tests that the input param for myExtrude is taken from the pipeline value.
2731+
// Tests that the input param for myExtrude is taken from the pipeline value and same-name
2732+
// keyword args.
26952733
let ast = r#"fn myExtrude(@sk, length) {
2696-
return extrude(sk, length = length)
2734+
return extrude(sk, length)
26972735
}
26982736
sketch001 = startSketchOn(XY)
26992737
|> circle(center = [0, 0], radius = 93.75)
@@ -2703,6 +2741,18 @@ sketch001 = startSketchOn(XY)
27032741
parse_execute(ast).await.unwrap();
27042742
}
27052743

2744+
#[tokio::test(flavor = "multi_thread")]
2745+
async fn dont_use_unlabelled_as_input() {
2746+
// `length` should be used as the `length` argument to extrude, not the unlabelled input
2747+
let ast = r#"length = 10
2748+
startSketchOn(XY)
2749+
|> circle(center = [0, 0], radius = 93.75)
2750+
|> extrude(length)
2751+
"#;
2752+
2753+
parse_execute(ast).await.unwrap();
2754+
}
2755+
27062756
#[tokio::test(flavor = "multi_thread")]
27072757
async fn ascription_in_binop() {
27082758
let ast = r#"foo = tan(0): number(rad) - 4deg"#;

‎rust/kcl-lib/src/parsing/ast/types/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ impl<T> Node<T> {
186186
self.comment_start = start;
187187
}
188188

189-
pub fn map_ref<'a, U: 'a>(&'a self, f: fn(&'a T) -> U) -> Node<U> {
189+
pub fn map_ref<'a, U: 'a>(&'a self, f: impl Fn(&'a T) -> U) -> Node<U> {
190190
Node {
191191
inner: f(&self.inner),
192192
start: self.start,
@@ -1187,7 +1187,7 @@ impl Expr {
11871187

11881188
pub fn ident_name(&self) -> Option<&str> {
11891189
match self {
1190-
Expr::Name(ident) => Some(&ident.name.name),
1190+
Expr::Name(name) => name.local_ident().map(|n| n.inner),
11911191
_ => None,
11921192
}
11931193
}
@@ -2371,7 +2371,7 @@ impl Name {
23712371

23722372
pub fn local_ident(&self) -> Option<Node<&str>> {
23732373
if self.path.is_empty() && !self.abs_path {
2374-
Some(self.name.map_ref(|n| &n.name))
2374+
Some(self.name.map_ref(|n| &*n.name))
23752375
} else {
23762376
None
23772377
}

‎rust/kcl-lib/src/std/args.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,9 @@ impl Arg {
5959
#[derive(Debug, Clone, Default)]
6060
pub struct KwArgs {
6161
/// Unlabeled keyword args. Currently only the first arg can be unlabeled.
62-
pub unlabeled: Option<Arg>,
62+
/// If the argument was a local variable, then the first element of the tuple is its name
63+
/// which may be used to treat this arg as a labelled arg.
64+
pub unlabeled: Option<(Option<String>, Arg)>,
6365
/// Labeled args.
6466
pub labeled: IndexMap<String, Arg>,
6567
pub errors: Vec<Arg>,
@@ -342,6 +344,7 @@ impl Args {
342344
self.kw_args
343345
.unlabeled
344346
.as_ref()
347+
.map(|(_, a)| a)
345348
.or(self.args.first())
346349
.or(self.pipe_value.as_ref())
347350
}

‎rust/kcl-lib/src/std/array.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ async fn call_map_closure(
4848
ctxt: &ExecutorContext,
4949
) -> Result<KclValue, KclError> {
5050
let kw_args = KwArgs {
51-
unlabeled: Some(Arg::new(input, source_range)),
51+
unlabeled: Some((None, Arg::new(input, source_range))),
5252
labeled: Default::default(),
5353
errors: Vec::new(),
5454
};
@@ -104,7 +104,7 @@ async fn call_reduce_closure(
104104
let mut labeled = IndexMap::with_capacity(1);
105105
labeled.insert("accum".to_string(), Arg::new(accum, source_range));
106106
let kw_args = KwArgs {
107-
unlabeled: Some(Arg::new(elem, source_range)),
107+
unlabeled: Some((None, Arg::new(elem, source_range))),
108108
labeled,
109109
errors: Vec::new(),
110110
};

‎rust/kcl-lib/src/std/patterns.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ async fn make_transform<T: GeometryTrait>(
424424
meta: vec![source_range.into()],
425425
};
426426
let kw_args = KwArgs {
427-
unlabeled: Some(Arg::new(repetition_num, source_range)),
427+
unlabeled: Some((None, Arg::new(repetition_num, source_range))),
428428
labeled: Default::default(),
429429
errors: Vec::new(),
430430
};

‎rust/kcl-lib/tests/kcl_samples/enclosure/ast.snap

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -456,13 +456,7 @@ description: Result of parsing enclosure.kcl
456456
},
457457
{
458458
"type": "LabeledArg",
459-
"label": {
460-
"commentStart": 0,
461-
"end": 0,
462-
"name": "length",
463-
"start": 0,
464-
"type": "Identifier"
465-
},
459+
"label": null,
466460
"arg": {
467461
"abs_path": false,
468462
"commentStart": 0,
@@ -3178,13 +3172,7 @@ description: Result of parsing enclosure.kcl
31783172
},
31793173
{
31803174
"type": "LabeledArg",
3181-
"label": {
3182-
"commentStart": 0,
3183-
"end": 0,
3184-
"name": "length",
3185-
"start": 0,
3186-
"type": "Identifier"
3187-
},
3175+
"label": null,
31883176
"arg": {
31893177
"abs_path": false,
31903178
"commentStart": 0,

0 commit comments

Comments
 (0)