Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
12 changes: 12 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,18 @@ end

`data_errors` returns a `{ fact_incomplete_definition: ... }` sentinel that downstream facts can detect.

#### A fact must never resolve to `nil`

Every fact has to return a real value or signal an error (`data_errors` / an error
hash). `nil` is not a valid result: it leaves the facts downstream unable to tell a
missing value apart from a satisfied one, so the missing dependency propagates
silently. The evaluator enforces this — a fact that resolves to `nil` raises
`FactGraph::NullFactError` naming the offending fact. In practice this catches two
mistakes: an `allow_unmet_dependencies` fact whose branch returns `nil` instead of
`data_errors`, and an input fact whose validator permits `nil` (leave the `nil`
case to an error instead of passing it through). Note that `false` is a valid
value and does not raise.

`must_match` is a convenience wrapper that runs a pattern-match block and falls back to `data_errors` on `NoMatchingPatternError`:

```ruby
Expand Down
4 changes: 4 additions & 0 deletions lib/fact_graph.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@
module FactGraph
class ValidationError < StandardError; end

# Raised when a fact resolves to nil instead of a value or an unmet-dependency
# error hash — nil leaves dependents unable to tell "missing" from "satisfied".
class NullFactError < StandardError; end

class Graph
@graph_registry = []

Expand Down
19 changes: 17 additions & 2 deletions lib/fact_graph/fact.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ def call(input, results)
results[module_name] ||= {}

if !resolver.respond_to?(:call)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thoughts on flipping this to unless ?

forbid_nil!(resolver)
results[module_name][name] = resolver
return resolver
end
Expand Down Expand Up @@ -195,11 +196,25 @@ def call(input, results)
resolved_errors = data_errors
end

value = resolved_errors || data.instance_exec(&resolver)
forbid_nil!(value)

if per_entity
results[module_name][name] ||= {}
results[module_name][name][entity_id] = resolved_errors || data.instance_exec(&resolver)
results[module_name][name][entity_id] = value
else
results[module_name][name] = resolved_errors || data.instance_exec(&resolver)
results[module_name][name] = value
end
end

private

def forbid_nil!(value)
return unless value.nil?

fact_ref = per_entity ? ":#{module_name}/:#{name} (entity #{entity_id})" : ":#{module_name}/:#{name}"
raise FactGraph::NullFactError,
"Fact #{fact_ref} resolved to nil. A fact must return a value or signal an " \
"unmet dependency (return the error hash / data_errors), never nil."
end
end
73 changes: 73 additions & 0 deletions spec/fact_graph/fact_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,79 @@
end
end

describe "null fact detection" do
before { FactGraph::Graph.graph_registry = [] }

def evaluate_single_fact(module_name, fact_name, input = {})
graph = FactGraph::Graph.prepare_fact_objects(input)
graph[module_name][fact_name].call(input, {})
end

it "raises when a resolver proc returns nil" do
Class.new(FactGraph::Graph) do
in_module :leaks do
fact :leaky do
proc { nil }
end
end
end
expect { evaluate_single_fact(:leaks, :leaky) }
.to raise_error(FactGraph::NullFactError, /:leaks\/:leaky resolved to nil/)
end

it "raises when an allow_unmet_dependencies fact returns nil on its unmet path" do
Class.new(FactGraph::Graph) do
in_module :leaks do
fact :missing_input do
input :missing_input, value: :integer
end
fact :leaky, allow_unmet_dependencies: true do
dependency :missing_input
proc { nil }
end
end
end
expect { evaluate_single_fact(:leaks, :leaky) }
.to raise_error(FactGraph::NullFactError, /:leaks\/:leaky resolved to nil/)
end

it "raises when an input fact passes through a nil value" do
Class.new(FactGraph::Graph) do
in_module :leaks do
fact :optional do
input [:wrapper, :optional]
end
end
end
expect { evaluate_single_fact(:leaks, :optional, {wrapper: {optional: nil}}) }
.to raise_error(FactGraph::NullFactError, /:leaks\/:optional resolved to nil/)
end

it "names the entity for a per-entity fact that returns nil" do
Class.new(FactGraph::Graph) do
in_module :leaks do
fact :leaky, per_entity: :items do
proc { nil }
end
end
end
expect { FactGraph::Evaluator.evaluate({items: [{}]}) }
.to raise_error(FactGraph::NullFactError, /:leaks\/:leaky \(entity 0\)/)
end

it "does not raise when a fact resolves to false" do
Class.new(FactGraph::Graph) do
in_module :ok do
fact :falsey do
proc { false }
end
end
end
result = evaluate_single_fact(:ok, :falsey)
expect(result).to eq false
end
end

describe "short-form input with no value predicates" do
before do
FactGraph::Graph.graph_registry = []
Expand Down