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.

Sign up for GitHub

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

Better (post-parser) errors #1058

Closed
wants to merge 14 commits into from
Closed

Better (post-parser) errors #1058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
a33f77b
Add `generate_row_and_column` and `generate_error_info` public functi…
GuillaumeGomez May 27, 2024
e452b76
Create `askama_parser::WithSpan` type
GuillaumeGomez May 27, 2024
368fd82
Wrap `Expr` into `WithSpan`
GuillaumeGomez May 27, 2024
a2c50d1
Update `Node` variants to use `WithSpan`
GuillaumeGomez May 27, 2024
e40056e
Add `Node::span` method
GuillaumeGomez May 27, 2024
c36eeb3
Update askama_parser AST tests
GuillaumeGomez May 27, 2024
e47b973
Update to new `WithSpan` type in `askama_derive`
GuillaumeGomez May 27, 2024
a2881bb
Add `askama_derive::FileInfo` type
GuillaumeGomez May 27, 2024
debd9af
Migrate config errors to new more complete `CompileError` version
GuillaumeGomez May 27, 2024
dbfa09e
Add `Context::generate_error` method
GuillaumeGomez May 27, 2024
344b0ef
Migrate errors with file information
GuillaumeGomez May 27, 2024
326b02e
Remove `From` trait implementations for `CompileError`
GuillaumeGomez May 27, 2024
a6613b6
Update ui tests
GuillaumeGomez May 27, 2024
22d352b
Fix clippy lints
GuillaumeGomez May 27, 2024
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
133 changes: 87 additions & 46 deletions askama_derive/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{env, fs};
#[cfg(feature = "serde")]
use serde::Deserialize;

use crate::{CompileError, CRATE};
use crate::{CompileError, FileInfo, CRATE};
use parser::node::Whitespace;
use parser::Syntax;

Expand All @@ -22,6 +22,7 @@ pub(crate) struct Config<'a> {
impl<'a> Config<'a> {
pub(crate) fn new(
s: &'a str,
config_path: Option<&str>,
template_whitespace: Option<&str>,
) -> std::result::Result<Config<'a>, CompileError> {
let root = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
Expand Down Expand Up @@ -54,12 +55,18 @@ impl<'a> Config<'a> {
WhitespaceHandling::default(),
),
};
let file_info = config_path.map(|path| FileInfo::new(Path::new(path), None, None));
if let Some(template_whitespace) = template_whitespace {
whitespace = match template_whitespace {
"suppress" => WhitespaceHandling::Suppress,
"minimize" => WhitespaceHandling::Minimize,
"preserve" => WhitespaceHandling::Preserve,
s => return Err(format!("invalid value for `whitespace`: \"{s}\"").into()),
s => {
return Err(CompileError::new(
format!("invalid value for `whitespace`: \"{s}\""),
file_info,
))
}
};
}

Expand All @@ -71,13 +78,19 @@ impl<'a> Config<'a> {
.insert(name.to_string(), raw_s.try_into()?)
.is_some()
{
return Err(format!("syntax \"{name}\" is already defined").into());
return Err(CompileError::new(
format!("syntax \"{name}\" is already defined",),
file_info,
));
}
}
}

if !syntaxes.contains_key(default_syntax) {
return Err(format!("default syntax \"{default_syntax}\" not found").into());
return Err(CompileError::new(
format!("default syntax \"{default_syntax}\" not found"),
file_info,
));
}

let mut escapers = Vec::new();
Expand Down Expand Up @@ -125,11 +138,10 @@ impl<'a> Config<'a> {
}
}

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

Expand All @@ -156,11 +168,13 @@ impl<'a> TryInto<Syntax<'a>> for RawSyntax<'a> {
syntax.comment_end,
] {
if s.len() < 2 {
return Err(
format!("delimiters must be at least two characters long: {s:?}").into(),
);
return Err(CompileError::no_file_info(format!(
"delimiters must be at least two characters long: {s:?}"
)));
} else if s.chars().any(|c| c.is_whitespace()) {
return Err(format!("delimiters may not contain white spaces: {s:?}").into());
return Err(CompileError::no_file_info(format!(
"delimiters may not contain white spaces: {s:?}"
)));
}
}

Expand All @@ -170,10 +184,9 @@ 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(format!(
return Err(CompileError::no_file_info(format!(
"a delimiter may not be the prefix of another delimiter: {s1:?} vs {s2:?}",
)
.into());
)));
}
}

Expand All @@ -193,13 +206,14 @@ struct RawConfig<'a> {
impl RawConfig<'_> {
#[cfg(feature = "config")]
fn from_toml_str(s: &str) -> std::result::Result<RawConfig<'_>, CompileError> {
basic_toml::from_str(s)
.map_err(|e| format!("invalid TOML in {CONFIG_FILE_NAME}: {e}").into())
basic_toml::from_str(s).map_err(|e| {
CompileError::no_file_info(format!("invalid TOML in {CONFIG_FILE_NAME}: {e}"))
})
}

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

Expand Down Expand Up @@ -264,10 +278,14 @@ pub(crate) fn read_config_file(
};

if filename.exists() {
fs::read_to_string(&filename)
.map_err(|_| format!("unable to read {:?}", filename.to_str().unwrap()).into())
fs::read_to_string(&filename).map_err(|_| {
CompileError::no_file_info(format!("unable to read {:?}", filename.to_str().unwrap()))
})
} else if config_path.is_some() {
Err(format!("`{}` does not exist", root.display()).into())
Err(CompileError::no_file_info(format!(
"`{}` does not exist",
root.display()
)))
} else {
Ok("".to_string())
}
Expand All @@ -281,13 +299,31 @@ where
}

#[allow(clippy::match_wild_err_arm)]
pub(crate) fn get_template_source(tpl_path: &Path) -> std::result::Result<String, CompileError> {
pub(crate) fn get_template_source(
tpl_path: &Path,
import_from: Option<(&Rc<Path>, &str, &str)>,
) -> std::result::Result<String, CompileError> {
match fs::read_to_string(tpl_path) {
Err(_) => Err(format!(
"unable to open template file '{}'",
tpl_path.to_str().unwrap()
)
.into()),
Err(_) => {
if let Some((node_file, file_source, node_source)) = import_from {
Err(CompileError::new(
format!(
"unable to open template file '{}'",
tpl_path.to_str().unwrap(),
),
Some(FileInfo::new(
node_file,
Some(file_source),
Some(node_source),
)),
))
} else {
Err(CompileError::no_file_info(format!(
"unable to open template file '{}'",
tpl_path.to_str().unwrap()
)))
}
}
Ok(mut source) => {
if source.ends_with('\n') {
let _ = source.pop();
Expand All @@ -314,17 +350,17 @@ mod tests {

#[test]
fn get_source() {
let path = Config::new("", None)
let path = Config::new("", None, None)
.and_then(|config| config.find_template("b.html", None))
.unwrap();
assert_eq!(get_template_source(&path).unwrap(), "bar");
assert_eq!(get_template_source(&path, None).unwrap(), "bar");
}

#[test]
fn test_default_config() {
let mut root = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
root.push("templates");
let config = Config::new("", None).unwrap();
let config = Config::new("", None, None).unwrap();
assert_eq!(config.dirs, vec![root]);
}

Expand All @@ -333,7 +369,7 @@ mod tests {
fn test_config_dirs() {
let mut root = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
root.push("tpl");
let config = Config::new("[general]\ndirs = [\"tpl\"]", None).unwrap();
let config = Config::new("[general]\ndirs = [\"tpl\"]", None, None).unwrap();
assert_eq!(config.dirs, vec![root]);
}

Expand All @@ -347,7 +383,7 @@ mod tests {

#[test]
fn find_absolute() {
let config = Config::new("", None).unwrap();
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();
assert_eq_rooted(&path, "sub/b.html");
Expand All @@ -356,22 +392,22 @@ mod tests {
#[test]
#[should_panic]
fn find_relative_nonexistent() {
let config = Config::new("", None).unwrap();
let config = Config::new("", None, None).unwrap();
let root = config.find_template("a.html", None).unwrap();
config.find_template("c.html", Some(&root)).unwrap();
}

#[test]
fn find_relative() {
let config = Config::new("", None).unwrap();
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();
assert_eq_rooted(&path, "sub/c.html");
}

#[test]
fn find_relative_sub() {
let config = Config::new("", None).unwrap();
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();
assert_eq_rooted(&path, "sub/sub1/d.html");
Expand All @@ -394,7 +430,7 @@ mod tests {
"#;

let default_syntax = Syntax::default();
let config = Config::new(raw_config, None).unwrap();
let config = Config::new(raw_config, None, None).unwrap();
assert_eq!(config.default_syntax, "foo");

let foo = config.syntaxes.get("foo").unwrap();
Expand Down Expand Up @@ -426,7 +462,7 @@ mod tests {
"#;

let default_syntax = Syntax::default();
let config = Config::new(raw_config, None).unwrap();
let config = Config::new(raw_config, None, None).unwrap();
assert_eq!(config.default_syntax, "foo");

let foo = config.syntaxes.get("foo").unwrap();
Expand Down Expand Up @@ -463,7 +499,7 @@ mod tests {
default_syntax = "emoji"
"#;

let config = Config::new(raw_config, None).unwrap();
let config = Config::new(raw_config, None, None).unwrap();
assert_eq!(config.default_syntax, "emoji");

let foo = config.syntaxes.get("emoji").unwrap();
Expand All @@ -483,7 +519,7 @@ mod tests {
name = "too_short"
block_start = "<"
"#;
let config = Config::new(raw_config, None);
let config = Config::new(raw_config, None, None);
assert_eq!(
config.unwrap_err().msg,
r#"delimiters must be at least two characters long: "<""#,
Expand All @@ -494,7 +530,7 @@ mod tests {
name = "contains_ws"
block_start = " {{ "
"#;
let config = Config::new(raw_config, None);
let config = Config::new(raw_config, None, None);
assert_eq!(
config.unwrap_err().msg,
r#"delimiters may not contain white spaces: " {{ ""#,
Expand All @@ -507,7 +543,7 @@ mod tests {
expr_start = "{{$"
comment_start = "{{#"
"#;
let config = Config::new(raw_config, None);
let config = Config::new(raw_config, None, None);
assert_eq!(
config.unwrap_err().msg,
r#"a delimiter may not be the prefix of another delimiter: "{{" vs "{{$""#,
Expand All @@ -522,7 +558,7 @@ mod tests {
syntax = [{ name = "default" }]
"#;

let _config = Config::new(raw_config, None).unwrap();
let _config = Config::new(raw_config, None, None).unwrap();
}

#[cfg(feature = "toml")]
Expand All @@ -534,7 +570,7 @@ mod tests {
{ name = "foo", block_start = "%%" } ]
"#;

let _config = Config::new(raw_config, None).unwrap();
let _config = Config::new(raw_config, None, None).unwrap();
}

#[cfg(feature = "toml")]
Expand All @@ -546,7 +582,7 @@ mod tests {
default_syntax = "foo"
"#;

let _config = Config::new(raw_config, None).unwrap();
let _config = Config::new(raw_config, None, None).unwrap();
}

#[cfg(feature = "config")]
Expand All @@ -559,6 +595,7 @@ mod tests {
extensions = ["js"]
"#,
None,
None,
)
.unwrap();
assert_eq!(
Expand Down Expand Up @@ -587,11 +624,12 @@ mod tests {
whitespace = "suppress"
"#,
None,
None,
)
.unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Suppress);

let config = Config::new(r#""#, None).unwrap();
let config = Config::new(r#""#, None, None).unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Preserve);

let config = Config::new(
Expand All @@ -600,6 +638,7 @@ mod tests {
whitespace = "preserve"
"#,
None,
None,
)
.unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Preserve);
Expand All @@ -610,6 +649,7 @@ mod tests {
whitespace = "minimize"
"#,
None,
None,
)
.unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Minimize);
Expand All @@ -627,17 +667,18 @@ mod tests {
whitespace = "suppress"
"#,
Some(&"minimize".to_owned()),
None,
)
.unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Minimize);

let config = Config::new(r#""#, Some(&"minimize".to_owned())).unwrap();
let config = Config::new(r#""#, Some(&"minimize".to_owned()), None).unwrap();
assert_eq!(config.whitespace, WhitespaceHandling::Minimize);
}

#[test]
fn test_config_whitespace_error() {
let config = Config::new(r#""#, Some("trim"));
let config = Config::new(r#""#, None, Some("trim"));
if let Err(err) = config {
assert_eq!(err.msg, "invalid value for `whitespace`: \"trim\"");
} else {
Expand Down
Loading