Support warning and non-fatal errors when executing (#5431)

* Support warning and non-fatal errors when executing

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

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

* A snapshot a day keeps the bugs away! 📷🐛 (OS: namespace-profile-ubuntu-8-cores)

---------

Signed-off-by: Nick Cameron <nrc@ncameron.org>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
This commit is contained in:
Nick Cameron
2025-02-21 09:30:44 +13:00
committed by GitHub
parent 30029a63a1
commit 21efb2c0bd
7 changed files with 97 additions and 53 deletions

View File

@ -401,6 +401,10 @@ export class KclManager {
this.errors = errors
// Do not add the errors since the program was interrupted and the error is not a real KCL error
this.addDiagnostics(isInterrupted ? [] : kclErrorsToDiagnostics(errors))
// Add warnings and non-fatal errors
this.addDiagnostics(
isInterrupted ? [] : complilationErrorsToDiagnostics(execState.errors)
)
this.execState = execState
if (!errors.length) {
this.lastSuccessfulVariables = execState.variables
@ -464,6 +468,8 @@ export class KclManager {
this._logs = logs
this.addDiagnostics(kclErrorsToDiagnostics(errors))
// Add warnings and non-fatal errors
this.addDiagnostics(complilationErrorsToDiagnostics(execState.errors))
this._execState = execState
this._variables = execState.variables

View File

@ -295,6 +295,7 @@ export interface ExecState {
artifacts: { [key in ArtifactId]?: RustArtifact }
artifactCommands: ArtifactCommand[]
artifactGraph: ArtifactGraph
errors: CompilationError[]
}
/**
@ -308,6 +309,7 @@ export function emptyExecState(): ExecState {
artifacts: {},
artifactCommands: [],
artifactGraph: defaultArtifactGraph(),
errors: [],
}
}
@ -333,6 +335,7 @@ function execStateFromRust(
artifacts: execOutcome.artifacts,
artifactCommands: execOutcome.artifactCommands,
artifactGraph,
errors: execOutcome.errors,
}
}
@ -343,6 +346,7 @@ function mockExecStateFromRust(execOutcome: RustExecOutcome): ExecState {
artifacts: execOutcome.artifacts,
artifactCommands: execOutcome.artifactCommands,
artifactGraph: new Map<ArtifactId, Artifact>(),
errors: execOutcome.errors,
}
}

View File

@ -24,11 +24,6 @@ pub(super) const IMPORT_COORDS_VALUES: [(&str, &System); 3] =
[("zoo", KITTYCAD), ("opengl", OPENGL), ("vulkan", VULKAN)];
pub(super) const IMPORT_LENGTH_UNIT: &str = "lengthUnit";
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub(super) enum AnnotationScope {
Module,
}
pub(super) fn is_significant(attr: &&Node<Annotation>) -> bool {
match attr.name() {
Some(name) => SIGNIFICANT_ATTRS.contains(&name),

View File

@ -26,6 +26,7 @@ use crate::{
args::{Arg, KwArgs},
FunctionKind,
},
CompilationError,
};
enum StatementKind<'a> {
@ -47,13 +48,13 @@ impl ExecutorContext {
async fn handle_annotations(
&self,
annotations: impl Iterator<Item = &Node<Annotation>>,
scope: annotations::AnnotationScope,
body_type: BodyType,
exec_state: &mut ExecState,
) -> Result<bool, KclError> {
let mut no_prelude = false;
for annotation in annotations {
if annotation.name() == Some(annotations::SETTINGS) {
if scope == annotations::AnnotationScope::Module {
if matches!(body_type, BodyType::Root(_)) {
let old_units = exec_state.length_unit();
exec_state.mod_local.settings.update_from_annotation(annotation)?;
let new_units = exec_state.length_unit();
@ -63,23 +64,26 @@ impl ExecutorContext {
.await?;
}
} else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Settings can only be modified at the top level scope of a file".to_owned(),
source_ranges: vec![annotation.as_source_range()],
}));
exec_state.err(CompilationError::err(
annotation.as_source_range(),
"Settings can only be modified at the top level scope of a file",
));
}
}
if annotation.name() == Some(annotations::NO_PRELUDE) {
if scope == annotations::AnnotationScope::Module {
} else if annotation.name() == Some(annotations::NO_PRELUDE) {
if matches!(body_type, BodyType::Root(_)) {
no_prelude = true;
} else {
return Err(KclError::Semantic(KclErrorDetails {
message: "Prelude can only be skipped at the top level scope of a file".to_owned(),
source_ranges: vec![annotation.as_source_range()],
}));
exec_state.err(CompilationError::err(
annotation.as_source_range(),
"Prelude can only be skipped at the top level scope of a file",
));
}
} else {
exec_state.warn(CompilationError::err(
annotation.as_source_range(),
"Unknown annotation",
));
}
// TODO warn on unknown annotations
}
Ok(no_prelude)
}
@ -92,40 +96,34 @@ impl ExecutorContext {
exec_state: &mut ExecState,
body_type: BodyType,
) -> Result<Option<KclValue>, KclError> {
if let BodyType::Root(init_mem) = body_type {
let no_prelude = self
.handle_annotations(
program.inner_attrs.iter(),
annotations::AnnotationScope::Module,
let no_prelude = self
.handle_annotations(program.inner_attrs.iter(), body_type, exec_state)
.await?;
if !no_prelude && body_type == BodyType::Root(true) {
// Import std::prelude
let prelude_range = SourceRange::from(program).start_as_range();
let id = self
.open_module(
&ImportPath::Std {
path: vec!["std".to_owned(), "prelude".to_owned()],
},
&[],
exec_state,
prelude_range,
)
.await?;
if !no_prelude && init_mem {
// Import std::prelude
let prelude_range = SourceRange::from(program).start_as_range();
let id = self
.open_module(
&ImportPath::Std {
path: vec!["std".to_owned(), "prelude".to_owned()],
},
&[],
exec_state,
prelude_range,
)
.await?;
let (module_memory, module_exports) = self
.exec_module_for_items(id, exec_state, ExecutionKind::Isolated, prelude_range)
.await
let (module_memory, module_exports) = self
.exec_module_for_items(id, exec_state, ExecutionKind::Isolated, prelude_range)
.await
.unwrap();
for name in module_exports {
let item = exec_state
.memory()
.get_from(&name, module_memory, prelude_range)
.cloned()
.unwrap();
for name in module_exports {
let item = exec_state
.memory()
.get_from(&name, module_memory, prelude_range)
.cloned()
.unwrap();
exec_state.mut_memory().add(name, item, prelude_range)?;
}
exec_state.mut_memory().add(name, item, prelude_range)?;
}
}

View File

@ -28,7 +28,7 @@ use crate::{
settings::types::UnitLength,
source_range::SourceRange,
std::StdLib,
ExecError, KclErrorWithOutputs,
CompilationError, ExecError, KclErrorWithOutputs,
};
pub use artifact::{Artifact, ArtifactCommand, ArtifactGraph, ArtifactId};
@ -70,6 +70,8 @@ pub struct ExecOutcome {
pub artifact_commands: Vec<ArtifactCommand>,
/// Output artifact graph.
pub artifact_graph: ArtifactGraph,
/// Non-fatal errors and warnings.
pub errors: Vec<CompilationError>,
}
#[derive(Debug, Default, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
@ -147,7 +149,7 @@ pub struct TagEngineInfo {
pub surface: Option<ExtrudeSurface>,
}
#[derive(Debug, Clone, Deserialize, Serialize, PartialEq)]
#[derive(Debug, Copy, Clone, Deserialize, Serialize, PartialEq)]
pub enum BodyType {
Root(bool),
Block,
@ -641,6 +643,8 @@ impl ExecutorContext {
///
/// You can optionally pass in some initialization memory for partial
/// execution.
///
/// To access non-fatal errors and warnings, extract them from the `ExecState`.
pub async fn run(
&self,
program: &crate::Program,
@ -827,6 +831,16 @@ mod tests {
memory.get(name, SourceRange::default()).unwrap().to_owned()
}
#[tokio::test(flavor = "multi_thread")]
async fn test_execute_warn() {
let text = "@blah";
let (_, _, exec_state) = parse_execute(text).await.unwrap();
let errs = exec_state.errors();
assert_eq!(errs.len(), 1);
assert_eq!(errs[0].severity, crate::errors::Severity::Warning);
assert!(errs[0].message.contains("Unknown annotation"));
}
#[tokio::test(flavor = "multi_thread")]
async fn test_execute_fn_definitions() {
let ast = r#"fn def = (x) => {

View File

@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
use uuid::Uuid;
use crate::{
errors::{KclError, KclErrorDetails},
errors::{KclError, KclErrorDetails, Severity},
execution::{
annotations, kcl_value, memory::ProgramMemory, Artifact, ArtifactCommand, ArtifactGraph, ArtifactId,
ExecOutcome, ExecutorSettings, KclValue, Operation, UnitAngle, UnitLen,
@ -14,6 +14,7 @@ use crate::{
modules::{ModuleId, ModuleInfo, ModuleLoader, ModulePath, ModuleRepr},
parsing::ast::types::Annotation,
source_range::SourceRange,
CompilationError,
};
/// State for executing a program.
@ -48,6 +49,8 @@ pub(super) struct GlobalState {
pub artifact_graph: ArtifactGraph,
/// Module loader.
pub mod_loader: ModuleLoader,
/// Errors and warnings.
pub errors: Vec<CompilationError>,
}
#[derive(Debug, Clone)]
@ -87,6 +90,21 @@ impl ExecState {
};
}
/// Log a non-fatal error.
pub fn err(&mut self, e: CompilationError) {
self.global.errors.push(e);
}
/// Log a warning.
pub fn warn(&mut self, mut e: CompilationError) {
e.severity = Severity::Warning;
self.global.errors.push(e);
}
pub fn errors(&self) -> &[CompilationError] {
&self.global.errors
}
/// Convert to execution outcome when running in WebAssembly. We want to
/// reduce the amount of data that crosses the WASM boundary as much as
/// possible.
@ -103,6 +121,7 @@ impl ExecState {
artifacts: self.global.artifacts,
artifact_commands: self.global.artifact_commands,
artifact_graph: self.global.artifact_graph,
errors: self.global.errors,
}
}
@ -119,6 +138,7 @@ impl ExecState {
artifacts: Default::default(),
artifact_commands: Default::default(),
artifact_graph: Default::default(),
errors: self.global.errors,
}
}
@ -194,6 +214,7 @@ impl GlobalState {
artifact_responses: Default::default(),
artifact_graph: Default::default(),
mod_loader: Default::default(),
errors: Default::default(),
};
let root_id = ModuleId::default();

View File

@ -7,7 +7,7 @@ use crate::{
errors::ExecErrorWithState,
execution::{ExecState, ExecutorContext, ExecutorSettings},
settings::types::UnitLength,
ConnectionError, ExecError, KclErrorWithOutputs, Program,
ConnectionError, ExecError, KclError, KclErrorWithOutputs, Program,
};
#[derive(serde::Deserialize, serde::Serialize)]
@ -70,6 +70,12 @@ async fn do_execute_and_snapshot(
ctx.run_with_ui_outputs(&program, &mut exec_state)
.await
.map_err(|err| ExecErrorWithState::new(err.into(), exec_state.clone()))?;
if !exec_state.errors().is_empty() {
return Err(ExecErrorWithState::new(
KclErrorWithOutputs::no_outputs(KclError::Semantic(exec_state.errors()[0].clone().into())).into(),
exec_state.clone(),
));
}
let snapshot_png_bytes = ctx
.prepare_snapshot()
.await