Optional container support

Fix #5.

jinjalint/ast.py

@@ -170,6 +170,29 @@ def __str__(self):
         )
 
 
+@attr.s(frozen=True)
+class JinjaOptionalContainer(Jinja):
+    first_opening_if = attr.ib()  # JinjaTag
+    opening_tag = attr.ib()  # OpeningTag
+    first_closing_if = attr.ib()  # JinjaTag
+    content = attr.ib()  # Interpolated
+    second_opening_if = attr.ib()  # JinjaTag
+    closing_tag = attr.ib()  # ClosingTag
+    second_closing_if = attr.ib()  # JinjaTag
+
+    def __str__(self):
+        nodes = [
+            self.first_opening_if,
+            self.opening_tag,
+            self.first_closing_if,
+            self.content,
+            self.second_opening_if,
+            self.closing_tag,
+            self.second_closing_if,
+        ]
+        return ''.join(str(n) for n in nodes)
+
+
 @attr.s(frozen=True)
 class InterpolatedBase(Node):
     nodes = attr.ib()  # [any | Jinja]
@@ -225,10 +248,3 @@ def __init__(self, *args, **kwargs):
         kwargs = kwargs.copy()
         kwargs['nodes'] = _normalize_nodes(kwargs['nodes'])
         super().__init__(**kwargs)
-
-
-NODE_TYPE_LIST = [
-    Node, Slash, OpeningTag, ClosingTag, Element, String, Integer,
-    Attribute, Comment, JinjaVariable, JinjaComment, JinjaTag,
-    JinjaElement, JinjaElementPart, InterpolatedBase, Interpolated,
-]

jinjalint/check.py

@@ -25,6 +25,10 @@ def contains_exclusively(string, char):
     return string.replace(char, '') == ''
 
 
+def truncate(s, length=16):
+    return s[:length] + (s[length:] and '…')
+
+
 def check_indentation(file, config):
     indent_size = config.get('indent_size', 4)
 
@@ -36,8 +40,9 @@ def add_issue(location, msg):
     def check_indent(expected_level, node, inline=False):
         node_level = get_indent_level(file.source, node)
         if node_level is None:
-            if not inline and not isinstance(node, ast.Jinja):
-                add_issue(node.begin, 'Should be on the next line')
+            if not inline:  # and not isinstance(node, ast.Jinja):
+                node_s = repr(truncate(str(node)))
+                add_issue(node.begin, node_s + ' should be on the next line')
             return
 
         if node_level != expected_level:
@@ -118,6 +123,40 @@ def check_jinja_element_part(expected_level, part, inline=False):
         if part.content is not None:
             check_content(content_level, part.content, inline=inline)
 
+    def check_jinja_optional_container_if(expected_level, o_if, html_tag, c_if,
+                                          inline=False):
+        check_indent(expected_level, o_if, inline=inline)
+        shift = 0 if inline else indent_size
+        if isinstance(html_tag, ast.OpeningTag):
+            check_opening_tag(expected_level + shift, html_tag, inline=inline)
+        elif isinstance(html_tag, ast.ClosingTag):
+            check_indent(expected_level + shift, html_tag, inline=inline)
+        else:
+            raise AssertionError('invalid tag')
+        check_indent(expected_level, c_if, inline=inline)
+        return inline
+
+    def check_jinja_optional_container(expected_level, element, inline=False):
+        if element.first_opening_if.begin.line == \
+                element.second_opening_if.end.line:
+            inline = True
+
+        inline = check_jinja_optional_container_if(
+            expected_level,
+            element.first_opening_if,
+            element.opening_tag,
+            element.first_closing_if,
+            inline=inline)
+
+        check_content(expected_level, element.content, inline=inline)
+
+        check_jinja_optional_container_if(
+            expected_level,
+            element.second_opening_if,
+            element.closing_tag,
+            element.second_closing_if,
+            inline=inline)
+
     def check_jinja_element(expected_level, element, inline=False):
         if element.begin.line == element.end.line:
             inline = True
@@ -155,6 +194,7 @@ def check_node(expected_level, node, inline=False):
             ast.JinjaComment: check_jinja_comment,
             ast.JinjaElement: check_jinja_element,
             ast.JinjaElementPart: check_jinja_element_part,
+            ast.JinjaOptionalContainer: check_jinja_optional_container,
             ast.JinjaTag: check_jinja_tag,
             ast.JinjaVariable: check_jinja_variable,
             ast.String: check_string,

jinjalint/cli.py

@@ -21,7 +21,11 @@
 def print_issues(issues, config):
     sorted_issues = sorted(
         issues,
-        key=lambda i: (i.location.file_path, i.location.line),
+        key=lambda i: (
+            i.location.file_path,
+            i.location.line,
+            i.location.column
+        ),
     )
 
     for issue in sorted_issues:

jinjalint/parse.py

@@ -5,7 +5,7 @@
 from .ast import (
     Attribute, ClosingTag, Comment, Element, Integer, Interpolated,
     Jinja, JinjaComment, JinjaElement, JinjaElementPart, JinjaTag,
-    JinjaVariable, Location, OpeningTag, String,
+    JinjaVariable, JinjaOptionalContainer, Location, OpeningTag, String,
 )
 from .util import flatten
 
@@ -34,7 +34,7 @@
     ('filter', 'endfilter'),
     ('for', 'else', 'empty', 'endfor'),
     ('if', 'elif', 'else', 'endif'),
-    ('ifchanged', 'endifchanged'),
+    ('ifchanged', 'else', 'endifchanged'),
     ('ifequal', 'endifequal'),
     ('ifnotequal', 'endifnotequal'),
     ('spaceless', 'endspaceless'),
@@ -169,6 +169,13 @@ def make_jinja_element_part_parser(name_parser, content):
 
 
 def make_jinja_element_parser(name_parsers, content):
+    """
+    `name_parsers` must be a list of tag name parsers. For example,
+    `name_parsers` can be defined as follow in order to parse `if` statements:
+
+        name_parsers = [P.string(n) for n in ['if', 'elif', 'else', 'endif']]
+    """
+
     if len(name_parsers) == 1:
         tag = make_jinja_tag_parser(name_parsers[0])
         part = locate(P.seq(
@@ -454,12 +461,84 @@ def make_raw_text_element_parser(config, tag_name, jinja):
 quick_text = P.regex(r'[^{<]+', flags=re.DOTALL)
 
 
-def make_jinja_parser(config, content):
-    jinja_structured_elements_names = (
-        DEFAULT_JINJA_STRUCTURED_ELEMENTS_NAMES +
-        config.get('jinja_custom_elements_names', [])
+def _combine_optional_container(locations, nodes):
+    return JinjaOptionalContainer(
+        first_opening_if=nodes[0],
+        opening_tag=nodes[1],
+        first_closing_if=nodes[2],
+        content=nodes[3],
+        second_opening_if=nodes[4],
+        closing_tag=nodes[5],
+        second_closing_if=nodes[6],
+        **locations,
+    )
+
+
+# Awkward hack to handle optional HTML containers, for example:
+#
+#   {% if a %}
+#     <div>
+#   {% endif %}
+#   foo
+#   {% if a %}
+#     </div>
+#   {% endif %}
+#
+# Currently, this only works with `if` statements and the two conditions
+# must be exactly the same.
+def make_jinja_optional_container_parser(config, content, jinja):
+    jinja_if = make_jinja_tag_parser(P.string('if'))
+    jinja_endif = make_jinja_tag_parser(P.string('endif'))
+    opening_tag = make_opening_tag_parser(config, jinja, allow_slash=False)
+
+    @P.generate
+    def opt_container_impl():
+        o_first_if_node = yield jinja_if.skip(whitespace)
+        o_tag_node = yield opening_tag.skip(whitespace)
+        c_first_if_node = yield jinja_endif
+
+        content_nodes = yield content
+
+        o_second_if_node = yield jinja_if.skip(whitespace)
+        if o_second_if_node.content != o_first_if_node.content:
+            yield P.fail('expected `{% if ' + content + ' %}`')
+            return
+        html_tag_name = o_tag_node.name
+        if isinstance(html_tag_name, str):
+            closing_tag = make_closing_tag_parser(P.string(html_tag_name))
+        else:
+            assert isinstance(html_tag_name, Jinja)
+            closing_tag = make_closing_tag_parser(jinja)
+        c_tag_node = yield closing_tag.skip(whitespace)
+        c_second_if_node = yield jinja_endif
+
+        return [
+            o_first_if_node,
+            o_tag_node,
+            c_first_if_node,
+            content_nodes,
+            o_second_if_node,
+            c_tag_node,
+            c_second_if_node,
+        ]
+
+    return (
+        locate(opt_container_impl)
+        .combine(_combine_optional_container)
     )
 
+
+def make_jinja_parser(config, content):
+    # Allow to override elements with the configuration
+    jinja_structured_elements_names = dict(
+        (names[0], names)
+        for names
+        in (
+            DEFAULT_JINJA_STRUCTURED_ELEMENTS_NAMES +
+            config.get('jinja_custom_elements_names', [])
+        )
+    ).values()
+
     jinja_structured_element = P.alt(*[
         make_jinja_element_parser(
             [P.string(name) for name in names],
@@ -469,20 +548,29 @@ def make_jinja_parser(config, content):
     ])
 
     # These tag names can't begin a Jinja element
-    jinja_intermediate_tag_names = [
+    jinja_intermediate_tag_names = set(
         n
         for _, *sublist in jinja_structured_elements_names
         for n in sublist
-    ]
+    )
 
     jinja_intermediate_tag_name = P.alt(*(
         P.string(n) for n in jinja_intermediate_tag_names
     ))
 
     jinja_element_single = make_jinja_element_parser(
-        [jinja_intermediate_tag_name
-         .should_fail('not an intermediate Jinja tag name')
-         .then(jinja_name)],
+        [
+            P.alt(
+                # HACK: If we allow `{% if %}`s without `{% endif %}`s here,
+                # `make_jinja_optional_container_parser` doesn’t work. It
+                # is probably better to reject any structured tag name here.
+                P.string('if'),
+
+                jinja_intermediate_tag_name
+            )
+            .should_fail('not an intermediate Jinja tag name')
+            .then(jinja_name)
+        ],
         content=content,
     )
 
@@ -557,12 +645,18 @@ def content():
 
     element = make_element_parser(config, content, jinja)
 
+    opt_container = make_jinja_optional_container_parser(
+        config, content, jinja)
+
     content_ = interpolated(
-        (quick_text | comment | dtd | element | jinja | slow_text_char).many()
+        (
+            quick_text | comment | dtd | element | opt_container | jinja |
+            slow_text_char
+        ).many()
     )
 
     return {
-        'content': content,
+        'content': content_,
         'jinja': jinja,
         'element': element,
     }

jinjalint/parse_test.py

@@ -1,7 +1,7 @@
 import parsy as P
 
 from .parse import (
-    tag_name, tag_name_char, comment,
+    tag_name, tag_name_char, comment, jinja_comment,
     make_attribute_value_parser, make_attribute_parser,
     make_attributes_parser,
     make_closing_tag_parser, make_opening_tag_parser, make_parser,
@@ -54,6 +54,7 @@ def create_node(*args, **kwargs):
 Element = with_dummy_locations(ast.Element)
 ClosingTag = with_dummy_locations(ast.ClosingTag)
 Comment = with_dummy_locations(ast.Comment)
+JinjaComment = with_dummy_locations(ast.JinjaComment)
 Integer = with_dummy_locations(ast.Integer)
 Interp = with_dummy_locations(ast.Interpolated)
 JinjaComment = with_dummy_locations(ast.JinjaComment)
@@ -168,6 +169,12 @@ def test_comment():
     )
 
 
+def test_jinja_comment():
+    assert jinja_comment.parse('{# hello world #}') == JinjaComment(
+        text='hello world',
+    )
+
+
 def test_opening_tag():
     assert opening_tag.parse('<div>') == OpeningTag(
         name='div',
@@ -298,6 +305,9 @@ def test_self_closing_elements():
         closing_tag=None,
     )
 
+    src = '<br />'
+    assert src == str(element.parse(src))
+
 
 def test_jinja_blocks():
     assert jinja.parse('{% name something == 123 %}') == JinjaElement(
@@ -360,6 +370,7 @@ def test():
     test_attribute_value()
     test_attribute()
     test_comment()
+    test_jinja_comment()
     test_opening_tag()
     test_closing_tag()
     test_raw_text_elements()
@@ -372,5 +383,5 @@ def test():
     src = '<html lang="fr"><body>Hello<br></body></html>'
     assert src == str(element.parse(src))
 
-    src = '<br />'
-    assert src == str(element.parse(src))
+    src = '{% if a %}<a href="b">{% endif %}c<b>d</b>{% if a %}</a>{% endif %}'
+    assert src == str(content.parse(src))