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(remap): compile-time program result type checking #4902

Merged
merged 30 commits into from Nov 16, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c811d28
add ValueKind enum
JeanMertz Nov 5, 2020
af109a7
add ResolveKind enum
JeanMertz Nov 5, 2020
2b37bbe
add CompilerState
JeanMertz Nov 5, 2020
58b9e40
add Expression::resolves_to
JeanMertz Nov 5, 2020
17634a4
add option to constraint Program value kind
JeanMertz Nov 6, 2020
8739cc9
rename ResolveKind to ValueConstraint
JeanMertz Nov 6, 2020
03824a0
introduce "TypeCheck" type
JeanMertz Nov 6, 2020
64c4357
infallible program must not have any fallible expressions
JeanMertz Nov 6, 2020
f92b859
block is fallible if any expression is fallible
JeanMertz Nov 6, 2020
e3142cc
force boolean return value for remap condition
JeanMertz Nov 6, 2020
b41e586
store path type-checks in correct store
JeanMertz Nov 6, 2020
d476aeb
cleanup
JeanMertz Nov 6, 2020
7b6a988
add type_check tests
JeanMertz Nov 7, 2020
a557fd2
rename TypeCheck to TypeDef
JeanMertz Nov 7, 2020
4ae205e
rename State to ProgramState
JeanMertz Nov 7, 2020
df68194
impl Clone for remap::Error
JeanMertz Nov 10, 2020
7b837ee
add as_*, try_*, and unwrap_* funcs to Value
JeanMertz Nov 10, 2020
3eea108
add as_*_mut func to Value
JeanMertz Nov 10, 2020
7089514
fix remap condition
JeanMertz Nov 11, 2020
c9179ea
expand TypeDef impl
JeanMertz Nov 11, 2020
9d6d2f2
improve module structure
JeanMertz Nov 11, 2020
282b164
properly format program error
JeanMertz Nov 11, 2020
cf90d9a
configure remap transform to accept any return value
JeanMertz Nov 11, 2020
98d1e06
remove function error cases
JeanMertz Nov 11, 2020
4446083
add type definitions to functions
JeanMertz Nov 12, 2020
01ef918
add function type definition tests
JeanMertz Nov 12, 2020
3a7e485
cleanup
JeanMertz Nov 12, 2020
1c7be2a
temporarily allow `None` values in conditions
JeanMertz Nov 12, 2020
8c067d8
clippy
JeanMertz Nov 12, 2020
c4c762a
update tests
JeanMertz Nov 12, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 11 additions & 2 deletions lib/remap-lang/src/error.rs
@@ -1,4 +1,4 @@
use crate::{expression, function, parser::Rule, value};
use crate::{expression, function, parser::Rule, program, value};
use std::error::Error as StdError;
use std::fmt;

Expand All @@ -7,6 +7,9 @@ pub enum Error {
#[error("parser error: {0}")]
Parser(String),

#[error("program error")]
Program(#[from] program::Error),

#[error("unexpected token sequence")]
Rule(#[from] Rule),

Expand Down Expand Up @@ -107,7 +110,7 @@ impl fmt::Display for Rule {
}
}

#[derive(Debug)]
#[derive(Debug, PartialEq)]
pub struct RemapError(pub(crate) Error);

impl StdError for RemapError {
Expand All @@ -130,6 +133,12 @@ impl fmt::Display for RemapError {
}
}

impl From<Error> for RemapError {
fn from(error: Error) -> Self {
RemapError(error)
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
146 changes: 146 additions & 0 deletions lib/remap-lang/src/expression.rs
@@ -1,4 +1,5 @@
use crate::{CompilerState, Object, Result, State, Value, ValueKind};
use std::fmt;

pub(super) mod arithmetic;
pub(super) mod assignment;
Expand Down Expand Up @@ -70,18 +71,22 @@ pub enum ResolveKind {
}

impl ResolveKind {
/// Returns `true` if this is a [`ResolveKind::Exact`].
pub fn is_exact(&self) -> bool {
matches!(self, ResolveKind::Exact(_))
}

/// Returns `true` if this is a [`ResolveKind::Any`].
pub fn is_any(&self) -> bool {
matches!(self, ResolveKind::Any)
}

/// Returns `true` if this is a [`ResolveKind::Maybe`].
pub fn is_maybe(&self) -> bool {
matches!(self, ResolveKind::Maybe(_))
}

/// Get a collection of [`ValueKind`]s accepted by this [`ResolveKind`].
pub fn value_kinds(&self) -> Vec<ValueKind> {
use ResolveKind::*;

Expand All @@ -92,6 +97,88 @@ impl ResolveKind {
Maybe(v) => v.value_kinds(),
}
}

/// Merge two [`ResolveKind`]s, such that the new `ResolveKind` provides the
/// most constraint possible resolve kind variant.
pub fn merge(&self, other: &Self) -> Self {
use ResolveKind::*;

if let Maybe(kind) = self {
let other = match other {
Maybe(v) => v,
_ => other,
};

return Maybe(Box::new(kind.merge(other)));
}

if let Maybe(kind) = other {
return Maybe(Box::new(self.merge(kind)));
}

if self.is_any() || other.is_any() {
return Any;
}

StephenWakely marked this conversation as resolved.
Show resolved Hide resolved
let mut kinds: Vec<_> = self
.value_kinds()
.into_iter()
.chain(other.value_kinds().into_iter())
.collect();

kinds.sort();
kinds.dedup();

if kinds.len() == 1 {
Exact(kinds[0])
} else {
OneOf(kinds)
}
}

/// Returns `true` if the _other_ [`ResolveKind`] is contained within the
/// current one.
///
/// That is to say, its constraints must be more strict or equal to the
/// constraints of the current one.
pub fn contains(&self, other: &Self) -> bool {
// If we don't expect none, but the other does, the other's requirement
// is less strict than ours.
if !self.is_maybe() && other.is_maybe() {
return false;
}

let self_kinds = self.value_kinds();
let other_kinds = other.value_kinds();

for kind in other_kinds {
if !self_kinds.contains(&kind) {
return false;
}
}

true
}
}

impl fmt::Display for ResolveKind {
/// Print a human readable version of the resolve kind constraints.
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
use ResolveKind::*;

match self {
Any => f.write_str("any value"),
OneOf(v) => {
f.write_str("any of ")?;
f.write_str(&v.iter().map(|v| v.as_str()).collect::<Vec<_>>().join(", "))
}
Exact(v) => f.write_str(&v),
Maybe(v) => {
f.write_str("none or ")?;
f.write_str(&v.to_string())
}
}
}
}

pub trait Expression: Send + Sync + std::fmt::Debug + dyn_clone::DynClone {
Expand Down Expand Up @@ -154,3 +241,62 @@ expression_dispatch![
Path,
Variable,
];

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_contains() {
use ResolveKind::*;
use ValueKind::*;

let cases = vec![
(true, Any, Any),
(true, Any, Exact(String)),
(true, Any, Exact(Integer)),
(true, Any, OneOf(vec![Float, Boolean])),
(true, Any, OneOf(vec![Map])),
(false, Any, Maybe(Box::new(Any))),
(true, Exact(String), Exact(String)),
(true, Exact(String), OneOf(vec![String])),
(false, Exact(String), Exact(Array)),
(false, Exact(String), OneOf(vec![Integer])),
(false, Exact(String), OneOf(vec![Integer, Float])),
(false, Exact(String), Maybe(Box::new(Any))),
];

for (expect, this, other) in cases {
assert_eq!(this.contains(&other), expect);
}
}

#[test]
fn test_merge() {
use ResolveKind::*;
use ValueKind::*;

let cases = vec![
(Any, Any, Any),
(Maybe(Box::new(Any)), Maybe(Box::new(Any)), Any),
(Maybe(Box::new(Any)), Any, Maybe(Box::new(Any))),
(Any, OneOf(vec![Integer, String]), Any),
(OneOf(vec![Integer, Float]), Exact(Integer), Exact(Float)),
(Exact(Integer), Exact(Integer), Exact(Integer)),
(
Maybe(Box::new(Exact(Integer))),
Maybe(Box::new(Exact(Integer))),
Exact(Integer),
),
(
OneOf(vec![String, Integer, Float, Boolean]),
OneOf(vec![Integer, String]),
OneOf(vec![Float, Boolean]),
),
];

for (expect, this, other) in cases {
assert_eq!(this.merge(&other), expect);
}
}
}
7 changes: 5 additions & 2 deletions lib/remap-lang/src/expression/arithmetic.rs
Expand Up @@ -48,10 +48,13 @@ impl Expression for Arithmetic {
result.map(Some).map_err(Into::into)
}

fn resolves_to(&self, _: &CompilerState) -> ResolveKind {
fn resolves_to(&self, state: &CompilerState) -> ResolveKind {
let lhs_kind = self.lhs.resolves_to(state);
let rhs_kind = self.rhs.resolves_to(state);

use Operator::*;
match self.op {
Or => ResolveKind::Any,
Or => lhs_kind.merge(&rhs_kind),
Multiply | Add => ResolveKind::OneOf(vec![
ValueKind::String,
ValueKind::Integer,
Expand Down
7 changes: 6 additions & 1 deletion lib/remap-lang/src/lib.rs
Expand Up @@ -157,6 +157,10 @@ mod tests {
fn execute(&self, _: &mut State, _: &mut dyn Object) -> Result<Option<Value>> {
Ok(Some(format!("regex: {:?}", self.0).into()))
}

fn resolves_to(&self, _: &CompilerState) -> ResolveKind {
ResolveKind::Any
}
}

#[test]
Expand Down Expand Up @@ -234,7 +238,8 @@ mod tests {
];

for (script, expectation) in cases {
let program = Program::new(script, &[Box::new(RegexPrinter)]).unwrap();
let accept = ResolveKind::Maybe(Box::new(ResolveKind::Any));
let program = Program::new(script, &[Box::new(RegexPrinter)], accept).unwrap();
let mut runtime = Runtime::new(State::default());
let mut event = HashMap::default();

Expand Down
58 changes: 56 additions & 2 deletions lib/remap-lang/src/program.rs
@@ -1,6 +1,14 @@
use crate::{parser, CompilerState, Error, Expr, Function, RemapError};
use crate::{
parser, CompilerState, Error as E, Expr, Expression, Function, RemapError, ResolveKind,
};
use pest::Parser;

#[derive(thiserror::Error, Debug, PartialEq)]
pub enum Error {
#[error("program is expected to resolve to {0}, but instead resolves to {1}")]
ResolvesTo(ResolveKind, ResolveKind),
}

/// The program to execute.
///
/// This object is passed to [`Runtime::execute`](crate::Runtime::execute).
Expand All @@ -16,9 +24,10 @@ impl Program {
pub fn new(
source: &str,
function_definitions: &[Box<dyn Function>],
must_resolve_to: ResolveKind,
) -> Result<Self, RemapError> {
let pairs = parser::Parser::parse(parser::Rule::program, source)
.map_err(|s| Error::Parser(s.to_string()))
.map_err(|s| E::Parser(s.to_string()))
.map_err(RemapError)?;

let compiler_state = CompilerState::default();
Expand All @@ -30,6 +39,51 @@ impl Program {

let expressions = parser.pairs_to_expressions(pairs).map_err(RemapError)?;

let will_resolve_to = expressions
.last()
.map(|e| e.resolves_to(&parser.compiler_state))
.unwrap_or_else(|| ResolveKind::Maybe(Box::new(ResolveKind::Any)));

if !must_resolve_to.contains(&will_resolve_to) {
return Err(RemapError::from(E::from(Error::ResolvesTo(
must_resolve_to,
will_resolve_to,
))));
}

Ok(Self { expressions })
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::ValueKind;

#[test]
fn program_test() {
use ResolveKind::*;

let cases = vec![
(".foo", Any, Ok(())),
(
".foo",
Exact(ValueKind::String),
Err("remap error: program error: program is expected to resolve to string, but instead resolves to any value".to_owned()),
),
(
"false || 2",
OneOf(vec![ValueKind::String, ValueKind::Float]),
Err("remap error: program error: program is expected to resolve to any of string, float, but instead resolves to any of integer, boolean".to_owned()),
),
];

for (source, must_resolve_to, expect) in cases {
let program = Program::new(source, &[], must_resolve_to)
.map(|_| ())
.map_err(|e| e.to_string());

assert_eq!(program, expect);
}
}
}
2 changes: 1 addition & 1 deletion lib/remap-lang/src/value.rs
Expand Up @@ -18,7 +18,7 @@ pub enum Value {
Null,
}

#[derive(Eq, PartialEq, Debug, Clone, Copy)]
#[derive(Eq, PartialEq, Hash, Debug, Clone, Copy, Ord, PartialOrd)]
pub enum ValueKind {
String,
Integer,
Expand Down