Skip to content

Commit ce457a1

Browse files
nrcnrc
authored
Make warnings ignorable via settings (#7979)
Allow or deny warnings Signed-off-by: Nick Cameron <[email protected]>
1 parent 9f58521 commit ce457a1

File tree

10 files changed

+296
-112
lines changed

10 files changed

+296
-112
lines changed

‎public/kcl-samples/screenshots/spool.png

486 Bytes
Loading

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,25 @@ pub(crate) const IMPL_PRIMITIVE: &str = "primitive";
3333
pub(super) const IMPL_VALUES: [&str; 3] = [IMPL_RUST, IMPL_KCL, IMPL_PRIMITIVE];
3434

3535
pub(crate) const DEPRECATED: &str = "deprecated";
36+
pub(crate) const WARNINGS: &str = "warnings";
37+
pub(crate) const WARN_ALLOW: &str = "allow";
38+
pub(crate) const WARN_DENY: &str = "deny";
39+
pub(crate) const WARN_UNKNOWN_UNITS: &str = "unknownUnits";
40+
pub(crate) const WARN_UNKNOWN_ATTR: &str = "unknownAttribute";
41+
pub(crate) const WARN_MOD_RETURN_VALUE: &str = "moduleReturnValue";
42+
pub(crate) const WARN_DEPRECATED: &str = "deprecated";
43+
pub(crate) const WARN_EXPERIMENTAL: &str = "experimental";
44+
pub(crate) const WARN_IGNORED_Z_AXIS: &str = "ignoredZAxis";
45+
pub(crate) const WARN_UNNECESSARY_CLOSE: &str = "unnecessaryClose";
46+
pub(super) const WARN_VALUES: [&str; 7] = [
47+
WARN_UNKNOWN_UNITS,
48+
WARN_UNKNOWN_ATTR,
49+
WARN_MOD_RETURN_VALUE,
50+
WARN_DEPRECATED,
51+
WARN_EXPERIMENTAL,
52+
WARN_IGNORED_Z_AXIS,
53+
WARN_UNNECESSARY_CLOSE,
54+
];
3655

3756
#[derive(Clone, Copy, Eq, PartialEq, Debug, Default)]
3857
pub enum Impl {
@@ -92,6 +111,64 @@ pub(super) fn expect_ident(expr: &Expr) -> Result<&str, KclError> {
92111
)))
93112
}
94113

114+
pub(super) fn many_of(
115+
expr: &Expr,
116+
of: &[&'static str],
117+
source_range: SourceRange,
118+
) -> Result<Vec<&'static str>, KclError> {
119+
const UNEXPECTED_MSG: &str = "Unexpected warnings value, expected a name or array of names, e.g., `unknownUnits` or `[unknownUnits, deprecated]`";
120+
121+
let values = match expr {
122+
Expr::Name(name) => {
123+
if let Some(name) = name.local_ident() {
124+
vec![*name]
125+
} else {
126+
return Err(KclError::new_semantic(KclErrorDetails::new(
127+
UNEXPECTED_MSG.to_owned(),
128+
vec![expr.into()],
129+
)));
130+
}
131+
}
132+
Expr::ArrayExpression(e) => {
133+
let mut result = Vec::new();
134+
for e in &e.elements {
135+
if let Expr::Name(name) = e {
136+
if let Some(name) = name.local_ident() {
137+
result.push(*name);
138+
continue;
139+
}
140+
}
141+
return Err(KclError::new_semantic(KclErrorDetails::new(
142+
UNEXPECTED_MSG.to_owned(),
143+
vec![e.into()],
144+
)));
145+
}
146+
result
147+
}
148+
_ => {
149+
return Err(KclError::new_semantic(KclErrorDetails::new(
150+
UNEXPECTED_MSG.to_owned(),
151+
vec![expr.into()],
152+
)));
153+
}
154+
};
155+
156+
values
157+
.into_iter()
158+
.map(|v| {
159+
of.iter()
160+
.find(|vv| **vv == v)
161+
.ok_or_else(|| {
162+
KclError::new_semantic(KclErrorDetails::new(
163+
format!("Unexpected warning value: `{v}`; accepted values: {}", of.join(", "),),
164+
vec![source_range],
165+
))
166+
})
167+
.copied()
168+
})
169+
.collect::<Result<Vec<&str>, KclError>>()
170+
}
171+
95172
// Returns the unparsed number literal.
96173
pub(super) fn expect_number(expr: &Expr) -> Result<String, KclError> {
97174
if let Expr::Literal(lit) = expr {

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

Lines changed: 73 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,51 @@ impl ExecutorContext {
6565
"The standard library can only be skipped at the top level scope of a file",
6666
));
6767
}
68+
} else if annotation.name() == Some(annotations::WARNINGS) {
69+
// TODO we should support setting warnings for the whole project, not just one file
70+
if matches!(body_type, BodyType::Root) {
71+
let props = annotations::expect_properties(annotations::WARNINGS, annotation)?;
72+
for p in props {
73+
match &*p.inner.key.name {
74+
annotations::WARN_ALLOW => {
75+
let allowed = annotations::many_of(
76+
&p.inner.value,
77+
&annotations::WARN_VALUES,
78+
annotation.as_source_range(),
79+
)?;
80+
exec_state.mod_local.allowed_warnings = allowed;
81+
}
82+
annotations::WARN_DENY => {
83+
let denied = annotations::many_of(
84+
&p.inner.value,
85+
&annotations::WARN_VALUES,
86+
annotation.as_source_range(),
87+
)?;
88+
exec_state.mod_local.denied_warnings = denied;
89+
}
90+
name => {
91+
return Err(KclError::new_semantic(KclErrorDetails::new(
92+
format!(
93+
"Unexpected warnings key: `{name}`; expected one of `{}`, `{}`",
94+
annotations::WARN_ALLOW,
95+
annotations::WARN_DENY,
96+
),
97+
vec![annotation.as_source_range()],
98+
)));
99+
}
100+
}
101+
}
102+
} else {
103+
exec_state.err(CompilationError::err(
104+
annotation.as_source_range(),
105+
"Warnings can only be customized at the top level scope of a file",
106+
));
107+
}
68108
} else {
69-
exec_state.warn(CompilationError::err(
70-
annotation.as_source_range(),
71-
"Unknown annotation",
72-
));
109+
exec_state.warn(
110+
CompilationError::err(annotation.as_source_range(), "Unknown annotation"),
111+
annotations::WARN_UNKNOWN_ATTR,
112+
);
73113
}
74114
}
75115
Ok(no_prelude)
@@ -687,7 +727,8 @@ impl ExecutorContext {
687727
exec_state.warn(CompilationError::err(
688728
metadata.source_range,
689729
"Imported module has no return value. The last statement of the module must be an expression, usually the Solid.",
690-
));
730+
),
731+
annotations::WARN_MOD_RETURN_VALUE);
691732

692733
let mut new_meta = vec![metadata.to_owned()];
693734
new_meta.extend(meta);
@@ -1327,7 +1368,7 @@ impl Node<BinaryExpression> {
13271368
),
13281369
);
13291370
err.tag = crate::errors::Tag::UnknownNumericUnits;
1330-
exec_state.warn(err);
1371+
exec_state.warn(err, annotations::WARN_UNKNOWN_UNITS);
13311372
}
13321373
}
13331374
}
@@ -1816,6 +1857,7 @@ mod test {
18161857
use super::*;
18171858
use crate::{
18181859
ExecutorSettings, UnitLen,
1860+
errors::Severity,
18191861
exec::UnitType,
18201862
execution::{ContextType, parse_execute},
18211863
};
@@ -2250,4 +2292,29 @@ y = x[0mm + 1]
22502292

22512293
parse_execute(&ast).await.unwrap();
22522294
}
2295+
2296+
#[tokio::test(flavor = "multi_thread")]
2297+
async fn custom_warning() {
2298+
let warn = r#"
2299+
a = PI * 2
2300+
"#;
2301+
let result = parse_execute(warn).await.unwrap();
2302+
assert_eq!(result.exec_state.errors().len(), 1);
2303+
assert_eq!(result.exec_state.errors()[0].severity, Severity::Warning);
2304+
2305+
let allow = r#"
2306+
@warnings(allow = unknownUnits)
2307+
a = PI * 2
2308+
"#;
2309+
let result = parse_execute(allow).await.unwrap();
2310+
assert_eq!(result.exec_state.errors().len(), 0);
2311+
2312+
let deny = r#"
2313+
@warnings(deny = [unknownUnits])
2314+
a = PI * 2
2315+
"#;
2316+
let result = parse_execute(deny).await.unwrap();
2317+
assert_eq!(result.exec_state.errors().len(), 1);
2318+
assert_eq!(result.exec_state.errors()[0].severity, Severity::Error);
2319+
}
22532320
}

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
errors::{KclError, KclErrorDetails},
77
execution::{
88
BodyType, EnvironmentRef, ExecState, ExecutorContext, KclValue, Metadata, StatementKind, TagEngineInfo,
9-
TagIdentifier,
9+
TagIdentifier, annotations,
1010
cad_op::{Group, OpArg, OpKclValue, Operation},
1111
kcl_value::FunctionSource,
1212
memory,
@@ -290,16 +290,19 @@ impl FunctionDefinition<'_> {
290290
callsite: SourceRange,
291291
) -> Result<Option<KclValue>, KclError> {
292292
if self.deprecated {
293-
exec_state.warn(CompilationError::err(
294-
callsite,
295-
format!(
296-
"{} is deprecated, see the docs for a recommended replacement",
297-
match &fn_name {
298-
Some(n) => format!("`{n}`"),
299-
None => "This function".to_owned(),
300-
}
293+
exec_state.warn(
294+
CompilationError::err(
295+
callsite,
296+
format!(
297+
"{} is deprecated, see the docs for a recommended replacement",
298+
match &fn_name {
299+
Some(n) => format!("`{n}`"),
300+
None => "This function".to_owned(),
301+
}
302+
),
301303
),
302-
));
304+
annotations::WARN_DEPRECATED,
305+
);
303306
}
304307

305308
type_check_params_kw(fn_name.as_deref(), self, &mut args.kw_args, exec_state)?;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::{
1010
execution::{
1111
EnvironmentRef, ExecState, Face, Geometry, GeometryWithImportedGeometry, Helix, ImportedGeometry, MetaSettings,
1212
Metadata, Plane, Sketch, Solid, TagIdentifier,
13-
annotations::{SETTINGS, SETTINGS_UNIT_LENGTH},
13+
annotations::{self, SETTINGS, SETTINGS_UNIT_LENGTH},
1414
types::{NumericType, PrimitiveType, RuntimeType, UnitLen},
1515
},
1616
parsing::ast::types::{
@@ -377,6 +377,7 @@ impl KclValue {
377377
Some(SourceRange::new(0, 0, literal.module_id)),
378378
crate::errors::Tag::Deprecated,
379379
),
380+
annotations::WARN_DEPRECATED,
380381
);
381382
}
382383
}

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,9 @@ pub(super) struct ModuleState {
107107
pub(super) path: ModulePath,
108108
/// Artifacts for only this module.
109109
pub artifacts: ModuleArtifactState,
110+
111+
pub(super) allowed_warnings: Vec<&'static str>,
112+
pub(super) denied_warnings: Vec<&'static str>,
110113
}
111114

112115
impl ExecState {
@@ -132,8 +135,19 @@ impl ExecState {
132135
}
133136

134137
/// Log a warning.
135-
pub fn warn(&mut self, mut e: CompilationError) {
136-
e.severity = Severity::Warning;
138+
pub fn warn(&mut self, mut e: CompilationError, name: &'static str) {
139+
debug_assert!(annotations::WARN_VALUES.contains(&name));
140+
141+
if self.mod_local.allowed_warnings.contains(&name) {
142+
return;
143+
}
144+
145+
if self.mod_local.denied_warnings.contains(&name) {
146+
e.severity = Severity::Error;
147+
} else {
148+
e.severity = Severity::Warning;
149+
}
150+
137151
self.global.errors.push(e);
138152
}
139153

@@ -492,6 +506,8 @@ impl ModuleState {
492506
kcl_version: "0.1".to_owned(),
493507
},
494508
artifacts: Default::default(),
509+
allowed_warnings: Vec::new(),
510+
denied_warnings: Vec::new(),
495511
}
496512
}
497513

0 commit comments

Comments
 (0)