Fixed incorrect lexer position reports (#630)

CHANGELOG.md

@@ -22,6 +22,8 @@ All notable changes to MiniJinja are documented here.
   (`1 is sameas 1`).  #626
 - Added error context for strict undefined errors during template
   rendering.  #627
+- Syntax errors caused by the lexer now include the correct position of
+  the error.  #630
 
 ## 2.4.0
 

minijinja/src/compiler/lexer.rs

@@ -18,6 +18,7 @@ pub struct WhitespaceConfig {
 pub struct Tokenizer<'s> {
     stack: Vec<LexerState>,
     source: &'s str,
+    filename: &'s str,
     current_line: u32,
     current_col: u32,
     current_offset: usize,
@@ -287,6 +288,7 @@ impl<'s> Tokenizer<'s> {
     /// Creates a new tokenizer.
     pub fn new(
         input: &'s str,
+        filename: &'s str,
         in_expr: bool,
         syntax_config: SyntaxConfig,
         whitespace_config: WhitespaceConfig,
@@ -302,6 +304,7 @@ impl<'s> Tokenizer<'s> {
         }
         Tokenizer {
             source,
+            filename,
             stack: vec![if in_expr {
                 LexerState::Variable
             } else {
@@ -319,6 +322,11 @@ impl<'s> Tokenizer<'s> {
         }
     }
 
+    /// Returns the current filename.
+    pub fn filename(&self) -> &str {
+        self.filename
+    }
+
     /// Produces the next token from the tokenizer.
     pub fn next_token(&mut self) -> Result<Option<(Token<'s>, Span)>, Error> {
         loop {
@@ -398,7 +406,14 @@ impl<'s> Tokenizer<'s> {
 
     #[inline]
     fn syntax_error(&mut self, msg: &'static str) -> Error {
-        Error::new(ErrorKind::SyntaxError, msg)
+        let mut span = self.span(self.loc());
+        if span.start_col == span.end_col {
+            span.end_col += 1;
+            span.end_offset += 1;
+        }
+        let mut err = Error::new(ErrorKind::SyntaxError, msg);
+        err.set_filename_and_span(self.filename, span);
+        err
     }
 
     fn eat_number(&mut self) -> Result<(Token<'s>, Span), Error> {
@@ -469,7 +484,7 @@ impl<'s> Tokenizer<'s> {
             } else {
                 u128::from_str_radix(&num, radix)
                     .map(Token::Int128)
-                    .map_err(|_| self.syntax_error("invalid integer"))
+                    .map_err(|_| self.syntax_error("invalid integer (too large)"))
             }),
             self.span(old_loc),
         ))
@@ -509,6 +524,7 @@ impl<'s> Tokenizer<'s> {
             })
             .count();
         if escaped || self.rest_bytes().get(str_len + 1) != Some(&delim) {
+            self.advance(str_len + 1);
             return Err(self.syntax_error("unexpected end of string"));
         }
         let s = self.advance(str_len + 2);
@@ -638,6 +654,7 @@ impl<'s> Tokenizer<'s> {
                     self.handle_tail_ws(ws);
                     Ok(ControlFlow::Continue(()))
                 } else {
+                    self.advance(self.rest_bytes().len());
                     Err(self.syntax_error("unexpected end of comment"))
                 }
             }
@@ -726,6 +743,7 @@ impl<'s> Tokenizer<'s> {
                 return Ok(ControlFlow::Break((Token::TemplateData(result), span)));
             }
         }
+        self.advance(self.rest_bytes().len());
         Err(self.syntax_error("unexpected end of raw block"))
     }
 
@@ -896,7 +914,8 @@ pub fn tokenize(
 ) -> impl Iterator<Item = Result<(Token<'_>, Span), Error>> {
     // This function is unused in minijinja itself, it's only used in tests and in the
     // unstable machinery as a convenient alternative to the tokenizer.
-    let mut tokenizer = Tokenizer::new(input, in_expr, syntax_config, whitespace_config);
+    let mut tokenizer =
+        Tokenizer::new(input, "<string>", in_expr, syntax_config, whitespace_config);
     std::iter::from_fn(move || tokenizer.next_token().transpose())
 }
 

minijinja/src/compiler/parser.rs

@@ -103,11 +103,13 @@ impl<'a> TokenStream<'a> {
     /// Tokenize a template
     pub fn new(
         source: &'a str,
+        filename: &'a str,
         in_expr: bool,
         syntax_config: SyntaxConfig,
         whitespace_config: WhitespaceConfig,
     ) -> TokenStream<'a> {
-        let mut tokenizer = Tokenizer::new(source, in_expr, syntax_config, whitespace_config);
+        let mut tokenizer =
+            Tokenizer::new(source, filename, in_expr, syntax_config, whitespace_config);
         let current = tokenizer.next_token().transpose();
         TokenStream {
             tokenizer,
@@ -229,21 +231,60 @@ macro_rules! with_recursion_guard {
 }
 
 impl<'a> Parser<'a> {
+    /// Creates a new parser.
+    ///
+    /// `in_expr` is necessary to parse within an expression context.  Otherwise
+    /// the parser starts out in template context.  This means that when
+    /// [`parse`](Self::parse) is to be called, the `in_expr` argument must be
+    /// `false` and for [`parse_standalone_expr`](Self::parse_standalone_expr)
+    /// it must be `true`.
     pub fn new(
         source: &'a str,
+        filename: &'a str,
         in_expr: bool,
         syntax_config: SyntaxConfig,
         whitespace_config: WhitespaceConfig,
     ) -> Parser<'a> {
         Parser {
-            stream: TokenStream::new(source, in_expr, syntax_config, whitespace_config),
+            stream: TokenStream::new(source, filename, in_expr, syntax_config, whitespace_config),
             in_macro: false,
             in_loop: false,
             blocks: BTreeSet::new(),
             depth: 0,
         }
     }
 
+    /// Parses a template.
+    pub fn parse(&mut self) -> Result<ast::Stmt<'a>, Error> {
+        let span = self.stream.last_span();
+        self.subparse(&|_| false)
+            .map(|children| {
+                ast::Stmt::Template(Spanned::new(
+                    ast::Template { children },
+                    self.stream.expand_span(span),
+                ))
+            })
+            .map_err(|err| self.attach_location_to_error(err))
+    }
+
+    /// Parses an expression and asserts that there is no more input after it.
+    pub fn parse_standalone_expr(&mut self) -> Result<ast::Expr<'a>, Error> {
+        self.parse_expr()
+            .and_then(|result| {
+                if ok!(self.stream.next()).is_some() {
+                    syntax_error!("unexpected input after expression")
+                } else {
+                    Ok(result)
+                }
+            })
+            .map_err(|err| self.attach_location_to_error(err))
+    }
+
+    /// Returns the current filename.
+    pub fn filename(&self) -> &str {
+        self.stream.tokenizer.filename()
+    }
+
     fn parse_ifexpr(&mut self) -> Result<ast::Expr<'a>, Error> {
         let mut span = self.stream.last_span();
         let mut expr = ok!(self.parse_or());
@@ -668,11 +709,11 @@ impl<'a> Parser<'a> {
         Ok(expr)
     }
 
-    pub fn parse_expr(&mut self) -> Result<ast::Expr<'a>, Error> {
+    fn parse_expr(&mut self) -> Result<ast::Expr<'a>, Error> {
         with_recursion_guard!(self, self.parse_ifexpr())
     }
 
-    pub fn parse_expr_noif(&mut self) -> Result<ast::Expr<'a>, Error> {
+    fn parse_expr_noif(&mut self) -> Result<ast::Expr<'a>, Error> {
         self.parse_or()
     }
 
@@ -1175,49 +1216,33 @@ impl<'a> Parser<'a> {
         Ok(rv)
     }
 
-    pub fn parse(&mut self) -> Result<ast::Stmt<'a>, Error> {
-        let span = self.stream.last_span();
-        Ok(ast::Stmt::Template(Spanned::new(
-            ast::Template {
-                children: ok!(self.subparse(&|_| false)),
-            },
-            self.stream.expand_span(span),
-        )))
+    #[inline]
+    fn attach_location_to_error(&mut self, mut err: Error) -> Error {
+        if err.line().is_none() {
+            err.set_filename_and_span(self.filename(), self.stream.last_span())
+        }
+        err
     }
 }
 
 /// Parses a template.
 pub fn parse<'source>(
     source: &'source str,
-    filename: &str,
+    filename: &'source str,
     syntax_config: SyntaxConfig,
     whitespace_config: WhitespaceConfig,
 ) -> Result<ast::Stmt<'source>, Error> {
-    let mut parser = Parser::new(source, false, syntax_config, whitespace_config);
-    parser.parse().map_err(|mut err| {
-        if err.line().is_none() {
-            err.set_filename_and_span(filename, parser.stream.last_span())
-        }
-        err
-    })
+    Parser::new(source, filename, false, syntax_config, whitespace_config).parse()
 }
 
-/// Parses an expression
+/// Parses a standalone expression.
 pub fn parse_expr(source: &str) -> Result<ast::Expr<'_>, Error> {
-    let mut parser = Parser::new(source, true, Default::default(), Default::default());
-    parser
-        .parse_expr()
-        .and_then(|result| {
-            if ok!(parser.stream.next()).is_some() {
-                syntax_error!("unexpected input after expression")
-            } else {
-                Ok(result)
-            }
-        })
-        .map_err(|mut err| {
-            if err.line().is_none() {
-                err.set_filename_and_span("<expression>", parser.stream.last_span())
-            }
-            err
-        })
+    Parser::new(
+        source,
+        "<expression>",
+        true,
+        Default::default(),
+        Default::default(),
+    )
+    .parse_standalone_expr()
 }

minijinja/tests/inputs/err_partial_float.txt

@@ -0,0 +1,3 @@
+{}
+---
+{{ 42e+ }}

minijinja/tests/inputs/err_raw_block_missing_end.txt

@@ -0,0 +1,3 @@
+{}
+---
+{% raw %}this raw block is missing an end tag

minijinja/tests/inputs/err_too_large_integer.txt

@@ -0,0 +1,3 @@
+{}
+---
+{{ 10234237582375823572357238572385723582375827582375238572385723857238572385723523758 }}

minijinja/tests/inputs/err_unexpected_character.txt

@@ -0,0 +1,3 @@
+{}
+---
+{{ foo @ }}

minijinja/tests/inputs/err_unterminated_string.txt

@@ -0,0 +1,3 @@
+{}
+---
+{{ "this string is missing a closing quote

minijinja/tests/snapshots/test_templates__vm@err_bad_trailing_underscore.txt.snap

@@ -16,7 +16,7 @@ Error {
 syntax error: '_' may not occur at end of number (in err_bad_trailing_underscore.txt:1)
 ----------------------- err_bad_trailing_underscore.txt -----------------------
    1 > {{ 10_ }}
-     i ^^ syntax error
+     i       ^ syntax error
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 No referenced variables
 -------------------------------------------------------------------------------

minijinja/tests/snapshots/test_templates__vm@err_partial_float.txt.snap

@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{{ 42e+ }}"
+info: {}
+input_file: minijinja/tests/inputs/err_partial_float.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "invalid float",
+    name: "err_partial_float.txt",
+    line: 1,
+}
+
+syntax error: invalid float (in err_partial_float.txt:1)
+---------------------------- err_partial_float.txt ----------------------------
+   1 > {{ 42e+ }}
+     i        ^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------

minijinja/tests/snapshots/test_templates__vm@err_raw_block_missing_end.txt.snap

@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{% raw %}this raw block is missing an end tag"
+info: {}
+input_file: minijinja/tests/inputs/err_raw_block_missing_end.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "unexpected end of raw block",
+    name: "err_raw_block_missing_end.txt",
+    line: 1,
+}
+
+syntax error: unexpected end of raw block (in err_raw_block_missing_end.txt:1)
+------------------------ err_raw_block_missing_end.txt ------------------------
+   1 > {% raw %}this raw block is missing an end tag
+     i                                              ^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------

minijinja/tests/snapshots/test_templates__vm@err_too_large_integer.txt.snap

@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{{ 10234237582375823572357238572385723582375827582375238572385723857238572385723523758 }}"
+info: {}
+input_file: minijinja/tests/inputs/err_too_large_integer.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "invalid integer (too large)",
+    name: "err_too_large_integer.txt",
+    line: 1,
+}
+
+syntax error: invalid integer (too large) (in err_too_large_integer.txt:1)
+-------------------------- err_too_large_integer.txt --------------------------
+   1 > {{ 10234237582375823572357238572385723582375827582375238572385723857238572385723523758 }}
+     i                                                                                       ^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------

minijinja/tests/snapshots/test_templates__vm@err_unexpected_character.txt.snap

@@ -0,0 +1,22 @@
+---
+source: minijinja/tests/test_templates.rs
+description: "{{ foo @ }}"
+info: {}
+input_file: minijinja/tests/inputs/err_unexpected_character.txt
+---
+!!!SYNTAX ERROR!!!
+
+Error {
+    kind: SyntaxError,
+    detail: "unexpected character",
+    name: "err_unexpected_character.txt",
+    line: 1,
+}
+
+syntax error: unexpected character (in err_unexpected_character.txt:1)
+------------------------ err_unexpected_character.txt -------------------------
+   1 > {{ foo @ }}
+     i        ^ syntax error
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+No referenced variables
+-------------------------------------------------------------------------------