mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-07 08:27:12 +00:00
feat(template): resolve template error locations (#333)
## Summary Fixes `fabro-sh/fabro#330` by making template partials that reference missing inputs validate structurally with a warning instead of failing the validate command. The template crate now owns MiniJinja semantic error classification and source-location mapping, so workflow diagnostics can consume already-resolved template locations instead of remapping fragment spans itself. ## What Changed - Added `TemplateErrorLocation` and `TemplateSourceOrigin` APIs to report source name, line, column, and span from `fabro-template`. - Classified wrapped MiniJinja errors by their deepest semantic cause, preserving the original source chain for renderer context. - Added fragment-origin rendering paths so attribute fragments embedded in full workflow source report locations in the original source text. - Removed workflow-side source span remapping from template diagnostics; workflow now only adds owner, node/edge, severity, rule, and fix context. - Added regression coverage for include/import/from/extends undefined variables and the CLI `fabro validate` partial fixture. ## Test Plan - `cargo nextest run -p fabro-template` - `cargo nextest run -p fabro-workflow transforms::variable_expansion transforms::file_inlining` - `cargo nextest run -p fabro-cli --test it cmd::validate` - `cargo +nightly-2026-04-14 fmt --check --all` - `cargo +nightly-2026-04-14 clippy -p fabro-template -p fabro-workflow -p fabro-cli --all-targets -- -D warnings` --- [](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 (context not reported, reasoning not reported) via [Codex](https://openai.com/codex)
This commit is contained in:
parent
bf96baa9f0
commit
15ff38fa53
9 changed files with 532 additions and 159 deletions
|
|
@ -190,6 +190,27 @@ fn bare_fabro_with_unbound_inputs_in_imported_prompt_validates_structurally_with
|
|||
");
|
||||
}
|
||||
|
||||
/// Regression: https://github.com/fabro-sh/fabro/issues/330
|
||||
///
|
||||
/// Undefined template variables in partials included by an imported prompt must
|
||||
/// surface as structural validation warnings, matching direct `@file` prompts.
|
||||
#[test]
|
||||
fn bare_fabro_with_unbound_inputs_in_template_partial_validates_structurally_with_warning() {
|
||||
let context = test_context!();
|
||||
let mut cmd = context.validate();
|
||||
cmd.arg(fixture("templated_unbound_partial/workflow.fabro"));
|
||||
fabro_snapshot!(context.filters(), cmd, @"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
----- stderr -----
|
||||
Workflow: TemplatedUnboundPartial (3 nodes, 2 edges)
|
||||
Graph: [FIXTURES]/templated_unbound_partial/workflow.fabro
|
||||
warning: [FIXTURES]/templated_unbound_partial/test-include.partial.md:1:4: undefined template variable `inputs.hello` in node `test_imported_include` attribute `prompt` [node: test_imported_include] (template_undefined_variable)
|
||||
Validation: OK
|
||||
");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bare_fabro_picks_up_sibling_workflow_toml_inputs() {
|
||||
let context = test_context!();
|
||||
|
|
|
|||
|
|
@ -18,7 +18,8 @@ pub use dependency::{
|
|||
};
|
||||
pub use store::{
|
||||
BundleTemplateStore, CachedTemplateStore, FilesystemTemplateStore, RecordingTemplateStore,
|
||||
TemplateIncludeResolver, TemplateLoadError, TemplateSource, TemplateStore,
|
||||
TemplateIncludeResolver, TemplateLoadError, TemplateSource, TemplateSourceOrigin,
|
||||
TemplateStore,
|
||||
};
|
||||
|
||||
pub type TemplateLoader = Arc<dyn Fn(&str) -> Option<String> + Send + Sync>;
|
||||
|
|
@ -38,6 +39,15 @@ impl TemplateRenderMode {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, Default, PartialEq, Eq)]
|
||||
pub struct TemplateErrorLocation {
|
||||
pub source_name: Option<String>,
|
||||
pub line: Option<u32>,
|
||||
pub column: Option<u32>,
|
||||
pub span_start: Option<usize>,
|
||||
pub span_len: Option<usize>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct TemplateContext {
|
||||
goal: Option<String>,
|
||||
|
|
@ -159,26 +169,26 @@ pub enum TemplateError {
|
|||
Syntax {
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
source_text: Option<Arc<str>>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source_code: Option<Box<NamedSource<Arc<str>>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
UndefinedVariable {
|
||||
expression: Option<String>,
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
source_text: Option<Arc<str>>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source_code: Option<Box<NamedSource<Arc<str>>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
Render {
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<String>,
|
||||
source_text: Option<Arc<str>>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<String>>>,
|
||||
source_code: Option<Box<NamedSource<Arc<str>>>>,
|
||||
source: Box<minijinja::Error>,
|
||||
},
|
||||
}
|
||||
|
|
@ -239,54 +249,158 @@ fn extract_expression(error: &minijinja::Error) -> Option<String> {
|
|||
Some(source.get(range)?.trim().to_owned())
|
||||
}
|
||||
|
||||
impl From<minijinja::Error> for TemplateError {
|
||||
fn from(error: minijinja::Error) -> Self {
|
||||
let line = error.line().and_then(|n| u32::try_from(n).ok());
|
||||
struct MiniJinjaErrorDetails {
|
||||
line: Option<u32>,
|
||||
source_name: Option<String>,
|
||||
source_text: Option<Arc<str>>,
|
||||
span: Option<SourceSpan>,
|
||||
source_code: Option<Box<NamedSource<Arc<str>>>>,
|
||||
}
|
||||
|
||||
impl MiniJinjaErrorDetails {
|
||||
fn from_error(error: &minijinja::Error, origin: Option<&TemplateSourceOrigin>) -> Self {
|
||||
let source_name = error.name().map(str::to_owned);
|
||||
let source_text = error.template_source().map(str::to_owned);
|
||||
let span = error.range().and_then(|range| {
|
||||
let mut source_text = error.template_source().map(Arc::<str>::from);
|
||||
let mut line = error.line().and_then(|n| u32::try_from(n).ok());
|
||||
let mut span_start = None;
|
||||
let mut span_len = None;
|
||||
|
||||
if let Some(range) = error.range() {
|
||||
let start = range.start;
|
||||
let len = range.end.checked_sub(range.start)?;
|
||||
Some((start, len).into())
|
||||
});
|
||||
let len = range.end.checked_sub(range.start);
|
||||
if let Some(len) = len {
|
||||
span_start = Some(start);
|
||||
span_len = Some(len);
|
||||
}
|
||||
}
|
||||
|
||||
if let (Some(origin), Some(fragment_start), Some(len)) = (origin, span_start, span_len) {
|
||||
if let Some(start) = origin.fragment_start().checked_add(fragment_start) {
|
||||
if let Some((origin_line, _)) = source_position(origin.source_text(), start) {
|
||||
source_text = Some(origin.clone_source_text());
|
||||
line = Some(origin_line);
|
||||
span_start = Some(start);
|
||||
span_len = Some(len);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let span = span_start
|
||||
.zip(span_len)
|
||||
.map(|(start, len)| (start, len).into());
|
||||
let source_code = source_name
|
||||
.as_ref()
|
||||
.zip(source_text.as_ref())
|
||||
.map(|(name, source)| Box::new(NamedSource::new(name.clone(), source.clone())));
|
||||
match error.kind() {
|
||||
ErrorKind::SyntaxError => Self::Syntax {
|
||||
line,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
},
|
||||
ErrorKind::UndefinedError => {
|
||||
let expression = extract_expression(&error);
|
||||
Self::UndefinedVariable {
|
||||
expression,
|
||||
line,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
}
|
||||
}
|
||||
_ => Self::Render {
|
||||
line,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
source: Box::new(error),
|
||||
},
|
||||
.map(|(name, source)| Box::new(NamedSource::new(name.clone(), Arc::clone(source))));
|
||||
Self {
|
||||
line,
|
||||
source_name,
|
||||
source_text,
|
||||
span,
|
||||
source_code,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn source_position(source_text: &str, offset: usize) -> Option<(u32, u32)> {
|
||||
if offset > source_text.len() || !source_text.is_char_boundary(offset) {
|
||||
return None;
|
||||
}
|
||||
let line = source_text[..offset]
|
||||
.bytes()
|
||||
.filter(|byte| *byte == b'\n')
|
||||
.count()
|
||||
+ 1;
|
||||
let line_start = source_text[..offset]
|
||||
.rfind('\n')
|
||||
.map_or(0, |index| index + 1);
|
||||
let column = source_text[line_start..offset].chars().count() + 1;
|
||||
Some((u32::try_from(line).ok()?, u32::try_from(column).ok()?))
|
||||
}
|
||||
|
||||
fn primary_template_error(error: &minijinja::Error) -> &minijinja::Error {
|
||||
let mut selected = matches!(
|
||||
error.kind(),
|
||||
ErrorKind::SyntaxError | ErrorKind::UndefinedError
|
||||
)
|
||||
.then_some(error);
|
||||
|
||||
let mut current = error as &(dyn std::error::Error + 'static);
|
||||
while let Some(source) = current.source() {
|
||||
if let Some(template_error) = source.downcast_ref::<minijinja::Error>() {
|
||||
if matches!(
|
||||
template_error.kind(),
|
||||
ErrorKind::SyntaxError | ErrorKind::UndefinedError
|
||||
) {
|
||||
selected = Some(template_error);
|
||||
}
|
||||
}
|
||||
current = source;
|
||||
}
|
||||
|
||||
selected.unwrap_or(error)
|
||||
}
|
||||
|
||||
/// Converts a MiniJinja error into Fabro's template boundary error.
|
||||
///
|
||||
/// MiniJinja wraps semantic failures with operation-specific errors for
|
||||
/// include/import/extends rendering. Fabro classifies by the deepest semantic
|
||||
/// MiniJinja cause while storing the original outer error as the source, so
|
||||
/// renderers that walk the chain still show wrapper context.
|
||||
impl From<minijinja::Error> for TemplateError {
|
||||
fn from(error: minijinja::Error) -> Self {
|
||||
Self::from_minijinja(error, None)
|
||||
}
|
||||
}
|
||||
|
||||
impl TemplateError {
|
||||
fn from_minijinja(
|
||||
error: minijinja::Error,
|
||||
origin: Option<(&str, &TemplateSourceOrigin)>,
|
||||
) -> Self {
|
||||
let primary = primary_template_error(&error);
|
||||
let primary_origin = origin.and_then(|(source_name, origin)| {
|
||||
(primary.name() == Some(source_name)).then_some(origin)
|
||||
});
|
||||
let details = MiniJinjaErrorDetails::from_error(primary, primary_origin);
|
||||
match primary.kind() {
|
||||
ErrorKind::SyntaxError => Self::Syntax {
|
||||
line: details.line,
|
||||
source_name: details.source_name,
|
||||
source_text: details.source_text,
|
||||
span: details.span,
|
||||
source_code: details.source_code,
|
||||
source: Box::new(error),
|
||||
},
|
||||
ErrorKind::UndefinedError => {
|
||||
let expression = extract_expression(primary);
|
||||
Self::UndefinedVariable {
|
||||
expression,
|
||||
line: details.line,
|
||||
source_name: details.source_name,
|
||||
source_text: details.source_text,
|
||||
span: details.span,
|
||||
source_code: details.source_code,
|
||||
source: Box::new(error),
|
||||
}
|
||||
}
|
||||
_ => {
|
||||
let render_origin = origin.and_then(|(source_name, origin)| {
|
||||
(error.name() == Some(source_name)).then_some(origin)
|
||||
});
|
||||
let details = MiniJinjaErrorDetails::from_error(&error, render_origin);
|
||||
Self::Render {
|
||||
line: details.line,
|
||||
source_name: details.source_name,
|
||||
source_text: details.source_text,
|
||||
span: details.span,
|
||||
source_code: details.source_code,
|
||||
source: Box::new(error),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn expression(&self) -> Option<&str> {
|
||||
match self {
|
||||
|
|
@ -298,6 +412,18 @@ impl TemplateError {
|
|||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn location(&self) -> TemplateErrorLocation {
|
||||
let span = self.span();
|
||||
TemplateErrorLocation {
|
||||
source_name: self.source_name().map(ToOwned::to_owned),
|
||||
line: self.line(),
|
||||
column: self.column(),
|
||||
span_start: span.map(|span| span.offset()),
|
||||
span_len: span.map(|span| span.len()),
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn line(&self) -> Option<u32> {
|
||||
match self {
|
||||
|
|
@ -344,16 +470,10 @@ impl TemplateError {
|
|||
pub fn column(&self) -> Option<u32> {
|
||||
let source_text = self.source_text()?;
|
||||
let offset = self.span()?.offset();
|
||||
if offset > source_text.len() || !source_text.is_char_boundary(offset) {
|
||||
return None;
|
||||
}
|
||||
let line_start = source_text[..offset]
|
||||
.rfind('\n')
|
||||
.map_or(0, |index| index + 1);
|
||||
u32::try_from(source_text[line_start..offset].chars().count() + 1).ok()
|
||||
source_position(source_text, offset).map(|(_, column)| column)
|
||||
}
|
||||
|
||||
fn source_code_ref(&self) -> Option<&NamedSource<String>> {
|
||||
fn source_code_ref(&self) -> Option<&NamedSource<Arc<str>>> {
|
||||
match self {
|
||||
Self::LoaderDependentString { .. } | Self::Load { .. } => None,
|
||||
Self::Syntax { source_code, .. }
|
||||
|
|
@ -411,7 +531,7 @@ fn is_plain_text(template: &str) -> bool {
|
|||
}
|
||||
|
||||
pub fn render(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(None, template, ctx, UndefinedBehavior::Strict, None)
|
||||
render_with(None, template, ctx, UndefinedBehavior::Strict, None, None)
|
||||
}
|
||||
|
||||
pub fn render_named(
|
||||
|
|
@ -419,12 +539,39 @@ pub fn render_named(
|
|||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(
|
||||
Some(name.into()),
|
||||
render_named_with_origin(name, template, ctx, TemplateRenderMode::Strict, None)
|
||||
}
|
||||
|
||||
pub fn render_named_fragment(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
origin: &TemplateSourceOrigin,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_named_with_origin(
|
||||
name,
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Strict,
|
||||
TemplateRenderMode::Strict,
|
||||
Some(origin),
|
||||
)
|
||||
}
|
||||
|
||||
pub fn render_named_with_origin(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
mode: TemplateRenderMode,
|
||||
origin: Option<&TemplateSourceOrigin>,
|
||||
) -> Result<String, TemplateError> {
|
||||
let name = name.into();
|
||||
render_with(
|
||||
Some(&name),
|
||||
template,
|
||||
ctx,
|
||||
mode.undefined_behavior(),
|
||||
None,
|
||||
origin,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -434,12 +581,14 @@ pub fn render_named_with_loader(
|
|||
ctx: &TemplateContext,
|
||||
loader: &TemplateLoader,
|
||||
) -> Result<String, TemplateError> {
|
||||
let name = name.into();
|
||||
render_with(
|
||||
Some(name.into()),
|
||||
Some(&name),
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Strict,
|
||||
Some(loader),
|
||||
None,
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -448,7 +597,14 @@ pub fn render_named_with_loader(
|
|||
/// passes (e.g. manifest scanning, `fabro validate` on a bare `.fabro`) where
|
||||
/// the user has not yet bound inputs — strict checking happens elsewhere.
|
||||
pub fn render_lenient(template: &str, ctx: &TemplateContext) -> Result<String, TemplateError> {
|
||||
render_with(None, template, ctx, UndefinedBehavior::Chainable, None)
|
||||
render_with(
|
||||
None,
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Chainable,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
}
|
||||
|
||||
pub fn render_lenient_named(
|
||||
|
|
@ -456,12 +612,21 @@ pub fn render_lenient_named(
|
|||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_with(
|
||||
Some(name.into()),
|
||||
render_named_with_origin(name, template, ctx, TemplateRenderMode::Lenient, None)
|
||||
}
|
||||
|
||||
pub fn render_lenient_named_fragment(
|
||||
name: impl Into<String>,
|
||||
template: &str,
|
||||
origin: &TemplateSourceOrigin,
|
||||
ctx: &TemplateContext,
|
||||
) -> Result<String, TemplateError> {
|
||||
render_named_with_origin(
|
||||
name,
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Chainable,
|
||||
None,
|
||||
TemplateRenderMode::Lenient,
|
||||
Some(origin),
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -471,27 +636,30 @@ pub fn render_lenient_named_with_loader(
|
|||
ctx: &TemplateContext,
|
||||
loader: &TemplateLoader,
|
||||
) -> Result<String, TemplateError> {
|
||||
let name = name.into();
|
||||
render_with(
|
||||
Some(name.into()),
|
||||
Some(&name),
|
||||
template,
|
||||
ctx,
|
||||
UndefinedBehavior::Chainable,
|
||||
Some(loader),
|
||||
None,
|
||||
)
|
||||
}
|
||||
|
||||
fn render_with(
|
||||
name: Option<String>,
|
||||
name: Option<&str>,
|
||||
template: &str,
|
||||
ctx: &TemplateContext,
|
||||
undefined: UndefinedBehavior,
|
||||
loader: Option<&TemplateLoader>,
|
||||
origin: Option<&TemplateSourceOrigin>,
|
||||
) -> Result<String, TemplateError> {
|
||||
if is_plain_text(template) {
|
||||
return Ok(template.to_owned());
|
||||
}
|
||||
if loader.is_none() {
|
||||
reject_loader_dependent_string(name.as_deref(), template)?;
|
||||
reject_loader_dependent_string(name, template)?;
|
||||
}
|
||||
let mut env = Environment::new();
|
||||
env.set_undefined_behavior(undefined);
|
||||
|
|
@ -501,11 +669,12 @@ fn render_with(
|
|||
let loader = Arc::clone(loader);
|
||||
env.set_loader(move |name| Ok(loader(name)));
|
||||
}
|
||||
let origin = name.zip(origin);
|
||||
match name {
|
||||
Some(name) => env.render_named_str(&name, template, ctx.clone().into_value()),
|
||||
Some(name) => env.render_named_str(name, template, ctx.clone().into_value()),
|
||||
None => env.render_str(template, ctx.clone().into_value()),
|
||||
}
|
||||
.map_err(TemplateError::from)
|
||||
.map_err(|error| TemplateError::from_minijinja(error, origin))
|
||||
}
|
||||
|
||||
pub fn render_source(
|
||||
|
|
@ -555,25 +724,26 @@ fn render_rooted_source(
|
|||
}
|
||||
});
|
||||
|
||||
env.render_named_str(
|
||||
&source.path.to_string(),
|
||||
&source.content,
|
||||
ctx.clone().into_value(),
|
||||
)
|
||||
.map_err(|error| {
|
||||
if let Some(error) = load_error
|
||||
.lock()
|
||||
.expect("template load error mutex should not be poisoned")
|
||||
.take()
|
||||
{
|
||||
TemplateError::Load {
|
||||
source_name: Some(source.path.to_string()),
|
||||
source: Box::new(error),
|
||||
let source_name = source.path.to_string();
|
||||
let origin = source
|
||||
.origin
|
||||
.as_ref()
|
||||
.map(|origin| (source_name.as_str(), origin));
|
||||
env.render_named_str(&source_name, &source.content, ctx.clone().into_value())
|
||||
.map_err(|error| {
|
||||
if let Some(error) = load_error
|
||||
.lock()
|
||||
.expect("template load error mutex should not be poisoned")
|
||||
.take()
|
||||
{
|
||||
TemplateError::Load {
|
||||
source_name: Some(source.path.to_string()),
|
||||
source: Box::new(error),
|
||||
}
|
||||
} else {
|
||||
TemplateError::from_minijinja(error, origin)
|
||||
}
|
||||
} else {
|
||||
TemplateError::from(error)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
fn joined_template_path(root: &ManifestPath, name: &str, parent: &str) -> String {
|
||||
|
|
@ -601,6 +771,7 @@ mod tests {
|
|||
use std::collections::HashMap;
|
||||
|
||||
use fabro_util::env::TestEnv;
|
||||
use fabro_util::error;
|
||||
use toml::map::Map;
|
||||
|
||||
use super::*;
|
||||
|
|
@ -764,6 +935,162 @@ mod tests {
|
|||
assert_eq!(rendered, "included content");
|
||||
}
|
||||
|
||||
fn assert_semantic_undefined_error(
|
||||
err: &TemplateError,
|
||||
expected_source: &str,
|
||||
expected_source_text: &str,
|
||||
) {
|
||||
let TemplateError::UndefinedVariable { expression, .. } = err else {
|
||||
panic!("expected undefined variable error, got {err:?}");
|
||||
};
|
||||
assert_eq!(expression.as_deref(), Some("inputs.hello"));
|
||||
|
||||
let location = err.location();
|
||||
assert_eq!(location.source_name.as_deref(), Some(expected_source));
|
||||
assert_eq!(location.line, Some(1));
|
||||
assert_eq!(
|
||||
location.span_start,
|
||||
expected_source_text.find("inputs.hello")
|
||||
);
|
||||
assert_eq!(location.span_len, Some("inputs.hello".len()));
|
||||
|
||||
let chain = error::collect_chain(err);
|
||||
assert!(
|
||||
chain.iter().any(|cause| cause.contains("undefined value")),
|
||||
"missing undefined cause in source chain: {chain:?}"
|
||||
);
|
||||
assert!(
|
||||
chain
|
||||
.iter()
|
||||
.skip(1)
|
||||
.any(|cause| cause.contains(expected_source)),
|
||||
"missing source context in source chain: {chain:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_source_reports_undefined_variable_from_include() {
|
||||
let ctx = TemplateContext::new();
|
||||
let source = TemplateSource::new(
|
||||
manifest_path("prompts/main.md"),
|
||||
manifest_path("prompts"),
|
||||
r#"{% include "partial.md" %}"#,
|
||||
);
|
||||
|
||||
let err = render_source(
|
||||
&source,
|
||||
&ctx,
|
||||
bundle_store(&[("prompts/partial.md", "{{ inputs.hello }}")]),
|
||||
TemplateRenderMode::Strict,
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_semantic_undefined_error(&err, "prompts/partial.md", "{{ inputs.hello }}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_source_reports_undefined_variable_from_imported_macro() {
|
||||
let ctx = TemplateContext::new();
|
||||
let source = TemplateSource::new(
|
||||
manifest_path("prompts/main.md"),
|
||||
manifest_path("prompts"),
|
||||
r#"{% import "macros.md" as macros %}{{ macros.greet() }}"#,
|
||||
);
|
||||
|
||||
let err = render_source(
|
||||
&source,
|
||||
&ctx,
|
||||
bundle_store(&[(
|
||||
"prompts/macros.md",
|
||||
r"{% macro greet() %}{{ inputs.hello }}{% endmacro %}",
|
||||
)]),
|
||||
TemplateRenderMode::Strict,
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_semantic_undefined_error(
|
||||
&err,
|
||||
"prompts/macros.md",
|
||||
r"{% macro greet() %}{{ inputs.hello }}{% endmacro %}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_source_reports_undefined_variable_from_from_imported_macro() {
|
||||
let ctx = TemplateContext::new();
|
||||
let source = TemplateSource::new(
|
||||
manifest_path("prompts/main.md"),
|
||||
manifest_path("prompts"),
|
||||
r#"{% from "macros.md" import greet %}{{ greet() }}"#,
|
||||
);
|
||||
|
||||
let err = render_source(
|
||||
&source,
|
||||
&ctx,
|
||||
bundle_store(&[(
|
||||
"prompts/macros.md",
|
||||
r"{% macro greet() %}{{ inputs.hello }}{% endmacro %}",
|
||||
)]),
|
||||
TemplateRenderMode::Strict,
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_semantic_undefined_error(
|
||||
&err,
|
||||
"prompts/macros.md",
|
||||
r"{% macro greet() %}{{ inputs.hello }}{% endmacro %}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_source_reports_undefined_variable_from_extended_layout() {
|
||||
let ctx = TemplateContext::new();
|
||||
let source = TemplateSource::new(
|
||||
manifest_path("pages/main.md"),
|
||||
manifest_path("pages"),
|
||||
r#"{% extends "layout.md" %}{% block body %}Body{% endblock %}"#,
|
||||
);
|
||||
|
||||
let err = render_source(
|
||||
&source,
|
||||
&ctx,
|
||||
bundle_store(&[(
|
||||
"pages/layout.md",
|
||||
"{{ inputs.hello }}:{% block body %}{% endblock %}",
|
||||
)]),
|
||||
TemplateRenderMode::Strict,
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_semantic_undefined_error(
|
||||
&err,
|
||||
"pages/layout.md",
|
||||
"{{ inputs.hello }}:{% block body %}{% endblock %}",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_named_fragment_reports_location_in_full_source() {
|
||||
let ctx = TemplateContext::new();
|
||||
let source_text = "digraph {\n plan [prompt=\"Hello {{ inputs.name }}\"]\n}\n";
|
||||
let fragment = "Hello {{ inputs.name }}";
|
||||
let origin = TemplateSourceOrigin::from_first_fragment_match(source_text, fragment)
|
||||
.expect("fragment should be present in source");
|
||||
|
||||
let err = render_named_fragment("workflow.fabro", fragment, &origin, &ctx).unwrap_err();
|
||||
|
||||
let location = err.location();
|
||||
assert_eq!(location.source_name.as_deref(), Some("workflow.fabro"));
|
||||
assert_eq!(location.line, Some(2));
|
||||
assert_eq!(location.column, Some(26));
|
||||
assert_eq!(location.span_start, source_text.find("inputs.name"));
|
||||
assert_eq!(location.span_len, Some("inputs.name".len()));
|
||||
assert_eq!(
|
||||
err.span().map(|span| span.offset()),
|
||||
source_text.find("inputs.name")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn render_source_supports_nested_include() {
|
||||
let ctx = TemplateContext::new();
|
||||
|
|
|
|||
|
|
@ -1,15 +1,54 @@
|
|||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::{Mutex, MutexGuard};
|
||||
use std::sync::{Arc, Mutex, MutexGuard};
|
||||
|
||||
use fabro_types::ManifestPath;
|
||||
use thiserror::Error;
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct TemplateSourceOrigin {
|
||||
source_text: Arc<str>,
|
||||
fragment_start: usize,
|
||||
}
|
||||
|
||||
impl TemplateSourceOrigin {
|
||||
#[must_use]
|
||||
fn new(source_text: impl Into<Arc<str>>, fragment_start: usize) -> Self {
|
||||
Self {
|
||||
source_text: source_text.into(),
|
||||
fragment_start,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn from_first_fragment_match(source_text: &str, fragment: &str) -> Option<Self> {
|
||||
source_text
|
||||
.find(fragment)
|
||||
.map(|fragment_start| Self::new(source_text, fragment_start))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn source_text(&self) -> &str {
|
||||
&self.source_text
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn clone_source_text(&self) -> Arc<str> {
|
||||
Arc::clone(&self.source_text)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn fragment_start(&self) -> usize {
|
||||
self.fragment_start
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq, Eq)]
|
||||
pub struct TemplateSource {
|
||||
pub path: ManifestPath,
|
||||
pub root: ManifestPath,
|
||||
pub content: String,
|
||||
pub origin: Option<TemplateSourceOrigin>,
|
||||
}
|
||||
|
||||
impl TemplateSource {
|
||||
|
|
@ -19,8 +58,15 @@ impl TemplateSource {
|
|||
path,
|
||||
root,
|
||||
content: content.into(),
|
||||
origin: None,
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_origin(mut self, origin: TemplateSourceOrigin) -> Self {
|
||||
self.origin = Some(origin);
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
pub trait TemplateStore: Send + Sync {
|
||||
|
|
|
|||
|
|
@ -200,7 +200,7 @@ impl FileInliningTransform {
|
|||
node_id.clone(),
|
||||
"prompt",
|
||||
)
|
||||
.with_source_text(self.source_text.as_deref(), prompt)
|
||||
.with_source_origin(self.source_text.as_deref(), prompt)
|
||||
.with_template_store(template_render_store(
|
||||
&self.current_dir,
|
||||
Arc::clone(&self.resolver),
|
||||
|
|
@ -234,7 +234,7 @@ impl FileInliningTransform {
|
|||
};
|
||||
let ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), goal)
|
||||
.with_source_origin(self.source_text.as_deref(), goal)
|
||||
.with_template_store(template_render_store(
|
||||
&self.current_dir,
|
||||
Arc::clone(&self.resolver),
|
||||
|
|
@ -270,7 +270,7 @@ impl FileInliningTransform {
|
|||
let (source, store) = self.template_source_for_resolved_file(&resolved)?;
|
||||
let target = owner_target
|
||||
.with_source_name(resolved.path.display().to_string())
|
||||
.with_source_text(Some(&resolved.content), &resolved.content)
|
||||
.with_source_origin(Some(&resolved.content), &resolved.content)
|
||||
.with_template_store(TemplateRenderStore::new(source, store));
|
||||
Ok(Some(render_file_contents(
|
||||
&resolved,
|
||||
|
|
|
|||
|
|
@ -675,7 +675,7 @@ impl ImportTransform {
|
|||
let path_ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
let mut ignored_goal_diagnostics = Vec::new();
|
||||
let goal_target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), graph.goal())
|
||||
.with_source_origin(self.source_text.as_deref(), graph.goal())
|
||||
.with_template_store(template_render_store(
|
||||
&self.current_dir,
|
||||
Arc::clone(&self.resolver),
|
||||
|
|
@ -700,7 +700,7 @@ impl ImportTransform {
|
|||
placeholder_id.clone(),
|
||||
"import",
|
||||
)
|
||||
.with_source_text(self.source_text.as_deref(), &import_path);
|
||||
.with_source_origin(self.source_text.as_deref(), &import_path);
|
||||
let rendered_import_path = render_template_for_target(
|
||||
&import_path,
|
||||
&path_ctx,
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ use std::sync::Arc;
|
|||
|
||||
use fabro_graphviz::graph::{AttrValue, Graph};
|
||||
use fabro_template::{
|
||||
TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateStore,
|
||||
render_lenient_named, render_named, render_source,
|
||||
TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin,
|
||||
TemplateStore, render_named_with_origin, render_source,
|
||||
};
|
||||
use fabro_util::error::collect_chain;
|
||||
use fabro_validate::{Diagnostic, Severity};
|
||||
|
|
@ -35,13 +35,12 @@ pub enum RenderMode {
|
|||
|
||||
#[derive(Clone)]
|
||||
pub(crate) struct TemplateRenderTarget {
|
||||
pub source_name: Option<String>,
|
||||
pub source_text: Option<String>,
|
||||
pub source_offset: Option<usize>,
|
||||
pub node_id: Option<String>,
|
||||
pub edge: Option<(String, String)>,
|
||||
pub owner: String,
|
||||
template_store: Option<TemplateRenderStore>,
|
||||
pub source_name: Option<String>,
|
||||
pub node_id: Option<String>,
|
||||
pub edge: Option<(String, String)>,
|
||||
pub owner: String,
|
||||
source_origin: Option<TemplateSourceOrigin>,
|
||||
template_store: Option<TemplateRenderStore>,
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
|
|
@ -61,8 +60,12 @@ impl TemplateRenderStore {
|
|||
text: &str,
|
||||
ctx: &TemplateContext,
|
||||
mode: TemplateRenderMode,
|
||||
origin: Option<&TemplateSourceOrigin>,
|
||||
) -> Result<String, TemplateError> {
|
||||
let mut source = self.source.clone();
|
||||
let mut source = match origin {
|
||||
Some(origin) => self.source.clone().with_origin(origin.clone()),
|
||||
None => self.source.clone(),
|
||||
};
|
||||
text.clone_into(&mut source.content);
|
||||
render_source(&source, ctx, Arc::clone(&self.store), mode)
|
||||
}
|
||||
|
|
@ -74,11 +77,10 @@ impl TemplateRenderTarget {
|
|||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: None,
|
||||
edge: None,
|
||||
owner: format!("graph attribute `{attr_name}`"),
|
||||
source_origin: None,
|
||||
template_store: None,
|
||||
}
|
||||
}
|
||||
|
|
@ -93,11 +95,10 @@ impl TemplateRenderTarget {
|
|||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: Some(node_id.clone()),
|
||||
edge: None,
|
||||
owner: format!("node `{node_id}` attribute `{attr_name}`"),
|
||||
source_origin: None,
|
||||
template_store: None,
|
||||
}
|
||||
}
|
||||
|
|
@ -114,11 +115,10 @@ impl TemplateRenderTarget {
|
|||
let attr_name = attr_name.into();
|
||||
Self {
|
||||
source_name,
|
||||
source_text: None,
|
||||
source_offset: None,
|
||||
node_id: None,
|
||||
edge: Some((from.clone(), to.clone())),
|
||||
owner: format!("edge `{from} -> {to}` attribute `{attr_name}`"),
|
||||
source_origin: None,
|
||||
template_store: None,
|
||||
}
|
||||
}
|
||||
|
|
@ -130,9 +130,10 @@ impl TemplateRenderTarget {
|
|||
}
|
||||
|
||||
#[must_use]
|
||||
pub(crate) fn with_source_text(mut self, source_text: Option<&str>, value: &str) -> Self {
|
||||
self.source_text = source_text.map(ToOwned::to_owned);
|
||||
self.source_offset = source_text.and_then(|source_text| source_text.find(value));
|
||||
pub(crate) fn with_source_origin(mut self, source_text: Option<&str>, value: &str) -> Self {
|
||||
self.source_origin = source_text.and_then(|source_text| {
|
||||
TemplateSourceOrigin::from_first_fragment_match(source_text, value)
|
||||
});
|
||||
self
|
||||
}
|
||||
|
||||
|
|
@ -159,11 +160,16 @@ pub(crate) fn render_template_for_target(
|
|||
) -> Result<String, Error> {
|
||||
let source_name = target.template_source_name();
|
||||
let render_with_mode = |mode| match target.template_store.as_ref() {
|
||||
Some(template_store) => template_store.render(text, ctx, mode),
|
||||
None if matches!(mode, TemplateRenderMode::Strict) => {
|
||||
render_named(source_name.clone(), text, ctx)
|
||||
Some(template_store) => {
|
||||
template_store.render(text, ctx, mode, target.source_origin.as_ref())
|
||||
}
|
||||
None => render_lenient_named(source_name.clone(), text, ctx),
|
||||
None => render_named_with_origin(
|
||||
source_name.clone(),
|
||||
text,
|
||||
ctx,
|
||||
mode,
|
||||
target.source_origin.as_ref(),
|
||||
),
|
||||
};
|
||||
match render_mode {
|
||||
RenderMode::Strict => render_with_mode(TemplateRenderMode::Strict)
|
||||
|
|
@ -197,16 +203,7 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) ->
|
|||
};
|
||||
let _ = write!(message, " in {}", target.owner);
|
||||
|
||||
let source_location = target
|
||||
.source_text
|
||||
.as_deref()
|
||||
.zip(target.source_offset)
|
||||
.zip(error.span())
|
||||
.and_then(|((source_text, source_offset), span)| {
|
||||
let absolute_offset = source_offset.checked_add(span.offset())?;
|
||||
let (line, column) = source_position(source_text, absolute_offset)?;
|
||||
Some((line, column, absolute_offset, span.len()))
|
||||
});
|
||||
let location = error.location();
|
||||
|
||||
Diagnostic {
|
||||
rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(),
|
||||
|
|
@ -217,42 +214,15 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) ->
|
|||
fix: Some(format!(
|
||||
"bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=<value>`"
|
||||
)),
|
||||
source_path: error
|
||||
.source_name()
|
||||
.map(ToOwned::to_owned)
|
||||
.or_else(|| target.source_name.clone()),
|
||||
line: source_location
|
||||
.map(|(line, _, _, _)| line)
|
||||
.or_else(|| error.line()),
|
||||
column: source_location
|
||||
.map(|(_, column, _, _)| column)
|
||||
.or_else(|| error.column()),
|
||||
span_start: source_location
|
||||
.map(|(_, _, span_start, _)| span_start)
|
||||
.or_else(|| error.span().map(|span| span.offset())),
|
||||
span_len: source_location
|
||||
.map(|(_, _, _, span_len)| span_len)
|
||||
.or_else(|| error.span().map(|span| span.len())),
|
||||
source_path: location.source_name.or_else(|| target.source_name.clone()),
|
||||
line: location.line,
|
||||
column: location.column,
|
||||
span_start: location.span_start,
|
||||
span_len: location.span_len,
|
||||
related: Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn source_position(source_text: &str, offset: usize) -> Option<(u32, u32)> {
|
||||
if offset > source_text.len() || !source_text.is_char_boundary(offset) {
|
||||
return None;
|
||||
}
|
||||
let line = source_text[..offset]
|
||||
.bytes()
|
||||
.filter(|byte| *byte == b'\n')
|
||||
.count()
|
||||
+ 1;
|
||||
let line_start = source_text[..offset]
|
||||
.rfind('\n')
|
||||
.map_or(0, |index| index + 1);
|
||||
let column = source_text[line_start..offset].chars().count() + 1;
|
||||
Some((u32::try_from(line).ok()?, u32::try_from(column).ok()?))
|
||||
}
|
||||
|
||||
/// Expands `{{ goal }}` / `{{ inputs.* }}` across all string attributes.
|
||||
pub struct TemplateTransform {
|
||||
pub inputs: HashMap<String, toml::Value>,
|
||||
|
|
@ -285,7 +255,7 @@ impl TemplateTransform {
|
|||
}
|
||||
let ctx = TemplateContext::for_input_scan(self.inputs.clone());
|
||||
let target = TemplateRenderTarget::graph_attr(self.source_name.clone(), "goal")
|
||||
.with_source_text(self.source_text.as_deref(), goal);
|
||||
.with_source_origin(self.source_text.as_deref(), goal);
|
||||
render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)
|
||||
}
|
||||
|
||||
|
|
@ -314,7 +284,7 @@ impl TemplateTransform {
|
|||
}
|
||||
let target = owner_for_attr(attr_name)
|
||||
.with_source_name(source_name.cloned().unwrap_or_else(|| "workflow".into()))
|
||||
.with_source_text(source_text, text);
|
||||
.with_source_origin(source_text, text);
|
||||
*text = render_template_for_target(text, ctx, render_mode, &target, diagnostics)?;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
1
test/templated_unbound_partial/test-include.partial.md
Normal file
1
test/templated_unbound_partial/test-include.partial.md
Normal file
|
|
@ -0,0 +1 @@
|
|||
{{ inputs.hello }}
|
||||
1
test/templated_unbound_partial/test-include.prompt.md
Normal file
1
test/templated_unbound_partial/test-include.prompt.md
Normal file
|
|
@ -0,0 +1 @@
|
|||
{% include "test-include.partial.md" %}
|
||||
7
test/templated_unbound_partial/workflow.fabro
Normal file
7
test/templated_unbound_partial/workflow.fabro
Normal file
|
|
@ -0,0 +1,7 @@
|
|||
digraph TemplatedUnboundPartial {
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
test_imported_include [label="Test", prompt="@test-include.prompt.md"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
|
||||
start -> test_imported_include -> exit
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue