diff --git a/README.md b/README.md index 1cc04c9..046b18d 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/lib/fact_graph.rb b/lib/fact_graph.rb index 6639f0b..9c3de0f 100644 --- a/lib/fact_graph.rb +++ b/lib/fact_graph.rb @@ -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 = [] diff --git a/lib/fact_graph/fact.rb b/lib/fact_graph/fact.rb index dd72557..d7d8cc8 100644 --- a/lib/fact_graph/fact.rb +++ b/lib/fact_graph/fact.rb @@ -147,6 +147,7 @@ def call(input, results) results[module_name] ||= {} if !resolver.respond_to?(:call) + forbid_nil!(resolver) results[module_name][name] = resolver return resolver end @@ -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 diff --git a/spec/fact_graph/fact_spec.rb b/spec/fact_graph/fact_spec.rb index bdb3e24..0203275 100644 --- a/spec/fact_graph/fact_spec.rb +++ b/spec/fact_graph/fact_spec.rb @@ -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 = []