Allow or deny warnings

Signed-off-by: Nick Cameron <nrc@ncameron.org>
This commit is contained in:
Nick Cameron
2025-06-13 16:46:29 +12:00
parent fe581ff1d2
commit 8395869b2e
7 changed files with 228 additions and 64 deletions

View File

@ -33,6 +33,21 @@ pub(crate) const IMPL_PRIMITIVE: &str = "primitive";
pub(super) const IMPL_VALUES: [&str; 3] = [IMPL_RUST, IMPL_KCL, IMPL_PRIMITIVE]; pub(super) const IMPL_VALUES: [&str; 3] = [IMPL_RUST, IMPL_KCL, IMPL_PRIMITIVE];
pub(crate) const DEPRECATED: &str = "deprecated"; pub(crate) const DEPRECATED: &str = "deprecated";
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_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] = [
WARN_UNKNOWN_UNITS,
WARN_UNKNOWN_ATTR,
WARN_MOD_RETURN_VALUE,
WARN_DEPRECATED,
WARN_IGNORED_Z_AXIS,
];
#[derive(Clone, Copy, Eq, PartialEq, Debug, Default)] #[derive(Clone, Copy, Eq, PartialEq, Debug, Default)]
pub enum Impl { pub enum Impl {
@ -92,6 +107,64 @@ pub(super) fn expect_ident(expr: &Expr) -> Result<&str, KclError> {
))) )))
} }
pub(super) fn many_of(
expr: &Expr,
of: &[&'static str],
source_range: SourceRange,
) -> Result<Vec<&'static str>, KclError> {
const UNEXPECTED_MSG: &str = "Unexpected warnings value, expected a name or array of names, e.g., `unknownUnits` or `[unknownUnits, deprecated]`";
let values = match expr {
Expr::Name(name) => {
if let Some(name) = name.local_ident() {
vec![*name]
} else {
return Err(KclError::new_semantic(KclErrorDetails::new(
UNEXPECTED_MSG.to_owned(),
vec![expr.into()],
)));
}
}
Expr::ArrayExpression(e) => {
let mut result = Vec::new();
for e in &e.elements {
if let Expr::Name(name) = e {
if let Some(name) = name.local_ident() {
result.push(*name);
continue;
}
}
return Err(KclError::new_semantic(KclErrorDetails::new(
UNEXPECTED_MSG.to_owned(),
vec![e.into()],
)));
}
result
}
_ => {
return Err(KclError::new_semantic(KclErrorDetails::new(
UNEXPECTED_MSG.to_owned(),
vec![expr.into()],
)))
}
};
values
.into_iter()
.map(|v| {
of.iter()
.find(|vv| **vv == v)
.ok_or_else(|| {
KclError::new_semantic(KclErrorDetails::new(
format!("Unexpected warning value: `{v}`; accepted values: {}", of.join(", "),),
vec![source_range],
))
})
.map(|v| *v)
})
.collect::<Result<Vec<&str>, KclError>>()
}
// Returns the unparsed number literal. // Returns the unparsed number literal.
pub(super) fn expect_number(expr: &Expr) -> Result<String, KclError> { pub(super) fn expect_number(expr: &Expr) -> Result<String, KclError> {
if let Expr::Literal(lit) = expr { if let Expr::Literal(lit) = expr {

View File

@ -67,12 +67,52 @@ impl ExecutorContext {
"The standard library can only be skipped at the top level scope of a file", "The standard library can only be skipped at the top level scope of a file",
)); ));
} }
} else { } else if annotation.name() == Some(annotations::WARNINGS) {
exec_state.warn(CompilationError::err( // TODO we should support setting warnings for the whole project, not just one file
if matches!(body_type, BodyType::Root) {
let props = annotations::expect_properties(annotations::WARNINGS, annotation)?;
for p in props {
match &*p.inner.key.name {
annotations::WARN_ALLOW => {
let allowed = annotations::many_of(
&p.inner.value,
&annotations::WARN_VALUES,
annotation.as_source_range(), annotation.as_source_range(),
"Unknown annotation", )?;
exec_state.mod_local.allowed_warnings = allowed;
}
annotations::WARN_DENY => {
let denied = annotations::many_of(
&p.inner.value,
&annotations::WARN_VALUES,
annotation.as_source_range(),
)?;
exec_state.mod_local.denied_warnings = denied;
}
name => {
return Err(KclError::new_semantic(KclErrorDetails::new(
format!(
"Unexpected warnings key: `{name}`; expected one of `{}`, `{}`",
annotations::WARN_ALLOW,
annotations::WARN_DENY,
),
vec![annotation.as_source_range()],
)))
}
}
}
} else {
exec_state.err(CompilationError::err(
annotation.as_source_range(),
"Warnings can only be customized at the top level scope of a file",
)); ));
} }
} else {
exec_state.warn(
CompilationError::err(annotation.as_source_range(), "Unknown annotation"),
annotations::WARN_UNKNOWN_ATTR,
);
}
} }
Ok(no_prelude) Ok(no_prelude)
} }
@ -685,7 +725,8 @@ impl ExecutorContext {
exec_state.warn(CompilationError::err( exec_state.warn(CompilationError::err(
metadata.source_range, metadata.source_range,
"Imported module has no return value. The last statement of the module must be an expression, usually the Solid.", "Imported module has no return value. The last statement of the module must be an expression, usually the Solid.",
)); ),
annotations::WARN_MOD_RETURN_VALUE);
let mut new_meta = vec![metadata.to_owned()]; let mut new_meta = vec![metadata.to_owned()];
new_meta.extend(meta); new_meta.extend(meta);
@ -1237,7 +1278,7 @@ impl Node<BinaryExpression> {
format!("{} numbers which have unknown or incompatible units.\nYou can probably fix this error by specifying the units using type ascription, e.g., `len: number(mm)` or `(a * b): number(deg)`.", verb), format!("{} numbers which have unknown or incompatible units.\nYou can probably fix this error by specifying the units using type ascription, e.g., `len: number(mm)` or `(a * b): number(deg)`.", verb),
); );
err.tag = crate::errors::Tag::UnknownNumericUnits; err.tag = crate::errors::Tag::UnknownNumericUnits;
exec_state.warn(err); exec_state.warn(err, annotations::WARN_UNKNOWN_UNITS);
} }
} }
} }
@ -1728,11 +1769,11 @@ impl Node<PipeExpression> {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use std::sync::Arc; use std::sync::Arc;
use tokio::io::AsyncWriteExt; use tokio::io::AsyncWriteExt;
use super::*; use super::*;
use crate::{ use crate::{
errors::Severity,
exec::UnitType, exec::UnitType,
execution::{parse_execute, ContextType}, execution::{parse_execute, ContextType},
ExecutorSettings, UnitLen, ExecutorSettings, UnitLen,
@ -2141,4 +2182,29 @@ c = ((PI * 2) / 3): number(deg)
let result = parse_execute(ast).await.unwrap(); let result = parse_execute(ast).await.unwrap();
assert_eq!(result.exec_state.errors().len(), 2); assert_eq!(result.exec_state.errors().len(), 2);
} }
#[tokio::test(flavor = "multi_thread")]
async fn custom_warning() {
let warn = r#"
a = PI * 2
"#;
let result = parse_execute(warn).await.unwrap();
assert_eq!(result.exec_state.errors().len(), 1);
assert_eq!(result.exec_state.errors()[0].severity, Severity::Warning);
let allow = r#"
@warnings(allow = unknownUnits)
a = PI * 2
"#;
let result = parse_execute(allow).await.unwrap();
assert_eq!(result.exec_state.errors().len(), 0);
let deny = r#"
@warnings(deny = [unknownUnits])
a = PI * 2
"#;
let result = parse_execute(deny).await.unwrap();
assert_eq!(result.exec_state.errors().len(), 1);
assert_eq!(result.exec_state.errors()[0].severity, Severity::Error);
}
} }

View File

@ -4,6 +4,7 @@ use indexmap::IndexMap;
use crate::{ use crate::{
errors::{KclError, KclErrorDetails}, errors::{KclError, KclErrorDetails},
execution::{ execution::{
annotations,
cad_op::{Group, OpArg, OpKclValue, Operation}, cad_op::{Group, OpArg, OpKclValue, Operation},
kcl_value::FunctionSource, kcl_value::FunctionSource,
memory, memory,
@ -290,7 +291,8 @@ impl FunctionDefinition<'_> {
callsite: SourceRange, callsite: SourceRange,
) -> Result<Option<KclValue>, KclError> { ) -> Result<Option<KclValue>, KclError> {
if self.deprecated { if self.deprecated {
exec_state.warn(CompilationError::err( exec_state.warn(
CompilationError::err(
callsite, callsite,
format!( format!(
"{} is deprecated, see the docs for a recommended replacement", "{} is deprecated, see the docs for a recommended replacement",
@ -299,7 +301,9 @@ impl FunctionDefinition<'_> {
None => "This function".to_owned(), None => "This function".to_owned(),
} }
), ),
)); ),
annotations::WARN_DEPRECATED,
);
} }
type_check_params_kw(fn_name.as_deref(), self, &mut args.kw_args, exec_state)?; type_check_params_kw(fn_name.as_deref(), self, &mut args.kw_args, exec_state)?;

View File

@ -7,7 +7,7 @@ use serde::Serialize;
use crate::{ use crate::{
errors::KclErrorDetails, errors::KclErrorDetails,
execution::{ execution::{
annotations::{SETTINGS, SETTINGS_UNIT_LENGTH}, annotations::{self, SETTINGS, SETTINGS_UNIT_LENGTH},
types::{NumericType, PrimitiveType, RuntimeType, UnitLen}, types::{NumericType, PrimitiveType, RuntimeType, UnitLen},
EnvironmentRef, ExecState, Face, Geometry, GeometryWithImportedGeometry, Helix, ImportedGeometry, MetaSettings, EnvironmentRef, ExecState, Face, Geometry, GeometryWithImportedGeometry, Helix, ImportedGeometry, MetaSettings,
Metadata, Plane, Sketch, Solid, TagIdentifier, Metadata, Plane, Sketch, Solid, TagIdentifier,
@ -377,6 +377,7 @@ impl KclValue {
Some(SourceRange::new(0, 0, literal.module_id)), Some(SourceRange::new(0, 0, literal.module_id)),
crate::errors::Tag::Deprecated, crate::errors::Tag::Deprecated,
), ),
annotations::WARN_DEPRECATED,
); );
} }
} }

View File

@ -108,6 +108,9 @@ pub(super) struct ModuleState {
pub(super) path: ModulePath, pub(super) path: ModulePath,
/// Artifacts for only this module. /// Artifacts for only this module.
pub artifacts: ModuleArtifactState, pub artifacts: ModuleArtifactState,
pub(super) allowed_warnings: Vec<&'static str>,
pub(super) denied_warnings: Vec<&'static str>,
} }
impl ExecState { impl ExecState {
@ -133,8 +136,19 @@ impl ExecState {
} }
/// Log a warning. /// Log a warning.
pub fn warn(&mut self, mut e: CompilationError) { pub fn warn(&mut self, mut e: CompilationError, name: &'static str) {
debug_assert!(annotations::WARN_VALUES.contains(&name));
if self.mod_local.allowed_warnings.contains(&name) {
return;
}
if self.mod_local.denied_warnings.contains(&name) {
e.severity = Severity::Error;
} else {
e.severity = Severity::Warning; e.severity = Severity::Warning;
}
self.global.errors.push(e); self.global.errors.push(e);
} }
@ -502,6 +516,8 @@ impl ModuleState {
kcl_version: "0.1".to_owned(), kcl_version: "0.1".to_owned(),
}, },
artifacts: Default::default(), artifacts: Default::default(),
allowed_warnings: Vec::new(),
denied_warnings: Vec::new(),
} }
} }

View File

@ -6,6 +6,7 @@ use serde::{Deserialize, Serialize};
use crate::{ use crate::{
execution::{ execution::{
annotations,
kcl_value::{KclValue, TypeDef}, kcl_value::{KclValue, TypeDef},
memory::{self}, memory::{self},
ExecState, Plane, PlaneInfo, Point3d, ExecState, Plane, PlaneInfo, Point3d,
@ -1147,7 +1148,8 @@ impl KclValue {
KclValue::Solid { .. } => Ok(self.clone()), KclValue::Solid { .. } => Ok(self.clone()),
_ => Err(self.into()), _ => Err(self.into()),
}, },
PrimitiveType::Plane => match self { PrimitiveType::Plane => {
match self {
KclValue::String { value: s, .. } KclValue::String { value: s, .. }
if [ if [
"xy", "xz", "yz", "-xy", "-xz", "-yz", "XY", "XZ", "YZ", "-XY", "-XZ", "-YZ", "xy", "xz", "yz", "-xy", "-xz", "-yz", "XY", "XZ", "YZ", "-XY", "-XZ", "-YZ",
@ -1175,7 +1177,7 @@ impl KclValue {
exec_state.warn(CompilationError::err( exec_state.warn(CompilationError::err(
self.into(), self.into(),
"Object with a zAxis field is being coerced into a plane, but the zAxis is ignored.", "Object with a zAxis field is being coerced into a plane, but the zAxis is ignored.",
)); ), annotations::WARN_IGNORED_Z_AXIS);
} }
let id = exec_state.mod_local.id_generator.next_uuid(); let id = exec_state.mod_local.id_generator.next_uuid();
@ -1194,7 +1196,8 @@ impl KclValue {
Ok(KclValue::Plane { value: Box::new(plane) }) Ok(KclValue::Plane { value: Box::new(plane) })
} }
_ => Err(self.into()), _ => Err(self.into()),
}, }
}
PrimitiveType::Face => match self { PrimitiveType::Face => match self {
KclValue::Face { .. } => Ok(self.clone()), KclValue::Face { .. } => Ok(self.clone()),
_ => Err(self.into()), _ => Err(self.into()),

View File

@ -5,6 +5,7 @@ use anyhow::Result;
use crate::{ use crate::{
errors::{KclError, KclErrorDetails}, errors::{KclError, KclErrorDetails},
execution::{ execution::{
annotations,
types::{ArrayLen, NumericType, RuntimeType}, types::{ArrayLen, NumericType, RuntimeType},
ExecState, KclValue, ExecState, KclValue,
}, },
@ -114,7 +115,7 @@ pub async fn min(exec_state: &mut ExecState, args: Args) -> Result<KclValue, Kcl
exec_state.warn(CompilationError::err( exec_state.warn(CompilationError::err(
args.source_range, args.source_range,
"Calling `min` on numbers which have unknown or incompatible units.\n\nYou may need to add information about the type of the argument, for example:\n using a numeric suffix: `42{ty}`\n or using type ascription: `foo(): number({ty})`", "Calling `min` on numbers which have unknown or incompatible units.\n\nYou may need to add information about the type of the argument, for example:\n using a numeric suffix: `42{ty}`\n or using type ascription: `foo(): number({ty})`",
)); ), annotations::WARN_UNKNOWN_UNITS);
} }
let mut result = f64::MAX; let mut result = f64::MAX;
@ -139,7 +140,7 @@ pub async fn max(exec_state: &mut ExecState, args: Args) -> Result<KclValue, Kcl
exec_state.warn(CompilationError::err( exec_state.warn(CompilationError::err(
args.source_range, args.source_range,
"Calling `max` on numbers which have unknown or incompatible units.\n\nYou may need to add information about the type of the argument, for example:\n using a numeric suffix: `42{ty}`\n or using type ascription: `foo(): number({ty})`", "Calling `max` on numbers which have unknown or incompatible units.\n\nYou may need to add information about the type of the argument, for example:\n using a numeric suffix: `42{ty}`\n or using type ascription: `foo(): number({ty})`",
)); ), annotations::WARN_UNKNOWN_UNITS);
} }
let mut result = f64::MIN; let mut result = f64::MIN;