Skip to content

Commit

Permalink
Add product type to the Rule trait to simplify unit testing. (#10720)
Browse files Browse the repository at this point in the history
### Problem

The `Rule` trait did not require that a `Rule` declare its own return type, which was ... strange. It also made unit testing more awkward, because rules needed to be declared in a type->rules map rather than a flat list.

### Solution

Add `Rule::product`, and simplify tests.

[ci skip-build-wheels]
  • Loading branch information
stuhood authored Sep 1, 2020
1 parent d665618 commit b543adb
Show file tree
Hide file tree
Showing 6 changed files with 162 additions and 229 deletions.
20 changes: 14 additions & 6 deletions src/rust/engine/rule_graph/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,29 +171,37 @@ type MonomorphizedGraph<R> = DiGraph<
/// Given the set of Rules and Queries, produce a RuleGraph that allows dependency nodes
/// to be found statically.
///
pub struct Builder<'t, R: Rule> {
rules: &'t BTreeMap<R::TypeId, Vec<R>>,
pub struct Builder<R: Rule> {
rules: BTreeMap<R::TypeId, Vec<R>>,
queries: Vec<Query<R>>,
params: ParamTypes<R::TypeId>,
}

impl<'t, R: Rule> Builder<'t, R> {
pub fn new(rules: &'t BTreeMap<R::TypeId, Vec<R>>, queries: Vec<Query<R>>) -> Builder<'t, R> {
impl<R: Rule> Builder<R> {
pub fn new(rules: Vec<R>, queries: Vec<Query<R>>) -> Builder<R> {
// Group rules by product/return type.
let mut rules_by_type = BTreeMap::new();
for rule in rules {
rules_by_type
.entry(rule.product())
.or_insert_with(Vec::new)
.push(rule);
}
// The set of all input Params in the graph: ie, those provided either via Queries, or via
// a Rule with a DependencyKey that provides a Param.
let params = queries
.iter()
.flat_map(|query| query.params.iter().cloned())
.chain(
rules
rules_by_type
.values()
.flatten()
.flat_map(|rule| rule.dependency_keys())
.filter_map(|dk| dk.provided_param()),
)
.collect::<ParamTypes<_>>();
Builder {
rules,
rules: rules_by_type,
queries,
params,
}
Expand Down
7 changes: 2 additions & 5 deletions src/rust/engine/rule_graph/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
mod builder;
mod rules;

use std::collections::{BTreeMap, HashMap, HashSet};
use std::collections::{HashMap, HashSet};
use std::io;

pub use crate::builder::Builder;
Expand Down Expand Up @@ -243,10 +243,7 @@ fn entry_with_deps_str<R: Rule>(entry: &EntryWithDeps<R>) -> String {
}

impl<R: Rule> RuleGraph<R> {
pub fn new(
rules: &BTreeMap<R::TypeId, Vec<R>>,
queries: Vec<Query<R>>,
) -> Result<RuleGraph<R>, String> {
pub fn new(rules: Vec<R>, queries: Vec<Query<R>>) -> Result<RuleGraph<R>, String> {
Builder::new(rules, queries).graph()
}

Expand Down
5 changes: 5 additions & 0 deletions src/rust/engine/rule_graph/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ pub trait Rule: Clone + Debug + Display + Hash + Eq + Sized + DisplayForGraph +
type TypeId: TypeId;
type DependencyKey: DependencyKey<TypeId = Self::TypeId>;

///
/// Returns the product (output) type for this Rule.
///
fn product(&self) -> Self::TypeId;

///
/// Return keys for the dependencies of this Rule.
///
Expand Down
Loading

0 comments on commit b543adb

Please sign in to comment.