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 1 commit
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
16 changes: 15 additions & 1 deletion rinja_derive/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,21 @@ impl TemplateInput<'_> {
let mut dependency_graph = Vec::new();
let mut check = vec![(Arc::clone(&self.path), source, source_path)];
while let Some((path, source, source_path)) = check.pop() {
let parsed = self.syntax.parse(source, source_path)?;
let parsed = match self.syntax.parse(Arc::clone(&source), source_path) {
Ok(parsed) => parsed,
Err(err) => {
let msg = err
.message
.unwrap_or_else(|| "failed to parse template source".into());
let file_path = err
.file_path
.as_deref()
.unwrap_or(Path::new("<source attribute>"));
Kijewski marked this conversation as resolved.
Show resolved Hide resolved
let file_info =
FileInfo::new(file_path, Some(&source), Some(&source[err.offset..]));
return Err(CompileError::new(msg, Some(file_info)));
}
};

let mut top = true;
let mut nested = vec![parsed.nodes()];
Expand Down
99 changes: 77 additions & 22 deletions rinja_derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@ use std::collections::HashMap;
use std::fmt;
use std::path::Path;

use annotate_snippets::{Level, Renderer, Snippet};
use config::{read_config_file, Config};
use generator::{Generator, MapChain};
use heritage::{Context, Heritage};
use input::{Print, TemplateArgs, TemplateInput};
use parser::{generate_error_info, strip_common, ErrorInfo, ParseError, Parsed, WithSpan};
use parser::{strip_common, Parsed, WithSpan};
#[cfg(not(feature = "__standalone"))]
use proc_macro::TokenStream as TokenStream12;
#[cfg(feature = "__standalone")]
Expand Down Expand Up @@ -188,14 +189,46 @@ struct CompileError {

impl CompileError {
fn new<S: fmt::Display>(msg: S, file_info: Option<FileInfo<'_>>) -> Self {
let span = Span::call_site();

if let Some(FileInfo {
path,
source: Some(source),
node_source: Some(node_source),
}) = file_info
{
if source
.as_bytes()
.as_ptr_range()
.contains(&node_source.as_ptr())
{
let label = msg.to_string();
let path = match std::env::current_dir() {
Ok(cwd) => strip_common(&cwd, path),
Err(_) => path.display().to_string(),
};

let start = node_source.as_ptr() as usize - source.as_ptr() as usize;
let annotation = Level::Error.span(start..start).label("close to this token");
let snippet = Snippet::source(source)
.origin(&path)
.fold(true)
.annotation(annotation);
let message = Level::Error.title(&label).snippet(snippet);

let mut msg = Renderer::styled().render(message).to_string();
if let Some((prefix, _)) = msg.split_once(' ') {
msg.replace_range(..=prefix.len(), "");
}
return Self { msg, span };
}
}

let msg = match file_info {
Some(file_info) => format!("{msg}{file_info}"),
None => msg.to_string(),
};
Self {
msg,
span: Span::call_site(),
}
Self { msg, span }
}

fn no_file_info<S: fmt::Display>(msg: S) -> Self {
Expand All @@ -219,14 +252,7 @@ impl fmt::Display for CompileError {
}
}

impl From<ParseError> for CompileError {
#[inline]
fn from(e: ParseError) -> Self {
// It already has the correct message so no need to do anything.
Self::no_file_info(e)
}
}

#[derive(Debug, Clone, Copy)]
struct FileInfo<'a> {
path: &'a Path,
source: Option<&'a str>,
Expand Down Expand Up @@ -255,18 +281,13 @@ impl fmt::Display for FileInfo<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match (self.source, self.node_source) {
(Some(source), Some(node_source)) => {
let (
ErrorInfo {
row,
column,
source_after,
},
file_path,
) = generate_error_info(source, node_source, self.path);
let (error_info, file_path) = generate_error_info(source, node_source, self.path);
write!(
f,
"\n --> {file_path}:{row}:{column}\n{source_after}",
row = row + 1
row = error_info.row,
column = error_info.column,
source_after = &error_info.source_after,
)
}
_ => {
Expand All @@ -280,6 +301,40 @@ impl fmt::Display for FileInfo<'_> {
}
}

struct ErrorInfo {
row: usize,
column: usize,
source_after: String,
}

fn generate_row_and_column(src: &str, input: &str) -> ErrorInfo {
let offset = src.len() - input.len();
let (source_before, source_after) = src.split_at(offset);

let source_after = match source_after.char_indices().enumerate().take(41).last() {
Some((80, (i, _))) => format!("{:?}...", &source_after[..i]),
_ => format!("{source_after:?}"),
};

let (row, last_line) = source_before.lines().enumerate().last().unwrap_or_default();
let column = last_line.chars().count();
ErrorInfo {
row,
column,
source_after,
}
}

/// Return the error related information and its display file path.
fn generate_error_info(src: &str, input: &str, file_path: &Path) -> (ErrorInfo, String) {
let file_path = match std::env::current_dir() {
Ok(cwd) => strip_common(&cwd, file_path),
Err(_) => file_path.display().to_string(),
};
let error_info = generate_row_and_column(src, input);
(error_info, file_path)
}

struct MsgValidEscapers<'a>(&'a [(Vec<Cow<'a, str>>, Cow<'a, str>)]);

impl fmt::Display for MsgValidEscapers<'_> {
Expand Down
1 change: 1 addition & 0 deletions rinja_derive_standalone/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,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
99 changes: 18 additions & 81 deletions rinja_parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::{fmt, str};
use nom::branch::alt;
use nom::bytes::complete::{escaped, is_not, tag, take_till, take_while_m_n};
use nom::character::complete::{anychar, char, one_of, satisfy};
use nom::combinator::{cut, eof, map, not, opt, recognize};
use nom::combinator::{complete, cut, eof, map, not, opt, recognize};
use nom::error::{Error, ErrorKind, FromExternalError};
use nom::multi::{many0_count, many1};
use nom::sequence::{delimited, pair, preceded, terminated, tuple};
Expand Down Expand Up @@ -110,31 +110,18 @@ impl<'a> Ast<'a> {
syntax: &Syntax<'_>,
) -> Result<Self, ParseError> {
let parse = |i: &'a str| Node::many(i, &State::new(syntax));
let (input, message) = match terminated(parse, cut(eof))(src) {
let (input, message) = match complete(terminated(parse, cut(eof)))(src) {
Ok(("", nodes)) => return Ok(Self { nodes }),
Ok(_) => unreachable!("eof() is not eof?"),
Err(nom::Err::Incomplete(_)) => unreachable!("complete() is not complete?"),
Err(
nom::Err::Error(ErrorContext { input, message, .. })
| nom::Err::Failure(ErrorContext { input, message, .. }),
) => (input, message),
Err(nom::Err::Incomplete(_)) => return Err(ParseError::Incomplete),
};

let offset = src.len() - input.len();
let (source_before, source_after) = src.split_at(offset);

let source_after = match source_after.char_indices().enumerate().take(41).last() {
Some((40, (i, _))) => format!("{:?}...", &source_after[..i]),
_ => format!("{source_after:?}"),
};

let (row, last_line) = source_before.lines().enumerate().last().unwrap_or_default();
let column = last_line.chars().count();
Err(ParseError::Details {
Err(ParseError {
message,
row,
column,
source_after,
offset: src.len() - input.len(),
file_path,
})
}
Expand Down Expand Up @@ -198,89 +185,39 @@ impl<'a, T: PartialEq> PartialEq for WithSpan<'a, T> {
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum ParseError {
Incomplete,
Details {
message: Option<Cow<'static, str>>,
row: usize,
column: usize,
source_after: String,
file_path: Option<Arc<Path>>,
},
pub struct ParseError {
pub message: Option<Cow<'static, str>>,
pub offset: usize,
pub file_path: Option<Arc<Path>>,
}

impl std::error::Error for ParseError {}

impl fmt::Display for ParseError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let (message, mut row, column, source, path) = match self {
ParseError::Incomplete => return write!(f, "parsing incomplete"),
ParseError::Details {
message,
row,
column,
source_after,
file_path,
} => (message, *row, column, source_after, file_path),
};
let ParseError {
message,
file_path,
offset,
} = self;

if let Some(message) = message {
writeln!(f, "{}", message)?;
}

let path = path
let path = file_path
.as_ref()
.and_then(|path| Some(strip_common(&current_dir().ok()?, path)));

row += 1;
match path {
Some(path) => f.write_fmt(format_args!(
"failed to parse template source\n --> {path}:{row}:{column}\n{source}",
)),
None => f.write_fmt(format_args!(
"failed to parse template source at row {row}, column {column} near:\n{source}",
)),
Some(path) => write!(f, "failed to parse template source\n --> {path}@{offset}"),
None => write!(f, "failed to parse template source near offset {offset}"),
}
}
}

pub(crate) type ParseErr<'a> = nom::Err<ErrorContext<'a>>;
pub(crate) type ParseResult<'a, T = &'a str> = Result<(&'a str, T), ParseErr<'a>>;

pub struct ErrorInfo {
pub row: usize,
pub column: usize,
pub source_after: String,
}

pub fn generate_row_and_column(src: &str, input: &str) -> ErrorInfo {
let offset = src.len() - input.len();
let (source_before, source_after) = src.split_at(offset);

let source_after = match source_after.char_indices().enumerate().take(41).last() {
Some((40, (i, _))) => format!("{:?}...", &source_after[..i]),
_ => format!("{source_after:?}"),
};

let (row, last_line) = source_before.lines().enumerate().last().unwrap_or_default();
let column = last_line.chars().count();
ErrorInfo {
row,
column,
source_after,
}
}

/// Return the error related information and its display file path.
pub fn generate_error_info(src: &str, input: &str, file_path: &Path) -> (ErrorInfo, String) {
let file_path = match std::env::current_dir() {
Ok(cwd) => strip_common(&cwd, file_path),
Err(_) => file_path.display().to_string(),
};
let error_info = generate_row_and_column(src, input);
(error_info, file_path)
}

/// This type is used to handle `nom` errors and in particular to add custom error messages.
/// It used to generate `ParserError`.
///
Expand Down Expand Up @@ -801,7 +738,7 @@ pub fn strip_common(base: &Path, path: &Path) -> String {
if path_parts.is_empty() {
path.display().to_string()
} else {
path_parts.join("/")
path_parts.join(std::path::MAIN_SEPARATOR_STR)
}
}

Expand Down
4 changes: 2 additions & 2 deletions rinja_parser/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ fn test_rust_macro() {
&*Ast::from_str("{{a.b.c!( hello )}}", None, &syntax)
.unwrap_err()
.to_string(),
"failed to parse template source at row 1, column 7 near:\n\"!( hello )}}\"",
"failed to parse template source near offset 7",
);
}

Expand Down Expand Up @@ -908,7 +908,7 @@ fn test_missing_space_after_kw() {
let err = Ast::from_str("{%leta=b%}", None, &syntax).unwrap_err();
assert!(matches!(
&*err.to_string(),
"failed to parse template source at row 1, column 0 near:\n\"{%leta=b%}\"",
"failed to parse template source near offset 0",
));
}

Expand Down
Loading