diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 96c09f9..1f6ec43 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -8,7 +8,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, windows-latest, macos-latest] + os: [ubuntu-22.04, windows-latest, macos-latest] python-version: ["3.7.17", "3.8", "3.9", "3.10", "3.11", "3.12", "3.13"] exclude: - os: macos-latest diff --git a/CHANGES.md b/CHANGES.md index d56f9f2..84164fd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -5,6 +5,7 @@ **Fixes** - Fixed `{% case %}` / `{% when %}` behavior. When using [`liquid.future.Environment`](https://jg-rp.github.io/liquid/api/future-environment), we now render any number of `{% else %}` blocks and allow `{% when %}` tags to appear after `{% else %}` tags. The default `Environment` continues to raise a `LiquidSyntaxError` in such cases. +- Fixed line numbers in some error messages. When parsing some Liquid expressions, we were always getting a line number of `1` in the event of a syntax error. See [issue #162](https://github.com/jg-rp/liquid/issues/162). **Changed** diff --git a/liquid/builtin/statement.py b/liquid/builtin/statement.py index 3043492..a70bf43 100644 --- a/liquid/builtin/statement.py +++ b/liquid/builtin/statement.py @@ -60,4 +60,6 @@ class Statement(Tag): def parse(self, stream: TokenStream) -> StatementNode: tok = stream.current expect(stream, TOKEN_STATEMENT) - return self.node_class(tok, self.env.parse_filtered_expression_value(tok.value)) + return self.node_class( + tok, self.env.parse_filtered_expression_value(tok.value, tok.linenum) + ) diff --git a/liquid/builtin/tags/assign_tag.py b/liquid/builtin/tags/assign_tag.py index 874c0ff..96cd536 100644 --- a/liquid/builtin/tags/assign_tag.py +++ b/liquid/builtin/tags/assign_tag.py @@ -65,8 +65,8 @@ class AssignTag(Tag): block = False node_class = AssignNode - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_filtered_expression_value(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_filtered_expression_value(value, linenum) def parse(self, stream: TokenStream) -> AssignNode: expect(stream, TOKEN_TAG, value=TAG_ASSIGN) @@ -83,5 +83,9 @@ def parse(self, stream: TokenStream) -> AssignNode: ) return self.node_class( - tok, AssignmentExpression(name, self._parse_expression(right)) + tok, + AssignmentExpression( + name, + self._parse_expression(right, stream.current.linenum), + ), ) diff --git a/liquid/builtin/tags/decrement_tag.py b/liquid/builtin/tags/decrement_tag.py index 7874f97..4981cb3 100644 --- a/liquid/builtin/tags/decrement_tag.py +++ b/liquid/builtin/tags/decrement_tag.py @@ -63,7 +63,9 @@ def parse(self, stream: TokenStream) -> DecrementNode: tok=tok, identifier=str( parse_unchained_identifier( - ExprTokenStream(tokenize(stream.current.value)) + ExprTokenStream( + tokenize(stream.current.value, stream.current.linenum) + ) ) ), ) diff --git a/liquid/builtin/tags/echo_tag.py b/liquid/builtin/tags/echo_tag.py index 1187386..467b726 100644 --- a/liquid/builtin/tags/echo_tag.py +++ b/liquid/builtin/tags/echo_tag.py @@ -30,8 +30,8 @@ class EchoTag(Tag): block = False node_class = EchoNode - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_filtered_expression_value(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_filtered_expression_value(value, linenum) def parse(self, stream: TokenStream) -> Node: # noqa: D102 expect(stream, TOKEN_TAG, value=TAG_ECHO) @@ -42,5 +42,5 @@ def parse(self, stream: TokenStream) -> Node: # noqa: D102 expr: Expression = NIL else: expect(stream, TOKEN_EXPRESSION) - expr = self._parse_expression(stream.current.value) + expr = self._parse_expression(stream.current.value, tok.linenum) return self.node_class(tok, expression=expr) diff --git a/liquid/builtin/tags/for_tag.py b/liquid/builtin/tags/for_tag.py index 38066b6..307fb75 100644 --- a/liquid/builtin/tags/for_tag.py +++ b/liquid/builtin/tags/for_tag.py @@ -334,7 +334,10 @@ def parse(self, stream: TokenStream) -> Node: stream.next_token() expect(stream, TOKEN_EXPRESSION) - expr = self.env.parse_loop_expression_value(stream.current.value) + expr = self.env.parse_loop_expression_value( + stream.current.value, + stream.current.linenum, + ) stream.next_token() block = parser.parse_block(stream, ENDFORBLOCK) diff --git a/liquid/builtin/tags/if_tag.py b/liquid/builtin/tags/if_tag.py index e927496..77eee8c 100644 --- a/liquid/builtin/tags/if_tag.py +++ b/liquid/builtin/tags/if_tag.py @@ -185,7 +185,10 @@ def __init__(self, env: Environment): def parse_expression(self, stream: TokenStream) -> Expression: """Pare a boolean expression from a stream of tokens.""" expect(stream, TOKEN_EXPRESSION) - return self.env.parse_boolean_expression_value(stream.current.value) + return self.env.parse_boolean_expression_value( + stream.current.value, + stream.current.linenum, + ) def parse(self, stream: TokenStream) -> Node: expect(stream, TOKEN_TAG, value=TAG_IF) diff --git a/liquid/builtin/tags/increment_tag.py b/liquid/builtin/tags/increment_tag.py index 66b3339..3e2f6e4 100644 --- a/liquid/builtin/tags/increment_tag.py +++ b/liquid/builtin/tags/increment_tag.py @@ -59,7 +59,9 @@ def parse(self, stream: TokenStream) -> IncrementNode: tok=tok, identifier=str( parse_unchained_identifier( - ExprTokenStream(tokenize(stream.current.value)) + ExprTokenStream( + tokenize(stream.current.value, stream.current.linenum) + ) ) ), ) diff --git a/liquid/builtin/tags/render_tag.py b/liquid/builtin/tags/render_tag.py index de58e9e..491f76f 100644 --- a/liquid/builtin/tags/render_tag.py +++ b/liquid/builtin/tags/render_tag.py @@ -1,4 +1,5 @@ """Parse tree node and tag definition for the built in "render" tag.""" + import sys from typing import Dict from typing import List @@ -249,7 +250,9 @@ class RenderTag(Tag): def parse(self, stream: TokenStream) -> Node: tok = next(stream) expect(stream, TOKEN_EXPRESSION) - expr_stream = ExprTokenStream(tokenize(stream.current.value)) + expr_stream = ExprTokenStream( + tokenize(stream.current.value, stream.current.linenum) + ) # Need a string. 'render' does not accept identifiers that resolve to a string. # This is the name of the template to be included. diff --git a/liquid/builtin/tags/tablerow_tag.py b/liquid/builtin/tags/tablerow_tag.py index 228dbb6..615934d 100644 --- a/liquid/builtin/tags/tablerow_tag.py +++ b/liquid/builtin/tags/tablerow_tag.py @@ -302,7 +302,9 @@ def parse(self, stream: TokenStream) -> TablerowNode: stream.next_token() expect(stream, TOKEN_EXPRESSION) - loop_expression = self.env.parse_loop_expression_value(stream.current.value) + loop_expression = self.env.parse_loop_expression_value( + stream.current.value, stream.current.linenum + ) stream.next_token() block = parser.parse_block(stream, END_TAGBLOCK) diff --git a/liquid/builtin/tags/unless_tag.py b/liquid/builtin/tags/unless_tag.py index 5a0ac8c..ee71dc1 100644 --- a/liquid/builtin/tags/unless_tag.py +++ b/liquid/builtin/tags/unless_tag.py @@ -184,7 +184,9 @@ def __init__(self, env: Environment): def parse_expression(self, stream: TokenStream) -> Expression: """Parse a boolean expression from a stream of tokens.""" expect(stream, TOKEN_EXPRESSION) - return self.env.parse_boolean_expression_value(stream.current.value) + return self.env.parse_boolean_expression_value( + stream.current.value, stream.current.linenum + ) def parse(self, stream: TokenStream) -> Union[UnlessNode, IllegalNode]: expect(stream, TOKEN_TAG, value=TAG_UNLESS) diff --git a/liquid/environment.py b/liquid/environment.py index 113c125..e90a86d 100644 --- a/liquid/environment.py +++ b/liquid/environment.py @@ -529,7 +529,8 @@ async def analyze_tags_async( ) def make_globals( - self, globals: Optional[Mapping[str, object]] = None # noqa: A002 + self, + globals: Optional[Mapping[str, object]] = None, # noqa: A002 ) -> Dict[str, object]: """Combine environment globals with template globals.""" if globals: @@ -577,12 +578,12 @@ def set_expression_cache_size(self, maxsize: int = 0) -> None: def _get_expression_parsers( self, cache_size: int = 0 ) -> Tuple[ - Callable[[str], "BooleanExpression"], - Callable[[str], "BooleanExpression"], - Callable[[str], "FilteredExpression"], - Callable[[str], "FilteredExpression"], - Callable[[str], "FilteredExpression"], - Callable[[str], "LoopExpression"], + Callable[[str, int], "BooleanExpression"], + Callable[[str, int], "BooleanExpression"], + Callable[[str, int], "FilteredExpression"], + Callable[[str, int], "FilteredExpression"], + Callable[[str, int], "FilteredExpression"], + Callable[[str, int], "LoopExpression"], ]: if cache_size >= 1: return ( diff --git a/liquid/extra/tags/if_expressions.py b/liquid/extra/tags/if_expressions.py index 650d633..e6b8318 100644 --- a/liquid/extra/tags/if_expressions.py +++ b/liquid/extra/tags/if_expressions.py @@ -26,7 +26,7 @@ def parse(self, stream: TokenStream) -> StatementNode: tok = stream.current expect(stream, TOKEN_STATEMENT) return StatementNode( - tok, self.env.parse_conditional_expression_value(tok.value) + tok, self.env.parse_conditional_expression_value(tok.value, tok.linenum) ) @@ -35,8 +35,8 @@ class InlineIfAssignTag(AssignTag): inline `if` expressions. """ - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_conditional_expression_value(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_conditional_expression_value(value, linenum) class InlineIfEchoTag(EchoTag): @@ -44,8 +44,8 @@ class InlineIfEchoTag(EchoTag): inline `if` expressions. """ - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_conditional_expression_value(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_conditional_expression_value(value, linenum) class InlineIfStatementWithParens(Statement): @@ -58,7 +58,10 @@ def parse(self, stream: TokenStream) -> StatementNode: tok = stream.current expect(stream, TOKEN_STATEMENT) return StatementNode( - tok, self.env.parse_conditional_expression_value_with_parens(tok.value) + tok, + self.env.parse_conditional_expression_value_with_parens( + tok.value, tok.linenum + ), ) @@ -68,8 +71,8 @@ class InlineIfAssignTagWithParens(AssignTag): terms with parentheses. """ - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_conditional_expression_value_with_parens(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_conditional_expression_value_with_parens(value, linenum) class InlineIfEchoTagWithParens(EchoTag): @@ -78,5 +81,5 @@ class InlineIfEchoTagWithParens(EchoTag): terms with parentheses. """ - def _parse_expression(self, value: str) -> Expression: - return self.env.parse_conditional_expression_value_with_parens(value) + def _parse_expression(self, value: str, linenum: int) -> Expression: + return self.env.parse_conditional_expression_value_with_parens(value, linenum) diff --git a/liquid/extra/tags/if_not.py b/liquid/extra/tags/if_not.py index 821db7c..2a31eb6 100644 --- a/liquid/extra/tags/if_not.py +++ b/liquid/extra/tags/if_not.py @@ -20,4 +20,6 @@ class IfNotTag(IfTag): def parse_expression(self, stream: TokenStream) -> Expression: """Pare a boolean expression from a stream of tokens.""" expect(stream, TOKEN_EXPRESSION) - return parse_boolean_expression_with_parens(stream.current.value) + return parse_boolean_expression_with_parens( + stream.current.value, stream.current.linenum + ) diff --git a/tests/test_malformed.py b/tests/test_malformed.py index 28f3266..068ee97 100644 --- a/tests/test_malformed.py +++ b/tests/test_malformed.py @@ -1,4 +1,5 @@ """Malformed template test cases.""" + from typing import Dict from typing import Iterable from typing import List @@ -16,6 +17,7 @@ from liquid.exceptions import FilterArgumentError from liquid.exceptions import LiquidSyntaxError from liquid.exceptions import LiquidTypeError +from liquid.exceptions import LiquidWarning from liquid.exceptions import TemplateNotFound from liquid.exceptions import lookup_warning from liquid.extra import IfNotTag @@ -33,20 +35,20 @@ class Case(NamedTuple): expect_render: str = "" @property - def exceptions(self): + def exceptions(self) -> Tuple[Type[Error], ...]: if isinstance(self.expect_exception, tuple): return self.expect_exception return (self.expect_exception,) @property - def warnings(self): + def warnings(self) -> Tuple[Type[LiquidWarning], ...]: return tuple(lookup_warning(e) for e in self.exceptions) class MalformedTemplateTestCase(TestCase): """Malformed template test case.""" - def setUp(self): + def setUp(self) -> None: self.global_context = { "product": { "some-tags": ["hello", "there"], @@ -55,7 +57,7 @@ def setUp(self): "tag": "goodbye", } - def _test(self, test_cases: Iterable[Case], mode: Mode = Mode.STRICT): + def _test(self, test_cases: Iterable[Case], mode: Mode = Mode.STRICT) -> None: """Helper method for running lists of `Case`s in each render mode.""" self._test_with_env(Environment(tolerance=mode), test_cases) @@ -75,7 +77,7 @@ def _test(self, test_cases: Iterable[Case], mode: Mode = Mode.STRICT): env, [case for case in test_cases if "||" not in case.template] ) - def _test_with_env(self, env: Environment, test_cases: Iterable[Case]): + def _test_with_env(self, env: Environment, test_cases: Iterable[Case]) -> None: for case in test_cases: with self.subTest(msg=case.description, mode=env.mode): if env.mode == Mode.STRICT: @@ -103,7 +105,9 @@ def _test_with_env(self, env: Environment, test_cases: Iterable[Case]): result = template.render() self.assertEqual(result, case.expect_render) - def _test_partial(self, test_cases: Iterable[Case], templates: Dict[str, str]): + def _test_partial( + self, test_cases: Iterable[Case], templates: Dict[str, str] + ) -> None: """Helper method for testing lists of 'include' or 'render' cases.""" env = Environment(loader=DictLoader(templates)) for case in test_cases: @@ -132,7 +136,7 @@ def _test_partial(self, test_cases: Iterable[Case], templates: Dict[str, str]): result = template.render() self.assertEqual(result, case.expect_render) - def test_liquid_syntax(self): + def test_liquid_syntax(self) -> None: """Test that we fail early and loud when parsing a malformed template.""" test_cases = [ Case( @@ -389,14 +393,14 @@ def test_liquid_syntax(self): self._test(test_cases, mode=Mode.WARN) self._test(test_cases, mode=Mode.LAX) - def test_liquid_syntax_from_template_api(self): + def test_liquid_syntax_from_template_api(self) -> None: """Test that syntax errors are raised by default when using the `Template` API.""" with self.assertRaises(LiquidSyntaxError): # Missing colon before filter argument Template(r"{{ a | sort foo }}") - def test_unrecoverable_syntax_errors(self): + def test_unrecoverable_syntax_errors(self) -> None: """Test that we fail early and loud when parsing a malformed template.""" test_cases = [ Case( @@ -415,7 +419,88 @@ def test_unrecoverable_syntax_errors(self): self._test(test_cases, mode=Mode.STRICT) - def test_bad_include(self): + def test_error_line_numbers(self) -> None: + """Test that we get the correct line number in error messages.""" + test_cases = [ + Case( + description="issue #162", + template="Hello, \nMy name is {{{ name }}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="echo", + template="Hello, \nMy name is {% echo { name %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="assign", + template="Hello, \n\n {% assign x = { name %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + Case( + description="decrement", + template="Hello, \n\n {% decrement { x %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + Case( + description="increment", + template="Hello, \n\n {% increment { x %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + Case( + description="for", + template="Hello, \n {% for x \nin { y %}{% endfor %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + Case( + description="if", + template="Hello, \n {% if x == { y %}{% endif %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="elsif", + template="Hello, \n {% if x == y %}\n{% elsif z < { %}{% endif %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + Case( + description="render", + template="Hello, \n {% render 'foo' x: { %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="tablerow", + template="Hello, \n {% tablerow x in { y %}{% endtablerow %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="unless", + template="Hello, \n {% unless x == { y %}{% endunless %}", + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 2", + ), + Case( + description="unless elsif", + template=( + "Hello, \n {% unless x == y %}\n{% elsif z < { %}{% endunless %}" + ), + expect_exception=LiquidSyntaxError, + expect_msg="unexpected '{', on line 3", + ), + ] + + self._test(test_cases, mode=Mode.STRICT) + + def test_bad_include(self) -> None: """Test that we gracefully handle include errors.""" test_cases = [ Case( @@ -485,7 +570,7 @@ def test_bad_include(self): self._test_partial(test_cases, templates) - def test_bad_render(self): + def test_bad_render(self) -> None: """Test that we gracefully handle render errors.""" test_cases = [ Case( @@ -546,7 +631,7 @@ def test_bad_render(self): self._test_partial(test_cases, templates) - def test_resume_block(self): + def test_resume_block(self) -> None: """Test that we continue to execute a block after a single statement error.""" source = ( r"{% if true %}" @@ -581,7 +666,7 @@ def test_resume_block(self): self.assertEqual(result, "before error after error") - def test_invalid_identifiers(self): + def test_invalid_identifiers(self) -> None: """Test that we gracefully handle invalid identifiers.""" test_cases = [ Case(