Merge remote-tracking branch 'origin/main' into paultag/import

This commit is contained in:
Paul R. Tagliamonte
2025-03-13 11:17:34 -04:00
122 changed files with 1813 additions and 1666 deletions

View File

@ -48,6 +48,15 @@ impl ExecErrorWithState {
}
}
impl ExecError {
pub fn as_kcl_error(&self) -> Option<&crate::KclError> {
let ExecError::Kcl(k) = &self else {
return None;
};
Some(&k.error)
}
}
impl From<ExecError> for ExecErrorWithState {
fn from(error: ExecError) -> Self {
Self {

View File

@ -596,12 +596,11 @@ impl ExecutorContext {
self.exec_module_for_result(module_id, exec_state, ExecutionKind::Normal, metadata.source_range)
.await?
.unwrap_or_else(|| {
// The module didn't have a return value. Currently,
// the only way to have a return value is with the final
// statement being an expression statement.
//
// TODO: Make a warning when we support them in the
// execution phase.
exec_state.warn(CompilationError::err(
metadata.source_range,
"Imported module has no return value. The last statement of the module must be an expression, usually the Solid.",
));
let mut new_meta = vec![metadata.to_owned()];
new_meta.extend(meta);
KclValue::KclNone {
@ -1187,7 +1186,7 @@ impl Node<CallExpressionKw> {
},
self.into(),
ctx.clone(),
exec_state.mod_local.pipe_value.clone().map(Arg::synthetic),
exec_state.mod_local.pipe_value.clone().map(|v| Arg::new(v, callsite)),
);
match ctx.stdlib.get_either(fn_name) {
FunctionKind::Core(func) => {
@ -1349,7 +1348,7 @@ impl Node<CallExpression> {
fn_args,
self.into(),
ctx.clone(),
exec_state.mod_local.pipe_value.clone().map(Arg::synthetic),
exec_state.mod_local.pipe_value.clone().map(|v| Arg::new(v, callsite)),
);
let mut return_value = {
// Don't early-return in this block.
@ -2000,7 +1999,11 @@ impl FunctionSource {
args,
source_range,
ctx.clone(),
exec_state.mod_local.pipe_value.clone().map(Arg::synthetic),
exec_state
.mod_local
.pipe_value
.clone()
.map(|v| Arg::new(v, source_range)),
);
func(exec_state, args).await.map(Some)

View File

@ -659,7 +659,11 @@ impl KclValue {
args,
source_range,
ctx.clone(),
exec_state.mod_local.pipe_value.clone().map(Arg::synthetic),
exec_state
.mod_local
.pipe_value
.clone()
.map(|v| Arg::new(v, source_range)),
);
let result = func(exec_state, args).await.map(Some);
exec_state.mut_stack().pop_env();

View File

@ -844,11 +844,23 @@ fn object_property(i: &mut TokenSlice) -> PResult<Node<ObjectProperty>> {
))
.parse_next(i)?;
ignore_whitespace(i);
let expr = expression
let expr = match expression
.context(expected(
"the value which you're setting the property to, e.g. in 'height: 4', the value is 4",
))
.parse_next(i)?;
.parse_next(i)
{
Ok(expr) => expr,
Err(_) => {
return Err(ErrMode::Cut(
CompilationError::fatal(
SourceRange::from(sep),
"This property has a label, but no value. Put some value after the equals sign",
)
.into(),
));
}
};
let result = Node {
start: key.start,
@ -2810,7 +2822,7 @@ fn fn_call_kw(i: &mut TokenSlice) -> PResult<Node<CallExpressionKw>> {
ignore_whitespace(i);
#[allow(clippy::large_enum_variant)]
pub enum ArgPlace {
enum ArgPlace {
NonCode(Node<NonCodeNode>),
LabeledArg(LabeledArg),
UnlabeledArg(Expr),
@ -2827,22 +2839,34 @@ fn fn_call_kw(i: &mut TokenSlice) -> PResult<Node<CallExpressionKw>> {
.parse_next(i)?;
let (args, non_code_nodes): (Vec<_>, BTreeMap<usize, _>) = args.into_iter().enumerate().try_fold(
(Vec::new(), BTreeMap::new()),
|(mut args, mut non_code_nodes), (i, e)| {
|(mut args, mut non_code_nodes), (index, e)| {
match e {
ArgPlace::NonCode(x) => {
non_code_nodes.insert(i, vec![x]);
non_code_nodes.insert(index, vec![x]);
}
ArgPlace::LabeledArg(x) => {
args.push(x);
}
ArgPlace::UnlabeledArg(arg) => {
return Err(ErrMode::Cut(
CompilationError::fatal(
SourceRange::from(arg),
"This argument needs a label, but it doesn't have one",
let followed_by_equals = peek((opt(whitespace), equals)).parse_next(i).is_ok();
let err = if followed_by_equals {
ErrMode::Cut(
CompilationError::fatal(
SourceRange::from(arg),
"This argument has a label, but no value. Put some value after the equals sign",
)
.into(),
)
.into(),
));
} else {
ErrMode::Cut(
CompilationError::fatal(
SourceRange::from(arg),
"This argument needs a label, but it doesn't have one",
)
.into(),
)
};
return Err(err);
}
}
Ok((args, non_code_nodes))
@ -4678,6 +4702,42 @@ baz = 2
);
}
}
#[test]
fn test_sensible_error_when_missing_rhs_of_kw_arg() {
for (i, program) in ["f(x, y=)"].into_iter().enumerate() {
let tokens = crate::parsing::token::lex(program, ModuleId::default()).unwrap();
let err = fn_call_kw.parse(tokens.as_slice()).unwrap_err();
let cause = err.inner().cause.as_ref().unwrap();
assert_eq!(
cause.message, "This argument has a label, but no value. Put some value after the equals sign",
"failed test {i}: {program}"
);
assert_eq!(
cause.source_range.start(),
program.find("y").unwrap(),
"failed test {i}: {program}"
);
}
}
#[test]
fn test_sensible_error_when_missing_rhs_of_obj_property() {
for (i, program) in ["{x = 1, y =}"].into_iter().enumerate() {
let tokens = crate::parsing::token::lex(program, ModuleId::default()).unwrap();
let err = object.parse(tokens.as_slice()).unwrap_err();
let cause = err.inner().cause.as_ref().unwrap();
assert_eq!(
cause.message, "This property has a label, but no value. Put some value after the equals sign",
"failed test {i}: {program}"
);
assert_eq!(
cause.source_range.start(),
program.rfind('=').unwrap(),
"failed test {i}: {program}"
);
}
}
}
#[cfg(test)]

View File

@ -159,6 +159,49 @@ impl Args {
})
}
/// Get a labelled keyword arg, check it's an array, and return all items in the array
/// plus their source range.
pub(crate) fn kw_arg_array_and_source<'a, T>(&'a self, label: &str) -> Result<Vec<(T, SourceRange)>, KclError>
where
T: FromKclValue<'a>,
{
let Some(arg) = self.kw_args.labeled.get(label) else {
let err = KclError::Semantic(KclErrorDetails {
source_ranges: vec![self.source_range],
message: format!("This function requires a keyword argument '{label}'"),
});
return Err(err);
};
let Some(array) = arg.value.as_array() else {
let err = KclError::Semantic(KclErrorDetails {
source_ranges: vec![arg.source_range],
message: format!(
"Expected an array of {} but found {}",
type_name::<T>(),
arg.value.human_friendly_type()
),
});
return Err(err);
};
array
.iter()
.map(|item| {
let source = SourceRange::from(item);
let val = FromKclValue::from_kcl_val(item).ok_or_else(|| {
KclError::Semantic(KclErrorDetails {
source_ranges: arg.source_ranges(),
message: format!(
"Expected a {} but found {}",
type_name::<T>(),
arg.value.human_friendly_type()
),
})
})?;
Ok((val, source))
})
.collect::<Result<Vec<_>, _>>()
}
/// Get the unlabeled keyword argument. If not set, returns None.
pub(crate) fn unlabeled_kw_arg_unconverted(&self) -> Option<&Arg> {
self.kw_args
@ -184,7 +227,7 @@ impl Args {
T::from_kcl_val(&arg.value).ok_or_else(|| {
let expected_type_name = tynm::type_name::<T>();
let actual_type_name = arg.value.human_friendly_type();
let msg_base = format!("This function expected this argument to be of type {expected_type_name} but it's actually of type {actual_type_name}");
let msg_base = format!("This function expected the input argument to be of type {expected_type_name} but it's actually of type {actual_type_name}");
let suggestion = match (expected_type_name.as_str(), actual_type_name) {
("SolidSet", "Sketch") => Some(
"You can convert a sketch (2D) into a Solid (3D) by calling a function like `extrude` or `revolve`",

View File

@ -5,7 +5,6 @@ use kcl_derive_docs::stdlib;
use kcmc::{each_cmd as mcmd, length_unit::LengthUnit, shared::CutType, ModelingCmd};
use kittycad_modeling_cmds as kcmc;
use super::utils::unique_count;
use crate::{
errors::{KclError, KclErrorDetails},
execution::{ChamferSurface, EdgeCut, ExecState, ExtrudeSurface, GeoMeta, KclValue, Solid},
@ -19,9 +18,11 @@ pub(crate) const DEFAULT_TOLERANCE: f64 = 0.0000001;
pub async fn chamfer(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
let solid = args.get_unlabeled_kw_arg("solid")?;
let length = args.get_kw_arg("length")?;
let tags = args.get_kw_arg("tags")?;
let tags = args.kw_arg_array_and_source::<EdgeReference>("tags")?;
let tag = args.get_kw_arg_opt("tag")?;
super::fillet::validate_unique(&tags)?;
let tags: Vec<EdgeReference> = tags.into_iter().map(|item| item.0).collect();
let value = inner_chamfer(solid, length, tags, tag, exec_state, args).await?;
Ok(KclValue::Solid { value })
}
@ -109,15 +110,6 @@ async fn inner_chamfer(
exec_state: &mut ExecState,
args: Args,
) -> Result<Box<Solid>, KclError> {
// Check if tags contains any duplicate values.
let unique_tags = unique_count(tags.clone());
if unique_tags != tags.len() {
return Err(KclError::Type(KclErrorDetails {
message: "Duplicate tags are not allowed.".to_string(),
source_ranges: vec![args.source_range],
}));
}
// If you try and tag multiple edges with a tagged chamfer, we want to return an
// error to the user that they can only tag one edge at a time.
if tag.is_some() && tags.len() > 1 {

View File

@ -1,6 +1,7 @@
//! Standard library fillets.
use anyhow::Result;
use indexmap::IndexMap;
use kcl_derive_docs::stdlib;
use kcmc::{
each_cmd as mcmd, length_unit::LengthUnit, ok_response::OkModelingCmdResponse, shared::CutType,
@ -11,13 +12,13 @@ use schemars::JsonSchema;
use serde::{Deserialize, Serialize};
use uuid::Uuid;
use super::utils::unique_count;
use crate::{
errors::{KclError, KclErrorDetails},
execution::{EdgeCut, ExecState, ExtrudeSurface, FilletSurface, GeoMeta, KclValue, Solid, TagIdentifier},
parsing::ast::types::TagNode,
settings::types::UnitLength,
std::Args,
SourceRange,
};
/// A tag or a uuid of an edge.
@ -40,13 +41,39 @@ impl EdgeReference {
}
}
pub(super) fn validate_unique<T: Eq + std::hash::Hash>(tags: &[(T, SourceRange)]) -> Result<(), KclError> {
// Check if tags contains any duplicate values.
let mut tag_counts: IndexMap<&T, Vec<SourceRange>> = Default::default();
for tag in tags {
tag_counts.entry(&tag.0).or_insert(Vec::new()).push(tag.1);
}
let mut duplicate_tags_source = Vec::new();
for (_tag, count) in tag_counts {
if count.len() > 1 {
duplicate_tags_source.extend(count)
}
}
if !duplicate_tags_source.is_empty() {
return Err(KclError::Type(KclErrorDetails {
message: "The same edge ID is being referenced multiple times, which is not allowed. Please select a different edge".to_string(),
source_ranges: duplicate_tags_source,
}));
}
Ok(())
}
/// Create fillets on tagged paths.
pub async fn fillet(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
// Get all args:
let solid = args.get_unlabeled_kw_arg("solid")?;
let radius = args.get_kw_arg("radius")?;
let tolerance = args.get_kw_arg_opt("tolerance")?;
let tags = args.get_kw_arg("tags")?;
let tags = args.kw_arg_array_and_source::<EdgeReference>("tags")?;
let tag = args.get_kw_arg_opt("tag")?;
// Run the function.
validate_unique(&tags)?;
let tags: Vec<EdgeReference> = tags.into_iter().map(|item| item.0).collect();
let value = inner_fillet(solid, radius, tags, tolerance, tag, exec_state, args).await?;
Ok(KclValue::Solid { value })
}
@ -129,15 +156,6 @@ async fn inner_fillet(
exec_state: &mut ExecState,
args: Args,
) -> Result<Box<Solid>, KclError> {
// Check if tags contains any duplicate values.
let unique_tags = unique_count(tags.clone());
if unique_tags != tags.len() {
return Err(KclError::Type(KclErrorDetails {
message: "Duplicate tags are not allowed.".to_string(),
source_ranges: vec![args.source_range],
}));
}
let mut solid = solid.clone();
for edge_tag in tags {
let edge_id = edge_tag.get_engine_id(exec_state, &args)?;
@ -432,3 +450,22 @@ pub(crate) fn default_tolerance(units: &UnitLength) -> f64 {
UnitLength::M => 0.001,
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_validate_unique() {
let dup_a = SourceRange::from([1, 3, 0]);
let dup_b = SourceRange::from([10, 30, 0]);
// Two entries are duplicates (abc) with different source ranges.
let tags = vec![("abc", dup_a), ("abc", dup_b), ("def", SourceRange::from([2, 4, 0]))];
let actual = validate_unique(&tags);
// Both the duplicates should show up as errors, with both of the
// source ranges they correspond to.
// But the unique source range 'def' should not.
let expected = vec![dup_a, dup_b];
assert_eq!(actual.err().unwrap().source_ranges(), expected);
}
}

View File

@ -1,4 +1,4 @@
use std::{collections::HashSet, f64::consts::PI};
use std::f64::consts::PI;
use kittycad_modeling_cmds::shared::Angle;
@ -8,16 +8,6 @@ use crate::{
source_range::SourceRange,
};
/// Count the number of unique items in a `Vec` in O(n) time.
pub(crate) fn unique_count<T: Eq + std::hash::Hash>(vec: Vec<T>) -> usize {
// Add to a set.
let mut set = HashSet::with_capacity(vec.len());
for item in vec {
set.insert(item);
}
set.len()
}
/// Get the distance between two points.
pub fn distance(a: Point2d, b: Point2d) -> f64 {
((b.x - a.x).powi(2) + (b.y - a.y).powi(2)).sqrt()
@ -686,11 +676,6 @@ mod get_tangential_arc_to_info_tests {
(num * 1000.0).round() / 1000.0
}
#[test]
fn test_unique_count() {
assert_eq!(unique_count(vec![1, 2, 2, 3, 2]), 3);
}
#[test]
fn test_basic_case() {
let result = get_tangential_arc_to_info(TangentialArcInfoInput {