Warn on inferred angle units

Signed-off-by: Nick Cameron <nrc@ncameron.org>

rust/kcl-lib/src/execution/annotations.rs

@@ -37,12 +37,14 @@ pub(crate) const WARNINGS: &str = "warnings";
 pub(crate) const WARN_ALLOW: &str = "allow";
 pub(crate) const WARN_DENY: &str = "deny";
 pub(crate) const WARN_UNKNOWN_UNITS: &str = "unknownUnits";
+pub(crate) const WARN_ANGLE_UNITS: &str = "angleUnits";
 pub(crate) const WARN_UNKNOWN_ATTR: &str = "unknownAttribute";
 pub(crate) const WARN_MOD_RETURN_VALUE: &str = "moduleReturnValue";
 pub(crate) const WARN_DEPRECATED: &str = "deprecated";
 pub(crate) const WARN_IGNORED_Z_AXIS: &str = "ignoredZAxis";
-pub(super) const WARN_VALUES: [&str; 5] = [
+pub(super) const WARN_VALUES: [&str; 6] = [
     WARN_UNKNOWN_UNITS,
+    WARN_ANGLE_UNITS,
     WARN_UNKNOWN_ATTR,
     WARN_MOD_RETURN_VALUE,
     WARN_DEPRECATED,

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

@@ -49,9 +49,20 @@ impl ExecutorContext {
         for annotation in annotations {
             if annotation.name() == Some(annotations::SETTINGS) {
                 if matches!(body_type, BodyType::Root) {
-                    if exec_state.mod_local.settings.update_from_annotation(annotation)? {
+                    let (updated_len, updated_angle) =
+                        exec_state.mod_local.settings.update_from_annotation(annotation)?;
+                    if updated_len {
                         exec_state.mod_local.explicit_length_units = true;
                     }
+                    if updated_angle {
+                        exec_state.warn(
+                            CompilationError::err(
+                                annotation.as_source_range(),
+                                "Prefer to use explicit units for angles",
+                            ),
+                            annotations::WARN_ANGLE_UNITS,
+                        );
+                    }
                 } else {
                     exec_state.err(CompilationError::err(
                         annotation.as_source_range(),
@@ -1204,12 +1215,12 @@ impl Node<BinaryExpression> {
 
         let value = match self.operator {
             BinaryOperator::Add => {
-                let (l, r, ty) = NumericType::combine_eq_coerce(left, right);
+                let (l, r, ty) = NumericType::combine_eq_coerce(left, right, None);
                 self.warn_on_unknown(&ty, "Adding", exec_state);
                 KclValue::Number { value: l + r, meta, ty }
             }
             BinaryOperator::Sub => {
-                let (l, r, ty) = NumericType::combine_eq_coerce(left, right);
+                let (l, r, ty) = NumericType::combine_eq_coerce(left, right, None);
                 self.warn_on_unknown(&ty, "Subtracting", exec_state);
                 KclValue::Number { value: l - r, meta, ty }
             }
@@ -1234,32 +1245,32 @@ impl Node<BinaryExpression> {
                 ty: exec_state.current_default_units(),
             },
             BinaryOperator::Neq => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l != r, meta }
             }
             BinaryOperator::Gt => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l > r, meta }
             }
             BinaryOperator::Gte => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l >= r, meta }
             }
             BinaryOperator::Lt => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l < r, meta }
             }
             BinaryOperator::Lte => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l <= r, meta }
             }
             BinaryOperator::Eq => {
-                let (l, r, ty) = NumericType::combine_eq(left, right);
+                let (l, r, ty) = NumericType::combine_eq(left, right, exec_state, self.as_source_range());
                 self.warn_on_unknown(&ty, "Comparing", exec_state);
                 KclValue::Bool { value: l == r, meta }
             }

rust/kcl-lib/src/execution/state.rs

@@ -542,10 +542,11 @@ impl MetaSettings {
     pub(crate) fn update_from_annotation(
         &mut self,
         annotation: &crate::parsing::ast::types::Node<Annotation>,
-    ) -> Result<bool, KclError> {
+    ) -> Result<(bool, bool), KclError> {
         let properties = annotations::expect_properties(annotations::SETTINGS, annotation)?;
 
         let mut updated_len = false;
+        let mut updated_angle = false;
         for p in properties {
             match &*p.inner.key.name {
                 annotations::SETTINGS_UNIT_LENGTH => {
@@ -558,6 +559,7 @@ impl MetaSettings {
                     let value = annotations::expect_ident(&p.inner.value)?;
                     let value = types::UnitAngle::from_str(value, annotation.as_source_range())?;
                     self.default_angle_units = value;
+                    updated_angle = true;
                 }
                 annotations::SETTINGS_VERSION => {
                     let value = annotations::expect_number(&p.inner.value)?;
@@ -576,6 +578,6 @@ impl MetaSettings {
             }
         }
 
-        Ok(updated_len)
+        Ok((updated_len, updated_angle))
     }
 }

rust/kcl-lib/src/execution/types.rs

@@ -506,7 +506,12 @@ impl NumericType {
     ///
     /// This combinator function is suitable for comparisons where uncertainty should
     /// be handled by the user.
-    pub fn combine_eq(a: TyF64, b: TyF64) -> (f64, f64, NumericType) {
+    pub fn combine_eq(
+        a: TyF64,
+        b: TyF64,
+        exec_state: &mut ExecState,
+        source_range: SourceRange,
+    ) -> (f64, f64, NumericType) {
         use NumericType::*;
         match (a.ty, b.ty) {
             (at, bt) if at == bt => (a.n, b.n, at),
@@ -521,8 +526,24 @@ impl NumericType {
             }
             (t @ Known(UnitType::Length(l1)), Default { len: l2, .. }) if l1 == l2 => (a.n, b.n, t),
             (Default { len: l1, .. }, t @ Known(UnitType::Length(l2))) if l1 == l2 => (a.n, b.n, t),
-            (t @ Known(UnitType::Angle(a1)), Default { angle: a2, .. }) if a1 == a2 => (a.n, b.n, t),
+            (t @ Known(UnitType::Angle(a1)), Default { angle: a2, .. }) if a1 == a2 => {
-            (Default { angle: a1, .. }, t @ Known(UnitType::Angle(a2))) if a1 == a2 => (a.n, b.n, t),
+                if b.n != 0.0 {
+                    exec_state.warn(
+                        CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                        annotations::WARN_ANGLE_UNITS,
+                    );
+                }
+                (a.n, b.n, t)
+            }
+            (Default { angle: a1, .. }, t @ Known(UnitType::Angle(a2))) if a1 == a2 => {
+                if a.n != 0.0 {
+                    exec_state.warn(
+                        CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                        annotations::WARN_ANGLE_UNITS,
+                    );
+                }
+                (a.n, b.n, t)
+            }
 
             _ => (a.n, b.n, Unknown),
         }
@@ -535,7 +556,11 @@ impl NumericType {
     /// coerced together, for example two arguments to the same function or two numbers in an array being used as a point.
     ///
     /// Prefer to use `combine_eq` if possible since using that prioritises correctness over ergonomics.
-    pub fn combine_eq_coerce(a: TyF64, b: TyF64) -> (f64, f64, NumericType) {
+    pub fn combine_eq_coerce(
+        a: TyF64,
+        b: TyF64,
+        for_errs: Option<(&mut ExecState, SourceRange)>,
+    ) -> (f64, f64, NumericType) {
         use NumericType::*;
         match (a.ty, b.ty) {
             (at, bt) if at == bt => (a.n, b.n, at),
@@ -553,8 +578,28 @@ impl NumericType {
 
             (t @ Known(UnitType::Length(l1)), Default { len: l2, .. }) => (a.n, l2.adjust_to(b.n, l1).0, t),
             (Default { len: l1, .. }, t @ Known(UnitType::Length(l2))) => (l1.adjust_to(a.n, l2).0, b.n, t),
-            (t @ Known(UnitType::Angle(a1)), Default { angle: a2, .. }) => (a.n, a2.adjust_to(b.n, a1).0, t),
+            (t @ Known(UnitType::Angle(a1)), Default { angle: a2, .. }) => {
-            (Default { angle: a1, .. }, t @ Known(UnitType::Angle(a2))) => (a1.adjust_to(a.n, a2).0, b.n, t),
+                if let Some((exec_state, source_range)) = for_errs {
+                    if b.n != 0.0 {
+                        exec_state.warn(
+                            CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                            annotations::WARN_ANGLE_UNITS,
+                        );
+                    }
+                }
+                (a.n, a2.adjust_to(b.n, a1).0, t)
+            }
+            (Default { angle: a1, .. }, t @ Known(UnitType::Angle(a2))) => {
+                if let Some((exec_state, source_range)) = for_errs {
+                    if a.n != 0.0 {
+                        exec_state.warn(
+                            CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                            annotations::WARN_ANGLE_UNITS,
+                        );
+                    }
+                }
+                (a1.adjust_to(a.n, a2).0, b.n, t)
+            }
 
             (Known(_), Known(_)) | (Default { .. }, Default { .. }) | (_, Unknown) | (Unknown, _) => {
                 (a.n, b.n, Unknown)
@@ -2350,14 +2395,18 @@ b = 180 / PI * a + 360
     #[tokio::test(flavor = "multi_thread")]
     async fn cos_coercions() {
         let program = r#"
-a = cos(units::toRadians(30))
+a = cos(units::toRadians(30deg))
 b = 3 / a
 c = cos(30deg)
-d = cos(30)
+d = cos(1rad)
 "#;
 
         let result = parse_execute(program).await.unwrap();
-        assert!(result.exec_state.errors().is_empty());
+        assert!(
+            result.exec_state.errors().is_empty(),
+            "{:?}",
+            result.exec_state.errors()
+        );
 
         assert_value_and_type("a", &result, 1.0, NumericType::default());
         assert_value_and_type("b", &result, 3.0, NumericType::default());

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

@@ -9,6 +9,7 @@ pub use crate::execution::fn_call::Args;
 use crate::{
     errors::{KclError, KclErrorDetails},
     execution::{
+        annotations,
         kcl_value::FunctionSource,
         types::{NumericType, PrimitiveType, RuntimeType, UnitAngle, UnitLen, UnitType},
         ExecState, ExtrudeSurface, Helix, KclObjectFields, KclValue, Metadata, PlaneInfo, Sketch, SketchSurface, Solid,
@@ -21,7 +22,7 @@ use crate::{
         sketch::FaceTag,
         sweep::SweepPath,
     },
-    ModuleId,
+    CompilationError, ModuleId,
 };
 
 const ERROR_STRING_SKETCH_TO_SOLID_HELPER: &str =
@@ -56,9 +57,17 @@ impl TyF64 {
         len.adjust_to(self.n, units).0
     }
 
-    pub fn to_degrees(&self) -> f64 {
+    pub fn to_degrees(&self, exec_state: &mut ExecState, source_range: SourceRange) -> f64 {
         let angle = match self.ty {
-            NumericType::Default { angle, .. } => angle,
+            NumericType::Default { angle, .. } => {
+                if self.n != 0.0 {
+                    exec_state.warn(
+                        CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                        annotations::WARN_ANGLE_UNITS,
+                    );
+                }
+                angle
+            }
             NumericType::Known(UnitType::Angle(angle)) => angle,
             _ => unreachable!(),
         };
@@ -68,9 +77,17 @@ impl TyF64 {
         angle.adjust_to(self.n, UnitAngle::Degrees).0
     }
 
-    pub fn to_radians(&self) -> f64 {
+    pub fn to_radians(&self, exec_state: &mut ExecState, source_range: SourceRange) -> f64 {
         let angle = match self.ty {
-            NumericType::Default { angle, .. } => angle,
+            NumericType::Default { angle, .. } => {
+                if self.n != 0.0 {
+                    exec_state.warn(
+                        CompilationError::err(source_range, "Prefer to use explicit units for angles"),
+                        annotations::WARN_ANGLE_UNITS,
+                    );
+                }
+                angle
+            }
             NumericType::Known(UnitType::Angle(angle)) => angle,
             _ => unreachable!(),
         };

rust/kcl-lib/src/std/math.rs

@@ -34,21 +34,21 @@ pub async fn rem(exec_state: &mut ExecState, args: Args) -> Result<KclValue, Kcl
 /// Compute the cosine of a number (in radians).
 pub async fn cos(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let num: TyF64 = args.get_unlabeled_kw_arg("input", &RuntimeType::angle(), exec_state)?;
-    let num = num.to_radians();
+    let num = num.to_radians(exec_state, args.source_range);
     Ok(args.make_user_val_from_f64_with_type(TyF64::new(libm::cos(num), exec_state.current_default_units())))
 }
 
 /// Compute the sine of a number (in radians).
 pub async fn sin(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let num: TyF64 = args.get_unlabeled_kw_arg("input", &RuntimeType::angle(), exec_state)?;
-    let num = num.to_radians();
+    let num = num.to_radians(exec_state, args.source_range);
     Ok(args.make_user_val_from_f64_with_type(TyF64::new(libm::sin(num), exec_state.current_default_units())))
 }
 
 /// Compute the tangent of a number (in radians).
 pub async fn tan(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let num: TyF64 = args.get_unlabeled_kw_arg("input", &RuntimeType::angle(), exec_state)?;
-    let num = num.to_radians();
+    let num = num.to_radians(exec_state, args.source_range);
     Ok(args.make_user_val_from_f64_with_type(TyF64::new(libm::tan(num), exec_state.current_default_units())))
 }
 
@@ -190,7 +190,7 @@ pub async fn atan(exec_state: &mut ExecState, args: Args) -> Result<KclValue, Kc
 pub async fn atan2(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let y = args.get_kw_arg("y", &RuntimeType::length(), exec_state)?;
     let x = args.get_kw_arg("x", &RuntimeType::length(), exec_state)?;
-    let (y, x, _) = NumericType::combine_eq_coerce(y, x);
+    let (y, x, _) = NumericType::combine_eq_coerce(y, x, Some((exec_state, args.source_range)));
     let result = libm::atan2(y, x);
 
     Ok(args.make_user_val_from_f64_with_type(TyF64::new(result, NumericType::radians())))
@@ -237,7 +237,7 @@ pub async fn ln(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclE
 pub async fn leg_length(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let hypotenuse: TyF64 = args.get_kw_arg("hypotenuse", &RuntimeType::length(), exec_state)?;
     let leg: TyF64 = args.get_kw_arg("leg", &RuntimeType::length(), exec_state)?;
-    let (hypotenuse, leg, ty) = NumericType::combine_eq_coerce(hypotenuse, leg);
+    let (hypotenuse, leg, ty) = NumericType::combine_eq_coerce(hypotenuse, leg, Some((exec_state, args.source_range)));
     let result = (hypotenuse.powi(2) - f64::min(hypotenuse.abs(), leg.abs()).powi(2)).sqrt();
     Ok(KclValue::from_number_with_type(result, ty, vec![args.into()]))
 }
@@ -246,7 +246,7 @@ pub async fn leg_length(exec_state: &mut ExecState, args: Args) -> Result<KclVal
 pub async fn leg_angle_x(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let hypotenuse: TyF64 = args.get_kw_arg("hypotenuse", &RuntimeType::length(), exec_state)?;
     let leg: TyF64 = args.get_kw_arg("leg", &RuntimeType::length(), exec_state)?;
-    let (hypotenuse, leg, _ty) = NumericType::combine_eq_coerce(hypotenuse, leg);
+    let (hypotenuse, leg, _ty) = NumericType::combine_eq_coerce(hypotenuse, leg, Some((exec_state, args.source_range)));
     let result = libm::acos(leg.min(hypotenuse) / hypotenuse).to_degrees();
     Ok(KclValue::from_number_with_type(
         result,
@@ -259,7 +259,7 @@ pub async fn leg_angle_x(exec_state: &mut ExecState, args: Args) -> Result<KclVa
 pub async fn leg_angle_y(exec_state: &mut ExecState, args: Args) -> Result<KclValue, KclError> {
     let hypotenuse: TyF64 = args.get_kw_arg("hypotenuse", &RuntimeType::length(), exec_state)?;
     let leg: TyF64 = args.get_kw_arg("leg", &RuntimeType::length(), exec_state)?;
-    let (hypotenuse, leg, _ty) = NumericType::combine_eq_coerce(hypotenuse, leg);
+    let (hypotenuse, leg, _ty) = NumericType::combine_eq_coerce(hypotenuse, leg, Some((exec_state, args.source_range)));
     let result = libm::asin(leg.min(hypotenuse) / hypotenuse).to_degrees();
     Ok(KclValue::from_number_with_type(
         result,

rust/kcl-lib/src/std/sketch.rs

@@ -132,6 +132,8 @@ async fn inner_involute_circular(
     args: Args,
 ) -> Result<Sketch, KclError> {
     let id = exec_state.next_uuid();
+    let angle_deg = angle.to_degrees(exec_state, args.source_range);
+    let angle_rad = angle.to_radians(exec_state, args.source_range);
 
     exec_state
         .batch_modeling_cmd(
@@ -141,7 +143,7 @@ async fn inner_involute_circular(
                 segment: PathSegment::CircularInvolute {
                     start_radius: LengthUnit(start_radius.to_mm()),
                     end_radius: LengthUnit(end_radius.to_mm()),
-                    angle: Angle::from_degrees(angle.to_degrees()),
+                    angle: Angle::from_degrees(angle_deg),
                     reverse: reverse.unwrap_or_default(),
                 },
             }),
@@ -157,11 +159,11 @@ async fn inner_involute_circular(
     let theta = f64::sqrt(end_radius * end_radius - start_radius * start_radius) / start_radius;
     let (x, y) = involute_curve(start_radius, theta);
 
-    end.x = x * libm::cos(angle.to_radians()) - y * libm::sin(angle.to_radians());
+    end.x = x * libm::cos(angle_rad) - y * libm::sin(angle_rad);
-    end.y = x * libm::sin(angle.to_radians()) + y * libm::cos(angle.to_radians());
+    end.y = x * libm::sin(angle_rad) + y * libm::cos(angle_rad);
 
-    end.x -= start_radius * libm::cos(angle.to_radians());
+    end.x -= start_radius * libm::cos(angle_rad);
-    end.y -= start_radius * libm::sin(angle.to_radians());
+    end.y -= start_radius * libm::sin(angle_rad);
 
     if reverse.unwrap_or_default() {
         end.x = -end.x;
@@ -718,7 +720,7 @@ pub async fn inner_angled_line_that_intersects(
             point_to_len_unit(path.get_to(), from.units),
         ],
         offset.map(|t| t.to_length_units(from.units)).unwrap_or_default(),
-        angle.to_degrees(),
+        angle.to_degrees(exec_state, args.source_range),
         from.ignore_units(),
     );
     let to = [
@@ -1256,8 +1258,8 @@ pub async fn relative_arc(
     radius: TyF64,
     tag: Option<TagNode>,
 ) -> Result<Sketch, KclError> {
-    let a_start = Angle::from_degrees(angle_start.to_degrees());
+    let a_start = Angle::from_degrees(angle_start.to_degrees(exec_state, args.source_range));
-    let a_end = Angle::from_degrees(angle_end.to_degrees());
+    let a_end = Angle::from_degrees(angle_end.to_degrees(exec_state, args.source_range));
     let radius = radius.to_length_units(from.units);
     let (center, end) = arc_center_and_end(from.ignore_units(), a_start, a_end, radius);
     if a_start == a_end {
@@ -1409,7 +1411,7 @@ async fn inner_tangential_arc_radius_angle(
     let (center, to, ccw) = match data {
         TangentialArcData::RadiusAndOffset { radius, offset } => {
             // KCL stdlib types use degrees.
-            let offset = Angle::from_degrees(offset.to_degrees());
+            let offset = Angle::from_degrees(offset.to_degrees(exec_state, args.source_range));
 
             // Calculate the end point from the angle and radius.
             // atan2 outputs radians.

rust/kcl-lib/src/std/utils.rs

@@ -6,7 +6,7 @@ use super::args::TyF64;
 use crate::execution::types::{NumericType, UnitLen};
 
 pub(crate) fn untype_point(p: [TyF64; 2]) -> ([f64; 2], NumericType) {
-    let (x, y, ty) = NumericType::combine_eq_coerce(p[0].clone(), p[1].clone());
+    let (x, y, ty) = NumericType::combine_eq_coerce(p[0].clone(), p[1].clone(), None);
     ([x, y], ty)
 }