Repository: jamescooke/flake8-aaa

Issue:
Add AAA03
When there exists an Arrange block, then:

"Expected 1 blank line before Act block, found none"

Discussion:

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e6440d2..3154922 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -11,6 +11,11 @@ Versioning <http://semver.org/spec/v2.0.0.html>`_.
 Unreleased_
 -----------
 
+Added
+-----
+
+* New rule ``AAA03`` "expected 1 blank line before Act block, found none"
+
 0.2.0_ - 2018/05/28
 -------------------
 
diff --git a/docs/rules.rst b/docs/rules.rst
index 683715d..236e05c 100644
--- a/docs/rules.rst
+++ b/docs/rules.rst
@@ -13,10 +13,8 @@ Resolution
 ..........
 
 Add an Act block to the test or mark a line that should be considered the
-action.
-
-Even if the result of a test action is ``None``, assign that result and pint it
-with a test::
+action. Even if the result of a test action is ``None``, assign that result and
+pin it with a test::
 
     result = action()
 
@@ -41,3 +39,22 @@ Resolution
 
 Splitting the failing test into multiple tests. Where there is complicated or
 reused set-up code then that should be extracted into one or more fixtures.
+
+AAA03: expected 1 blank line before Act block, found none
+---------------------------------------------------------
+
+For tests that have an Arrange block, there must be a blank line between the
+Arrange and Act blocks. The linter could not find a blank line before the Act
+block.
+
+This blank line creates separation between the arrangement and the action and
+makes the Act block easy to spot.
+
+This rule works best with `pycodestyle
+<https://pypi.org/project/pycodestyle/>`_'s ``E303`` rule enabled because it
+ensures that there are not multiple blank lines between the blocks.
+
+Resolution
+..........
+
+Add a blank line before the Act block.
diff --git a/flake8_aaa/arrange_block.py b/flake8_aaa/arrange_block.py
new file mode 100644
index 0000000..a27fb24
--- /dev/null
+++ b/flake8_aaa/arrange_block.py
@@ -0,0 +1,11 @@
+class ArrangeBlock:
+    """
+    Attributes:
+        nodes (list (ast Node))
+    """
+
+    def __init__(self):
+        self.nodes = []
+
+    def add_node(self, node):
+        self.nodes.append(node)
diff --git a/flake8_aaa/checker.py b/flake8_aaa/checker.py
index 6bb15e0..3175723 100644
--- a/flake8_aaa/checker.py
+++ b/flake8_aaa/checker.py
@@ -41,6 +41,6 @@ class Checker:
             self.load()
             for function_def in find_test_functions(self.tree):
                 try:
-                    Function(function_def).check_all()
+                    Function(function_def, self.lines).check_all()
                 except ValidationError as error:
                     yield error.to_flake8(type(self))
diff --git a/flake8_aaa/function.py b/flake8_aaa/function.py
index 89745ea..a10c8aa 100644
--- a/flake8_aaa/function.py
+++ b/flake8_aaa/function.py
@@ -1,4 +1,5 @@
 from .act_block import ActBlock
+from .arrange_block import ArrangeBlock
 from .exceptions import NotActionBlock, ValidationError
 from .helpers import function_is_noop
 from .types import ActBlockType
@@ -11,12 +12,14 @@ class Function:
         node (ast.FunctionDef): AST for the test under lint.
     """
 
-    def __init__(self, node):
+    def __init__(self, node, file_lines):
         """
         Args:
             node (ast.FunctionDef)
+            file_lines (list (str)): Lines of file under test.
         """
         self.node = node
+        self.lines = file_lines[self.node.lineno - 1:self.node.last_token.end[0]]
         self.act_block = None
 
     def check_all(self):
@@ -33,6 +36,8 @@ class Function:
             return
 
         self.act_block = self.load_act_block()
+        self.arrange_block = self.load_arrange_block()
+        self.check_act_arrange_spacing()
 
     def load_act_block(self):
         """
@@ -61,3 +66,61 @@ class Function:
             raise ValidationError(self.node.lineno, self.node.col_offset, 'AAA02 multiple Act blocks found in test')
 
         return act_blocks[0]
+
+    def load_arrange_block(self):
+        """
+        Returns:
+            ArrangeBlock: Or ``None`` if no Act block is found.
+
+        Raises:
+            ValidationError
+        """
+        arrange_block = ArrangeBlock()
+        for node in self.node.body:
+            if node == self.act_block.node:
+                break
+            arrange_block.add_node(node)
+
+        if len(arrange_block.nodes) > 0:
+            return arrange_block
+
+        return None
+
+    def check_act_arrange_spacing(self):
+        """
+        When Function has an Arrange block, then ensure that there is a blank
+        line between that and the Act block.
+
+        Returns:
+            None
+
+        Raises:
+            ValidationError: When no space found.
+
+        Note:
+            Due to Flake8's error ``E303``, we do not have to check that there
+            is more than one space.
+        """
+        if self.arrange_block:
+            line_before_act = self.get_line_relative_to_node(self.act_block.node, -1)
+            if line_before_act != '\n':
+                raise ValidationError(
+                    line_number=self.act_block.node.lineno,
+                    offset=self.act_block.node.col_offset,
+                    text='AAA03 expected 1 blank line before Act block, found none',
+                )
+
+    def get_line_relative_to_node(self, target_node, offset):
+        """
+        Args:
+            target_node (ast.node)
+            offset (int)
+
+        Returns:
+            str
+
+        Raises:
+            IndexError: when ``offset`` takes the request out of bounds of this
+                Function's lines.
+        """
+        return self.lines[target_node.lineno - self.node.lineno + offset]
diff --git a/setup.cfg b/setup.cfg
index 9211379..da865d7 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -9,6 +9,6 @@ not_skip=__init__.py
 max-line-length = 120
 
 [yapf]
-# coalesce_brackets = true
+coalesce_brackets = true
 dedent_closing_brackets = true
 column_limit = 120


Test patch:
diff --git a/tests/arrange_block/__init__.py b/tests/arrange_block/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/arrange_block/test_add_node.py b/tests/arrange_block/test_add_node.py
new file mode 100644
index 0000000..919c3be
--- /dev/null
+++ b/tests/arrange_block/test_add_node.py
@@ -0,0 +1,16 @@
+import pytest
+
+from flake8_aaa.arrange_block import ArrangeBlock
+
+
+@pytest.mark.parametrize('code_str', ['''
+def test():
+    pass
+'''])
+def test(first_node_with_tokens):
+    arrange_block = ArrangeBlock()
+
+    result = arrange_block.add_node(first_node_with_tokens)
+
+    assert result is None
+    assert arrange_block.nodes == [first_node_with_tokens]
diff --git a/tests/arrange_block/test_init.py b/tests/arrange_block/test_init.py
new file mode 100644
index 0000000..c7a5492
--- /dev/null
+++ b/tests/arrange_block/test_init.py
@@ -0,0 +1,7 @@
+from flake8_aaa.arrange_block import ArrangeBlock
+
+
+def test():
+    result = ArrangeBlock()
+
+    assert result.nodes == []
diff --git a/tests/conftest.py b/tests/conftest.py
index 98682c2..0ad8819 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -65,3 +65,14 @@ def first_node_with_tokens(code_str):
     tree = ast.parse(code_str)
     asttokens.ASTTokens(code_str, tree=tree)
     return tree.body[0]
+
+
+@pytest.fixture
+def lines(code_str):
+    """
+    Given ``code_str`` chop it into lines as flake8 would pass to a plugin.
+
+    Returns:
+        list (str)
+    """
+    return ['{}\n'.format(line) for line in code_str.split('\n')]
diff --git a/tests/fixtures/__init__.py b/tests/fixtures/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/test_fixtures.py b/tests/fixtures/test_first_token.py
similarity index 86%
rename from tests/test_fixtures.py
rename to tests/fixtures/test_first_token.py
index 19ba221..7c99ef1 100644
--- a/tests/test_fixtures.py
+++ b/tests/fixtures/test_first_token.py
@@ -6,7 +6,7 @@ import six
 
 @pytest.mark.skipif(six.PY2, reason='py2')
 @pytest.mark.parametrize('code_str', ['# stuff'])
-def test_comment_token_py3(first_token):
+def test_first_token_py3(first_token):
     result = first_token
 
     assert isinstance(result, tokenize.TokenInfo)
@@ -16,7 +16,7 @@ def test_comment_token_py3(first_token):
 
 @pytest.mark.skipif(six.PY3, reason='py3')
 @pytest.mark.parametrize('code_str', ['# stuff'])
-def test_comment_token_py2(first_token):
+def test_first_token_py2(first_token):
     result = first_token
 
     assert isinstance(result, tuple)
diff --git a/tests/fixtures/test_lines.py b/tests/fixtures/test_lines.py
new file mode 100644
index 0000000..7b14da5
--- /dev/null
+++ b/tests/fixtures/test_lines.py
@@ -0,0 +1,16 @@
+import pytest
+
+
+@pytest.mark.parametrize('code_str', ["""
+def test():
+    pass
+"""])
+def test_lines(lines):
+    result = lines
+
+    assert result == [
+        '\n',
+        'def test():\n',
+        '    pass\n',
+        '\n',
+    ]
diff --git a/tests/function/conftest.py b/tests/function/conftest.py
index 3949659..693eb5a 100644
--- a/tests/function/conftest.py
+++ b/tests/function/conftest.py
@@ -4,9 +4,30 @@ from flake8_aaa.function import Function
 
 
 @pytest.fixture
-def function(first_node_with_tokens):
+def function(first_node_with_tokens, lines):
     """
     Returns:
-        Function: Loaded with ``first_node_with_tokens`` node.
+        Function: Loaded with ``first_node_with_tokens`` node, lines of test
+        are passed to Function.
     """
-    return Function(first_node_with_tokens)
+    return Function(first_node_with_tokens, lines)
+
+
+@pytest.fixture
+def function_with_act_block(function):
+    """
+    Returns:
+        Function: With Act block loaded.
+    """
+    function.act_block = function.load_act_block()
+    return function
+
+
+@pytest.fixture
+def function_with_arrange_act_blocks(function_with_act_block):
+    """
+    Returns:
+        Function: With Arrange and Act blocks loaded.
+    """
+    function_with_act_block.arrange_block = function_with_act_block.load_arrange_block()
+    return function_with_act_block
diff --git a/tests/function/test_check_act_arrange_spacing.py b/tests/function/test_check_act_arrange_spacing.py
new file mode 100644
index 0000000..c06e9c9
--- /dev/null
+++ b/tests/function/test_check_act_arrange_spacing.py
@@ -0,0 +1,61 @@
+import pytest
+
+from flake8_aaa.exceptions import ValidationError
+
+
+@pytest.mark.parametrize(
+    'code_str',
+    ['''
+def test():
+    result = 1
+
+    assert result == 1
+'''],
+    ids=['no arrange'],
+)
+def test_no_arrange(function_with_arrange_act_blocks):
+    result = function_with_arrange_act_blocks.check_act_arrange_spacing()
+
+    assert result is None
+
+
+@pytest.mark.parametrize(
+    'code_str',
+    ['''
+def test():
+    x = 1
+    y = 2
+
+    result = x + y
+
+    assert result == 3
+'''],
+    ids=['well spaced test'],
+)
+def test_has_act_block_good_spacing(function_with_arrange_act_blocks):
+    result = function_with_arrange_act_blocks.check_act_arrange_spacing()
+
+    assert result is None
+
+
+# --- FAILURES ---
+
+
+@pytest.mark.parametrize(
+    'code_str',
+    ['''
+def test():
+    x = 1
+    y = 2
+    result = x + y
+    assert result == 3
+'''],
+    ids=['compact test'],
+)
+def test_missing_leading_space(function_with_arrange_act_blocks):
+    with pytest.raises(ValidationError) as excinfo:
+        function_with_arrange_act_blocks.check_act_arrange_spacing()
+
+    assert excinfo.value.line_number == 5
+    assert excinfo.value.offset == 4
+    assert excinfo.value.text.startswith('AAA03 ')
diff --git a/tests/function/test_check_all.py b/tests/function/test_check_all.py
index 6b98437..eccdfba 100644
--- a/tests/function/test_check_all.py
+++ b/tests/function/test_check_all.py
@@ -1,13 +1,46 @@
 import pytest
 
+from flake8_aaa.exceptions import ValidationError
+
 
 @pytest.mark.parametrize(
-    'code_str', [
+    'code_str',
+    [
         'def test():\n    pass',
         'def test_docstring():\n    """This test will work great"""',
-    ]
+    ],
+    ids=['pass', 'docstring'],
 )
 def test_noop(function):
     result = function.check_all()
 
     assert result is None
+
+
+@pytest.mark.parametrize(
+    'code_str',
+    [
+        '''
+def test(file_resource):
+    file_resource.connect()
+    result = file_resource.retrieve()
+
+    assert result.success is True
+''',
+        '''
+def test_push(queue):
+    item = Document()
+    queue.push(item)  # act
+
+    assert queue.pop() == item
+''',
+    ],
+    ids=['no line before result= act', 'no line before marked act'],
+)
+def test_missing_space_before_act(function):
+    with pytest.raises(ValidationError) as excinfo:
+        function.check_all()
+
+    assert excinfo.value.line_number == 4
+    assert excinfo.value.offset == 4
+    assert excinfo.value.text == 'AAA03 expected 1 blank line before Act block, found none'
diff --git a/tests/function/test_get_line_relative_to_node.py b/tests/function/test_get_line_relative_to_node.py
new file mode 100644
index 0000000..a4efa64
--- /dev/null
+++ b/tests/function/test_get_line_relative_to_node.py
@@ -0,0 +1,44 @@
+import pytest
+
+
+@pytest.mark.parametrize(
+    'code_str', [
+        '''
+
+# stuff
+
+def test():
+    x = 1
+    y = 2
+
+    # Do stuff
+    result = x + y
+    # Finished stuff
+
+    assert result == 4
+
+# End
+'''
+    ]
+)
+@pytest.mark.parametrize('offset, expected_line', [
+    [1, '    # Finished stuff\n'],
+    [-1, '    # Do stuff\n'],
+])
+def test(function, offset, expected_line):
+    act_block = function.load_act_block()
+
+    result = function.get_line_relative_to_node(act_block.node, -1)
+
+    assert result == '    # Do stuff\n'
+
+
+# --- FAILURES ---
+
+
+@pytest.mark.parametrize('code_str', [
+    'def test():\n    pass',
+])
+def test_out_of_bounds(function, first_node_with_tokens):
+    with pytest.raises(IndexError):
+        function.get_line_relative_to_node(first_node_with_tokens, 2)
diff --git a/tests/function/test_init.py b/tests/function/test_init.py
index 872d3d2..12858f6 100644
--- a/tests/function/test_init.py
+++ b/tests/function/test_init.py
@@ -4,11 +4,21 @@ from flake8_aaa.function import Function
 
 
 @pytest.mark.parametrize('code_str', ['''
+
 def test():
     pass
+
+def test_other():
+    pass
+
+# All done :D
 '''])
-def test(first_node_with_tokens):
-    result = Function(first_node_with_tokens)
+def test(first_node_with_tokens, lines):
+    result = Function(first_node_with_tokens, lines)
 
     assert result.node == first_node_with_tokens
+    assert result.lines == [
+        'def test():\n',
+        '    pass\n',
+    ]
     assert result.act_block is None
diff --git a/tests/function/test_load_arrange_block.py b/tests/function/test_load_arrange_block.py
new file mode 100644
index 0000000..c085a35
--- /dev/null
+++ b/tests/function/test_load_arrange_block.py
@@ -0,0 +1,36 @@
+import pytest
+
+from flake8_aaa.arrange_block import ArrangeBlock
+
+
+@pytest.mark.parametrize('code_str', ['''
+def test():
+    result = 1
+
+    assert result == 1
+'''])
+def test_none(function_with_act_block):
+    function_with_act_block.load_act_block()
+
+    result = function_with_act_block.load_arrange_block()
+
+    assert result is None
+
+
+@pytest.mark.parametrize(
+    'code_str', ['''
+def test():
+    x = 1
+    y = 1
+
+    result = x + y
+
+    assert result == 2
+''']
+)
+def test_simple(function_with_act_block):
+    function_with_act_block.load_act_block()
+
+    result = function_with_act_block.load_arrange_block()
+
+    assert isinstance(result, ArrangeBlock)