Skip to content

Commit

Permalink
feat(vrl): allow compiler to emit non-fatal errors (vectordotdev#12412)
Browse files Browse the repository at this point in the history
  • Loading branch information
JeanMertz authored Apr 27, 2022
1 parent 40ceae3 commit dcc7bd2
Showing 35 changed files with 196 additions and 129 deletions.
9 changes: 6 additions & 3 deletions benches/remap.rs
Original file line number Diff line number Diff line change
@@ -62,7 +62,8 @@ fn benchmark_remap(c: &mut Criterion) {
},
&Default::default(),
)
.unwrap(),
.unwrap()
.0,
);

let event = {
@@ -131,7 +132,8 @@ fn benchmark_remap(c: &mut Criterion) {
},
&Default::default(),
)
.unwrap(),
.unwrap()
.0,
);

let event = {
@@ -206,7 +208,8 @@ fn benchmark_remap(c: &mut Criterion) {
drop_on_abort: true,
..Default::default()
}, &Default::default())
.unwrap(),
.unwrap()
.0,
);

let mut event = Event::from("coerce me");
9 changes: 5 additions & 4 deletions lib/enrichment/src/find_enrichment_table_records.rs
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ impl Function for FindEnrichmentTableRecords {
) -> Compiled {
let registry = ctx
.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticError>)?;
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticMessage>)?;

let tables = registry
.table_ids()
@@ -149,9 +149,10 @@ impl Function for FindEnrichmentTableRecords {
) -> CompiledArgument {
match (name, expr) {
("table", Some(expr)) => {
let registry = ctx
.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticError>)?;
let registry =
ctx.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded)
as Box<dyn DiagnosticMessage>)?;

let tables = registry
.table_ids()
9 changes: 5 additions & 4 deletions lib/enrichment/src/get_enrichment_table_record.rs
Original file line number Diff line number Diff line change
@@ -86,7 +86,7 @@ impl Function for GetEnrichmentTableRecord {
) -> Compiled {
let registry = ctx
.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticError>)?;
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticMessage>)?;

let tables = registry
.table_ids()
@@ -141,9 +141,10 @@ impl Function for GetEnrichmentTableRecord {
) -> CompiledArgument {
match (name, expr) {
("table", Some(expr)) => {
let registry = ctx
.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded) as Box<dyn DiagnosticError>)?;
let registry =
ctx.get_external_context_mut::<TableRegistry>()
.ok_or(Box::new(vrl_util::Error::TablesNotLoaded)
as Box<dyn DiagnosticMessage>)?;

let tables = registry
.table_ids()
8 changes: 4 additions & 4 deletions lib/enrichment/src/vrl_util.rs
Original file line number Diff line number Diff line change
@@ -23,7 +23,7 @@ impl fmt::Display for Error {

impl std::error::Error for Error {}

impl DiagnosticError for Error {
impl DiagnosticMessage for Error {
fn code(&self) -> usize {
111
}
@@ -84,7 +84,7 @@ pub(crate) fn add_index(
}

/// Takes a static boolean argument and return the value it resolves to.
fn arg_to_bool(arg: &FunctionArgument) -> std::result::Result<bool, Box<dyn DiagnosticError>> {
fn arg_to_bool(arg: &FunctionArgument) -> std::result::Result<bool, Box<dyn DiagnosticMessage>> {
arg.expr()
.as_value()
.as_ref()
@@ -102,7 +102,7 @@ fn arg_to_bool(arg: &FunctionArgument) -> std::result::Result<bool, Box<dyn Diag
}

/// Takes a function argument (expected to be a static boolean) and returns a `Case`.
fn arg_to_case(arg: &FunctionArgument) -> std::result::Result<Case, Box<dyn DiagnosticError>> {
fn arg_to_case(arg: &FunctionArgument) -> std::result::Result<Case, Box<dyn DiagnosticMessage>> {
if arg_to_bool(arg)? {
Ok(Case::Sensitive)
} else {
@@ -123,7 +123,7 @@ pub(crate) fn index_from_args(
table: String,
registry: &mut TableRegistry,
args: &[(&'static str, Option<FunctionArgument>)],
) -> std::result::Result<EnrichmentTableRecord, Box<dyn DiagnosticError>> {
) -> std::result::Result<EnrichmentTableRecord, Box<dyn DiagnosticMessage>> {
let case_sensitive = args
.iter()
.find(|(name, _)| *name == "case_sensitive")
2 changes: 1 addition & 1 deletion lib/vector-vrl-functions/src/set_semantic_meaning.rs
Original file line number Diff line number Diff line change
@@ -69,7 +69,7 @@ impl Function for SetSemanticMeaning {
return Err(Box::new(ExpressionError::from(format!(
"meaning must be set on an external field: {}",
query
))) as Box<dyn DiagnosticError>);
))) as Box<dyn DiagnosticMessage>);
}

if let Some(list) = ctx.get_external_context_mut::<MeaningList>() {
11 changes: 10 additions & 1 deletion lib/vrl/cli/src/cmd.rs
Original file line number Diff line number Diff line change
@@ -46,6 +46,10 @@ pub struct Opts {
/// Should we use the VM to evaluate the VRL
#[clap(short, long = "runtime", default_value_t)]
runtime: VrlRuntime,

// Should the CLI emit warnings
#[clap(long = "print-warnings")]
print_warnings: bool,
}

impl Opts {
@@ -117,10 +121,15 @@ fn run(opts: &Opts) -> Result<(), Error> {
} else {
let objects = opts.read_into_objects()?;
let source = opts.read_program()?;
let program = vrl::compile(&source, &stdlib::all()).map_err(|diagnostics| {
let (program, warnings) = vrl::compile(&source, &stdlib::all()).map_err(|diagnostics| {
Error::Parse(Formatter::new(&source, diagnostics).colored().to_string())
})?;

if opts.print_warnings {
let warnings = Formatter::new(&source, warnings).colored().to_string();
eprintln!("{warnings}")
}

for mut object in objects {
let state = state::Runtime::default();
let runtime = Runtime::new(state);
54 changes: 31 additions & 23 deletions lib/vrl/compiler/src/compiler.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use bytes::Bytes;
use chrono::{TimeZone, Utc};
use diagnostic::DiagnosticError;
use diagnostic::{DiagnosticList, DiagnosticMessage, Severity};
use ordered_float::NotNan;
use parser::ast::{self, AssignmentOp, Node};

@@ -10,11 +10,11 @@ use crate::{
Function, Program, Value,
};

pub(crate) type Errors = Vec<Box<dyn DiagnosticError>>;
pub(crate) type Diagnostics = Vec<Box<dyn DiagnosticMessage>>;

pub(crate) struct Compiler<'a> {
fns: &'a [Box<dyn Function>],
errors: Errors,
diagnostics: Diagnostics,
fallible: bool,
abortable: bool,
local: LocalEnv,
@@ -24,7 +24,7 @@ impl<'a> Compiler<'a> {
pub(super) fn new(fns: &'a [Box<dyn Function>]) -> Self {
Self {
fns,
errors: vec![],
diagnostics: vec![],
fallible: false,
abortable: false,
local: LocalEnv::default(),
@@ -44,23 +44,31 @@ impl<'a> Compiler<'a> {
mut self,
ast: parser::Program,
external: &mut ExternalEnv,
) -> Result<Program, Errors> {
) -> Result<(Program, DiagnosticList), DiagnosticList> {
let expressions = self
.compile_root_exprs(ast, external)
.into_iter()
.map(|expr| Box::new(expr) as _)
.collect();

if !self.errors.is_empty() {
return Err(self.errors);
let (errors, warnings): (Vec<_>, Vec<_>) =
self.diagnostics.into_iter().partition(|diagnostic| {
matches!(diagnostic.severity(), Severity::Bug | Severity::Error)
});

if !errors.is_empty() {
return Err(errors.into());
}

Ok(Program {
expressions,
fallible: self.fallible,
abortable: self.abortable,
local_env: self.local,
})
Ok((
Program {
expressions,
fallible: self.fallible,
abortable: self.abortable,
local_env: self.local,
},
warnings.into(),
))
}

fn compile_root_exprs(
@@ -81,7 +89,7 @@ impl<'a> Compiler<'a> {
if expr.type_def((&self.local, external)).is_fallible() {
use crate::expression::Error;
let err = Error::Fallible { span };
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
}

Some(expr)
@@ -164,7 +172,7 @@ impl<'a> Compiler<'a> {
NanFloat => NotNan::new(0.0).unwrap().into(),
};

self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
value
});

@@ -252,7 +260,7 @@ impl<'a> Compiler<'a> {
let predicate = match self.compile_predicate(predicate, external) {
Ok(v) => v,
Err(err) => {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
return IfStatement::noop();
}
};
@@ -295,7 +303,7 @@ impl<'a> Compiler<'a> {
let rhs = Node::new(rhs_span, self.compile_expr(*rhs, external));

Op::new(lhs, opcode, rhs, (&mut self.local, external)).unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
Op::noop()
})
}
@@ -384,7 +392,7 @@ impl<'a> Compiler<'a> {
};

Assignment::new(node, &mut self.local, external).unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
Assignment::noop()
})
}
@@ -478,7 +486,7 @@ impl<'a> Compiler<'a> {
builder.compile(&mut self.local, external, block)
})
.unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
FunctionCall::noop()
})
}
@@ -501,7 +509,7 @@ impl<'a> Compiler<'a> {
let (span, ident) = node.take();

Variable::new(span, ident.clone(), &self.local).unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
Variable::noop(ident)
})
}
@@ -522,7 +530,7 @@ impl<'a> Compiler<'a> {
let node = Node::new(expr.span(), self.compile_expr(*expr, external));

Not::new(node, not.span(), (&self.local, external)).unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
Not::noop()
})
}
@@ -535,12 +543,12 @@ impl<'a> Compiler<'a> {
.map(|expr| Node::new(expr.span(), self.compile_expr(*expr, external)));

Abort::new(span, message, (&self.local, external)).unwrap_or_else(|err| {
self.errors.push(Box::new(err));
self.diagnostics.push(Box::new(err));
Abort::noop(span)
})
}

fn handle_parser_error(&mut self, error: parser::Error) {
self.errors.push(Box::new(error))
self.diagnostics.push(Box::new(error))
}
}
4 changes: 2 additions & 2 deletions lib/vrl/compiler/src/expression.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::collections::BTreeMap;
use std::fmt;

use diagnostic::{DiagnosticError, Label, Note};
use diagnostic::{DiagnosticMessage, Label, Note};
use dyn_clone::{clone_trait_object, DynClone};

use crate::{
@@ -396,7 +396,7 @@ pub enum Error {
Fallible { span: Span },
}

impl DiagnosticError for Error {
impl DiagnosticMessage for Error {
fn code(&self) -> usize {
use Error::*;

4 changes: 2 additions & 2 deletions lib/vrl/compiler/src/expression/abort.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::fmt;

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

use crate::{
@@ -137,7 +137,7 @@ impl std::error::Error for Error {
}
}

impl DiagnosticError for Error {
impl DiagnosticMessage for Error {
fn code(&self) -> usize {
use ErrorVariant::*;

4 changes: 2 additions & 2 deletions lib/vrl/compiler/src/expression/assignment.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{convert::TryFrom, fmt};

use diagnostic::{DiagnosticError, Label, Note};
use diagnostic::{DiagnosticMessage, Label, Note};
use lookup::LookupBuf;

use crate::{
@@ -520,7 +520,7 @@ impl std::error::Error for Error {
}
}

impl DiagnosticError for Error {
impl DiagnosticMessage for Error {
fn code(&self) -> usize {
use ErrorVariant::*;

6 changes: 3 additions & 3 deletions lib/vrl/compiler/src/expression/function_call.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{fmt, sync::Arc};

use anymap::AnyMap;
use diagnostic::{DiagnosticError, Label, Note, Urls};
use diagnostic::{DiagnosticMessage, Label, Note, Urls};

use crate::{
expression::assignment::Details,
@@ -804,7 +804,7 @@ pub(crate) enum Error {
#[error("function compilation error: error[E{}] {}", error.code(), error)]
Compilation {
call_span: Span,
error: Box<dyn DiagnosticError>,
error: Box<dyn DiagnosticMessage>,
},

#[error("can't abort infallible function")]
@@ -853,7 +853,7 @@ pub(crate) enum Error {
},
}

impl DiagnosticError for Error {
impl DiagnosticMessage for Error {
fn code(&self) -> usize {
use Error::*;

Loading
Oops, something went wrong.

0 comments on commit dcc7bd2

Please sign in to comment.