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

derive: better use of spans and file information in error messages #102

Merged
merged 7 commits into from
Jul 30, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
1 change: 1 addition & 0 deletions rinja_derive/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ with-warp = []

[dependencies]
parser = { package = "rinja_parser", version = "0.2.0", path = "../rinja_parser" }
annotate-snippets = "0.11.4"
basic-toml = { version = "0.1.1", optional = true }
memchr = "2"
mime = "0.3"
Expand Down
72 changes: 44 additions & 28 deletions rinja_derive/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ impl Config {
&self,
path: &str,
start_at: Option<&Path>,
file_info: Option<FileInfo<'_>>,
) -> Result<Arc<Path>, CompileError> {
if let Some(root) = start_at {
let relative = root.with_file_name(path);
Expand All @@ -213,10 +214,13 @@ impl Config {
}
}

Err(CompileError::no_file_info(format!(
"template {:?} not found in directories {:?}",
path, self.dirs
)))
Err(CompileError::new(
format!(
"template {:?} not found in directories {:?}",
path, self.dirs
),
file_info,
))
}
}

Expand Down Expand Up @@ -319,13 +323,15 @@ impl<'a> TryInto<Syntax<'a>> for RawSyntax<'a> {
syntax.comment_end,
] {
if s.len() < 2 {
return Err(CompileError::no_file_info(format!(
"delimiters must be at least two characters long: {s:?}"
)));
return Err(CompileError::no_file_info(
format!("delimiters must be at least two characters long: {s:?}"),
None,
Kijewski marked this conversation as resolved.
Show resolved Hide resolved
));
} else if s.chars().any(|c| c.is_whitespace()) {
return Err(CompileError::no_file_info(format!(
"delimiters may not contain white spaces: {s:?}"
)));
return Err(CompileError::no_file_info(
format!("delimiters may not contain white spaces: {s:?}"),
None,
Kijewski marked this conversation as resolved.
Show resolved Hide resolved
));
}
}

Expand All @@ -335,9 +341,12 @@ impl<'a> TryInto<Syntax<'a>> for RawSyntax<'a> {
(syntax.expr_start, syntax.comment_start),
] {
if s1.starts_with(s2) || s2.starts_with(s1) {
return Err(CompileError::no_file_info(format!(
"a delimiter may not be the prefix of another delimiter: {s1:?} vs {s2:?}",
)));
return Err(CompileError::no_file_info(
format!(
"a delimiter may not be the prefix of another delimiter: {s1:?} vs {s2:?}",
),
None,
Kijewski marked this conversation as resolved.
Show resolved Hide resolved
));
}
}

Expand All @@ -358,13 +367,16 @@ impl RawConfig<'_> {
#[cfg(feature = "config")]
fn from_toml_str(s: &str) -> Result<RawConfig<'_>, CompileError> {
basic_toml::from_str(s).map_err(|e| {
CompileError::no_file_info(format!("invalid TOML in {CONFIG_FILE_NAME}: {e}"))
CompileError::no_file_info(format!("invalid TOML in {CONFIG_FILE_NAME}: {e}"), None)
})
}

#[cfg(not(feature = "config"))]
fn from_toml_str(_: &str) -> Result<RawConfig<'_>, CompileError> {
Err(CompileError::no_file_info("TOML support not available"))
Err(CompileError::no_file_info(
"TOML support not available",
None,
))
}
}

Expand Down Expand Up @@ -428,13 +440,13 @@ pub(crate) fn read_config_file(config_path: Option<&str>) -> Result<String, Comp

if filename.exists() {
fs::read_to_string(&filename).map_err(|_| {
CompileError::no_file_info(format!("unable to read {:?}", filename.to_str().unwrap()))
CompileError::no_file_info(format!("unable to read {}", filename.display()), None)
})
} else if config_path.is_some() {
Err(CompileError::no_file_info(format!(
"`{}` does not exist",
root.display()
)))
Err(CompileError::no_file_info(
format!("`{}` does not exist", root.display()),
None,
))
} else {
Ok("".to_string())
}
Expand Down Expand Up @@ -489,32 +501,36 @@ mod tests {
#[test]
fn find_absolute() {
let config = Config::new("", None, None).unwrap();
let root = config.find_template("a.html", None).unwrap();
let path = config.find_template("sub/b.html", Some(&root)).unwrap();
let root = config.find_template("a.html", None, None).unwrap();
let path = config
.find_template("sub/b.html", Some(&root), None)
.unwrap();
assert_eq_rooted(&path, "sub/b.html");
}

#[test]
#[should_panic]
fn find_relative_nonexistent() {
let config = Config::new("", None, None).unwrap();
let root = config.find_template("a.html", None).unwrap();
config.find_template("c.html", Some(&root)).unwrap();
let root = config.find_template("a.html", None, None).unwrap();
config.find_template("c.html", Some(&root), None).unwrap();
}

#[test]
fn find_relative() {
let config = Config::new("", None, None).unwrap();
let root = config.find_template("sub/b.html", None).unwrap();
let path = config.find_template("c.html", Some(&root)).unwrap();
let root = config.find_template("sub/b.html", None, None).unwrap();
let path = config.find_template("c.html", Some(&root), None).unwrap();
assert_eq_rooted(&path, "sub/c.html");
}

#[test]
fn find_relative_sub() {
let config = Config::new("", None, None).unwrap();
let root = config.find_template("sub/b.html", None).unwrap();
let path = config.find_template("sub1/d.html", Some(&root)).unwrap();
let root = config.find_template("sub/b.html", None, None).unwrap();
let path = config
.find_template("sub1/d.html", Some(&root), None)
.unwrap();
assert_eq_rooted(&path, "sub/sub1/d.html");
}

Expand Down
15 changes: 11 additions & 4 deletions rinja_derive/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::config::WhitespaceHandling;
use crate::heritage::{Context, Heritage};
use crate::html::write_escaped_str;
use crate::input::{Source, TemplateInput};
use crate::{CompileError, MsgValidEscapers, CRATE};
use crate::{CompileError, FileInfo, MsgValidEscapers, CRATE};

#[derive(Clone, Copy, PartialEq, Debug)]
enum EvaluatedResult {
Expand Down Expand Up @@ -78,7 +78,13 @@ impl<'a> Generator<'a> {
pub(crate) fn build(mut self, ctx: &Context<'a>) -> Result<String, CompileError> {
let mut buf = Buffer::new();

self.impl_template(ctx, &mut buf)?;
if let Err(mut err) = self.impl_template(ctx, &mut buf) {
if err.span.is_none() {
err.span = self.input.source_span;
}
return Err(err);
}

self.impl_display(&mut buf);

#[cfg(feature = "with-actix-web")]
Expand Down Expand Up @@ -907,14 +913,15 @@ impl<'a> Generator<'a> {
&mut self,
ctx: &Context<'a>,
buf: &mut Buffer,
i: &'a Include<'_>,
i: &'a WithSpan<'_, Include<'_>>,
) -> Result<usize, CompileError> {
self.flush_ws(i.ws);
self.write_buf_writable(ctx, buf)?;
let file_info = ctx.path.map(|path| FileInfo::of(i, path, ctx.parsed));
let path = self
.input
.config
.find_template(i.path, Some(&self.input.path))?;
.find_template(i.path, Some(&self.input.path), file_info)?;

// We clone the context of the child in order to preserve their macros and imports.
// But also add all the imports and macros from this template that don't override the
Expand Down
16 changes: 12 additions & 4 deletions rinja_derive/src/heritage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ pub(crate) struct Context<'a> {
pub(crate) blocks: HashMap<&'a str, &'a BlockDef<'a>>,
pub(crate) macros: HashMap<&'a str, &'a Macro<'a>>,
pub(crate) imports: HashMap<&'a str, Arc<Path>>,
path: Option<&'a Path>,
parsed: &'a Parsed,
pub(crate) path: Option<&'a Path>,
pub(crate) parsed: &'a Parsed,
}

impl Context<'_> {
Expand Down Expand Up @@ -84,15 +84,23 @@ impl Context<'_> {
Some(FileInfo::of(e, path, parsed)),
));
}
extends = Some(config.find_template(e.path, Some(path))?);
extends = Some(config.find_template(
e.path,
Some(path),
Some(FileInfo::of(e, path, parsed)),
)?);
}
Node::Macro(m) => {
ensure_top(top, m, path, parsed, "macro")?;
macros.insert(m.name, &**m);
}
Node::Import(import) => {
ensure_top(top, import, path, parsed, "import")?;
let path = config.find_template(import.path, Some(path))?;
let path = config.find_template(
import.path,
Some(path),
Some(FileInfo::of(import, path, parsed)),
)?;
imports.insert(import.scope, path);
}
Node::BlockDef(b) => {
Expand Down
Loading