Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(diagnostics): improve assignment fallibility compilation error #553

Merged
merged 3 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# │ │ │
# │ │ this expression is fallible
# │ │ update the expression to be infallible
# │ │ note if an argument type is invalid it can render a function fallible
# │ │ '.result[0].an' argument type is 'string or undefined' and this function expects parameter 'value' of type 'string'
# │ or change this to an infallible assignment:
# │ .a, err = sha3(.result[0].an)
# │
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# │ │ │
# │ │ this expression is fallible
# │ │ update the expression to be infallible
# │ │ note if an argument type is invalid it can render a function fallible
# │ │ '.log' argument type is 'any' and this function expects parameter 'value' of type 'string'
# │ or change this to an infallible assignment:
# │ ., err = parse_common_log(.log)
# │
Expand Down
51 changes: 42 additions & 9 deletions src/compiler/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::compiler::expression::function_call::FunctionCallError;
use crate::compiler::expression::ExpressionError;
use crate::compiler::{
expression::{
assignment, function_call, literal, predicate, query, Abort, Array, Assignment, Block,
Expand All @@ -12,7 +14,7 @@ use crate::diagnostic::{DiagnosticList, DiagnosticMessage, Note};
use crate::parser::ast::{self, Node, QueryTarget};
use crate::path::PathPrefix;
use crate::path::{OwnedTargetPath, OwnedValuePath};
use crate::prelude::ArgumentList;
use crate::prelude::{expression, ArgumentList};
use crate::value::Value;

use super::state::TypeState;
Expand Down Expand Up @@ -52,11 +54,35 @@ pub struct Compiler<'a> {
// This should probably be kept on the call stack as the "compile_*" functions are called
// otherwise some expressions may remove it when they shouldn't (such as the RHS of an operation removing
// the error from the LHS)
fallible_expression_error: Option<Box<dyn DiagnosticMessage>>,
fallible_expression_error: Option<CompilerError>,

config: CompileConfig,
}

// TODO: The diagnostic related code is in dire need of refactoring.
// This is a workaround to avoid doing this work upfront.
#[derive(Debug)]
pub(crate) enum CompilerError {
FunctionCallError(FunctionCallError),
ExpressionError(ExpressionError),
}

impl CompilerError {
fn to_diagnostic(&self) -> &dyn DiagnosticMessage {
match self {
CompilerError::FunctionCallError(e) => e,
CompilerError::ExpressionError(e) => e,
}
}

fn into_diagnostic_boxed(self) -> Box<dyn DiagnosticMessage> {
match self {
CompilerError::FunctionCallError(e) => Box::new(e),
CompilerError::ExpressionError(e) => Box::new(e),
}
}
}

impl<'a> Compiler<'a> {
pub fn compile(
fns: &'a [Box<dyn Function>],
Expand Down Expand Up @@ -151,8 +177,9 @@ impl<'a> Compiler<'a> {

let type_def = expr.type_info(&original_state).result;
if type_def.is_fallible() && self.fallible_expression_error.is_none() {
let error = super::expression::Error::Fallible { span };
self.fallible_expression_error = Some(Box::new(error) as _);
self.fallible_expression_error = Some(CompilerError::ExpressionError(
expression::ExpressionError::Fallible { span },
));
}

Some(expr)
Expand Down Expand Up @@ -234,7 +261,7 @@ impl<'a> Compiler<'a> {

if let Some(expr) = self.compile_expr(node_expr, state) {
if let Some(error) = self.fallible_expression_error.take() {
self.diagnostics.push(error);
self.diagnostics.push(error.into_diagnostic_boxed());
}

node_exprs.push(expr);
Expand Down Expand Up @@ -353,7 +380,9 @@ impl<'a> Compiler<'a> {
Some(Predicate::new(
Node::new(span, exprs),
state,
self.fallible_expression_error.as_deref(),
self.fallible_expression_error
.as_ref()
.map(CompilerError::to_diagnostic),
))
}

Expand Down Expand Up @@ -510,7 +539,7 @@ impl<'a> Compiler<'a> {
let assignment = Assignment::new(
node,
state,
self.fallible_expression_error.as_deref(),
self.fallible_expression_error.as_ref(),
&self.config,
)
.map_err(|err| self.diagnostics.push(Box::new(err)))
Expand Down Expand Up @@ -696,12 +725,16 @@ impl<'a> Compiler<'a> {
state,
block,
local_snapshot,
&mut self.fallible_expression_error,
&mut self.config,
)
.map_err(|err| self.diagnostics.push(Box::new(err)))
.ok()
.map(|func| (arg_list, func))
.map(|result| {
if let Some(e) = result.error {
self.fallible_expression_error = Some(CompilerError::FunctionCallError(e));
}
(arg_list, result.function_call)
})
});

if let Some((args, function)) = &function_info {
Expand Down
58 changes: 29 additions & 29 deletions src/compiler/expression.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,33 @@
use std::fmt;

use dyn_clone::{clone_trait_object, DynClone};

pub use abort::Abort;
pub use array::Array;
pub use assignment::Assignment;
pub use block::Block;
pub use container::{Container, Variant};
pub use function::FunctionExpression;
pub use function_argument::FunctionArgument;
pub use function_call::FunctionCall;
pub use group::Group;
pub use if_statement::IfStatement;
pub use literal::Literal;
pub use noop::Noop;
pub use not::Not;
pub use object::Object;
pub use op::Op;
pub use predicate::Predicate;
pub use query::{Query, Target};
pub use unary::Unary;
pub use variable::Variable;

use crate::diagnostic::{DiagnosticMessage, Label, Note};
use crate::value::Value;
use dyn_clone::{clone_trait_object, DynClone};

use super::state::{TypeInfo, TypeState};
use super::{Context, Span, TypeDef};
pub use super::{ExpressionError2, Resolved};

mod abort;
mod array;
Expand All @@ -28,29 +51,6 @@ pub(crate) mod literal;
pub(crate) mod predicate;
pub mod query;

pub use super::{ExpressionError, Resolved};

use super::state::{TypeInfo, TypeState};
pub use abort::Abort;
pub use array::Array;
pub use assignment::Assignment;
pub use block::Block;
pub use container::{Container, Variant};
pub use function::FunctionExpression;
pub use function_argument::FunctionArgument;
pub use function_call::FunctionCall;
pub use group::Group;
pub use if_statement::IfStatement;
pub use literal::Literal;
pub use noop::Noop;
pub use not::Not;
pub use object::Object;
pub use op::Op;
pub use predicate::Predicate;
pub use query::{Query, Target};
pub use unary::Unary;
pub use variable::Variable;

pub trait Expression: Send + Sync + fmt::Debug + DynClone {
/// Resolve an expression to a concrete [`Value`].
///
Expand Down Expand Up @@ -373,17 +373,17 @@ impl From<Value> for Expr {
// -----------------------------------------------------------------------------

#[derive(thiserror::Error, Debug)]
pub enum Error {
pub enum ExpressionError {
#[error("unhandled error")]
Fallible { span: Span },

#[error("expression type unavailable")]
Missing { span: Span, feature: &'static str },
}

impl DiagnosticMessage for Error {
impl DiagnosticMessage for ExpressionError {
fn code(&self) -> usize {
use Error::{Fallible, Missing};
use ExpressionError::{Fallible, Missing};

match self {
Fallible { .. } => 100,
Expand All @@ -392,7 +392,7 @@ impl DiagnosticMessage for Error {
}

fn labels(&self) -> Vec<Label> {
use Error::{Fallible, Missing};
use ExpressionError::{Fallible, Missing};

match self {
Fallible { span } => vec![
Expand All @@ -410,7 +410,7 @@ impl DiagnosticMessage for Error {
}

fn notes(&self) -> Vec<Note> {
use Error::{Fallible, Missing};
use ExpressionError::{Fallible, Missing};

match self {
Fallible { .. } => vec![Note::SeeErrorDocs],
Expand Down
14 changes: 7 additions & 7 deletions src/compiler/expression/abort.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
use std::fmt;

use crate::diagnostic::{DiagnosticMessage, Label, Note, Urls};
use crate::parser::ast::Node;

use super::Expr;
use crate::compiler::{
expression::{ExpressionError, Resolved},
expression::{ExpressionError2, Resolved},
state::{TypeInfo, TypeState},
value::{Kind, VrlValueConvert},
Context, Expression, Span, TypeDef,
};
use crate::diagnostic::{DiagnosticMessage, Label, Note, Urls};
use crate::parser::ast::Node;

use super::Expr;

#[derive(Debug, Clone, PartialEq)]
pub struct Abort {
Expand Down Expand Up @@ -53,12 +53,12 @@ impl Expression for Abort {
let message = self
.message
.as_ref()
.map::<Result<_, ExpressionError>, _>(|expr| {
.map::<Result<_, ExpressionError2>, _>(|expr| {
Ok(expr.resolve(ctx)?.try_bytes_utf8_lossy()?.to_string())
})
.transpose()?;

Err(ExpressionError::Abort {
Err(ExpressionError2::Abort {
span: self.span,
message,
})
Expand Down
Loading
Loading