Merge pull request #53 from jamescooke/line_markers

Add line-wise analysis and docs

CHANGELOG.rst

@@ -22,6 +22,9 @@ Added
 * Command line functionality now available to assist with development and
   debugging.
 
+* New line-wise analysis, including updated blank line checking and a new
+  ``AAA99`` rule for node to line mapping collisions.
+
 Removed
 -------
 

Makefile

@@ -1,5 +1,5 @@
 lint_files=setup.py flake8_aaa tests
-rst_files=README.rst CHANGELOG.rst docs/discovery.rst docs/rules.rst
+rst_files=README.rst CHANGELOG.rst
 
 venv:
 	virtualenv venv --python=python3
@@ -22,7 +22,7 @@ lint:
 	@echo "=== flake8 ==="
 	flake8 $(lint_files) examples
 	@echo "=== mypy ==="
-	mypy flake8_aaa --ignore-missing-imports
+	$(MAKE) mypy
 	@echo "=== pylint ==="
 	./run_pylint.sh flake8_aaa
 	@echo "=== isort ==="
@@ -35,6 +35,10 @@ lint:
 	@echo "=== setup.py ==="
 	python setup.py check --metadata --strict
 
+.PHONY: mypy
+mypy:
+	mypy flake8_aaa tests --ignore-missing-imports
+
 .PHONY: test
 test:
 	pytest tests

README.rst

@@ -19,13 +19,21 @@
     :target: https://github.com/jamescooke/flake8-aaa/blob/master/LICENSE
 
 
-flake8-aaa
+Flake8-AAA
 ==========
 
-A `Flake8 <http://flake8.pycqa.org/en/latest/index.html>`_ plugin that checks
-Python tests follow the `Arrange Act Assert pattern
-<http://jamescooke.info/arrange-act-assert-pattern-for-python-developers.html>`_.
+A linter for Python tests.
 
+* Pytest and unittest styles supported.
+
+* Tests are linted against the `Arrange Act Assert pattern
+  <http://jamescooke.info/arrange-act-assert-pattern-for-python-developers.html>`_.
+
+* Provides a Flake8 interface to automatically lint test files as part of your
+  Flake8 run.
+
+* Provides a command line interface for custom (non-Flake8) usage and
+  debugging.
 
 Installation
 ------------
@@ -34,25 +42,21 @@ Install with ``pip``::
 
     $ pip install flake8-aaa
 
-Check that ``flake8-aaa`` was installed correctly by asking ``flake8`` for its
+Integration with Flake8
+-----------------------
+
+Check that Flake8-AAA was installed correctly by asking ``flake8`` for its
 version signature::
 
     $ flake8 --version
     3.5.0 (aaa: 0.4.0, mccabe: 0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0) CPython 3.5.2 on Linux
 
 The ``aaa: 0.4.0`` part of that output tells you ``flake8`` found this plugin.
-
-Usage
------
-
-Run ``flake8`` as usual against your project and ``flake8-aaa`` will check your
-tests::
+Now you can run ``flake8`` as usual against your project and Flake8-AAA will
+lint your tests::
 
     $ flake8
 
-There is more information on invoking ``flake8`` on the `Invoking Flake8
-<http://flake8.pycqa.org/en/latest/user/invocation.html>`_ documentation page.
-
 
 Resources
 ---------
@@ -67,5 +71,9 @@ Resources
 
 * `Changelog <https://github.com/jamescooke/flake8-aaa/blob/master/CHANGELOG.rst>`_
 
-Tested on Pythons 3.5 and 3.6. Python 2 supported up to `v0.4.0
-<https://pypi.org/project/flake8-aaa/0.4.0/>`_.
+Tested on Pythons 3.5 and 3.6.
+
+Python 2 supported up to ``v0.4.0``:
+`pypi <https://pypi.org/project/flake8-aaa/0.4.0/>`_,
+`docs <https://flake8-aaa.readthedocs.io/en/v0.4.0/>`_,
+`tag <https://github.com/jamescooke/flake8-aaa/releases/tag/v0.4.0>`_.

docs/commands.rst

@@ -18,6 +18,8 @@ Disabling Flake8-AAA selectively
 When Flake8-AAA finds the ``# noqa`` comment at the end of the line that
 defines a test function, it will ignore it.
 
+.. _command-line:
+
 Command line
 ------------
 
@@ -68,6 +70,8 @@ And once the error above is fixed, the return value returns to zero::
     $ echo "$?"
     0 
 
+.. _line-markers:
+
 Line markers
 ............
 

docs/rules.rst

@@ -89,3 +89,21 @@ Resolution
 ..........
 
 Add a blank line before the Assert block.
+
+AAA99: collision when marking this line as NEW_CODE, was already OLD_CODE
+-------------------------------------------------------------------------
+
+This is an error code that is raised when Flake8 tries to mark a single line as
+occupied by two different types of block. It *should* never happen. The values
+for ``NEW_CODE`` and ``OLD_CODE`` are from the list of :ref:`line-markers`.
+
+Resolution
+..........
+
+Please open a `new issue
+<https://github.com/jamescooke/flake8-aaa/issues/new>`_ containing the output
+for the failing test as generated by the :ref:`command-line` tool.
+
+You could hack around with your test to see if you can get it to work while
+waiting for someone to reply to your issue. If you're able to adjust the test
+to get it to work, that updated test would also be helpful for debugging.

flake8_aaa/__about__.py

@@ -1,9 +1,9 @@
 __short_name__ = 'aaa'
-__name__ = 'flake8-{}'.format(__short_name__)
+__iam__ = 'flake8-{}'.format(__short_name__)
 __version__ = '0.4.0'
 
 __author__ = 'James Cooke'
 __copyright__ = '2018, {}'.format(__author__)
 
-__description__ = 'Extends Flake8 to check Python tests follow the Arrange Act Assert pattern'
+__description__ = 'A linter for Python tests'
 __email__ = 'github@jamescooke.info'

flake8_aaa/act_block.py

@@ -2,7 +2,7 @@
 from typing import List, Type, TypeVar
 
 from .helpers import node_is_pytest_raises, node_is_result_assignment, node_is_unittest_raises
-from .types import ActBlockType, LineType
+from .types import ActBlockType
 
 AB = TypeVar('AB', bound='ActBlock')  # Place holder for ActBlock instances
 
@@ -24,7 +24,7 @@ def __init__(self, node: ast.AST, block_type: ActBlockType) -> None:
         self.block_type = block_type  # type: ActBlockType
 
     @classmethod
-    def build_body(cls: Type[AB], body: List[ast.stmt]):
+    def build_body(cls: Type[AB], body: List[ast.stmt]) -> List:
         """
         Note:
             Return type is probably ``-> List[AB]``, but can't get it to pass.
@@ -53,22 +53,3 @@ def build(cls: Type[AB], node: ast.AST) -> List[AB]:
             return cls.build_body(node.body)
 
         return []
-
-    def mark_line_types(self, line_types: List[LineType], first_line_no: int) -> List[LineType]:
-        """
-        Marks the lines occupied by this ActBlock.
-
-        Note:
-            Mutates the ``line_types`` list.
-
-        Raises:
-            AssertionError: When position in ``line_types`` has already been
-                marked to something other than ``???:unprocessed``.
-        """
-        # Lines calculated relative to file
-        start_line = self.node.first_token.start[0]  # type:ignore
-        end_line = self.node.last_token.end[0]  # type:ignore
-        for file_line_no in range(start_line, end_line + 1):
-            assert line_types[file_line_no - first_line_no] is LineType.unprocessed
-            line_types[file_line_no - first_line_no] = LineType.act_block
-        return line_types

flake8_aaa/function.py

@@ -5,7 +5,15 @@
 from .arrange_block import ArrangeBlock
 from .assert_block import AssertBlock
 from .exceptions import ValidationError
-from .helpers import format_errors, function_is_noop
+from .helpers import (
+    build_footprint,
+    build_multinode_footprint,
+    format_errors,
+    function_is_noop,
+    get_first_token,
+    get_last_token,
+)
+from .line_markers import LineMarkers
 from .types import ActBlockType, LineType
 
 
@@ -19,7 +27,7 @@ class Function:
             Function has been checked and is free of errors.
         first_line_no
         lines
-        line_types
+        line_markers: Line-wise marking for this function.
         node: AST for the test under lint.
     """
 
@@ -38,14 +46,14 @@ def __init__(self, node: ast.FunctionDef, file_lines: List[str]):
         self.act_block = None  # type: Optional[ActBlock]
         self.assert_block = None  # type: Optional[AssertBlock]
         self._errors = None  # type: Optional[List[Tuple[int, int, str, type]]]
-        self.line_types = len(self.lines) * [LineType.unprocessed]  # type: List[LineType]
+        self.line_markers = LineMarkers(len(self.lines))  # type: LineMarkers
 
     def __str__(self) -> str:
         out = '------+------------------------------------------------------------------------\n'
         for i, line in enumerate(self.lines):
             out += '{line_no:>2} {block}|{line}'.format(
                 line_no=i + self.first_line_no,
-                block=str(self.line_types[i]),
+                block=str(self.line_markers[i]),
                 line=line,
             )
             if self._errors:
@@ -64,21 +72,34 @@ def check_all(self) -> None:
             ValidationError: When an error is found.
         """
         # Function def
-        self.mark_line_types()
+        self.mark_def()
         if function_is_noop(self.node):
             return
         # ACT
         self.act_block = self.load_act_block()
-        self.act_block.mark_line_types(self.line_types, self.first_line_no)
+        self.line_markers.update(
+            build_footprint(self.act_block.node, self.first_line_no),
+            LineType.act_block,
+            self.first_line_no,
+        )
         # ARRANGE
         self.arrange_block = self.load_arrange_block()
         if self.arrange_block:
-            self.arrange_block.mark_line_types(self.line_types, self.first_line_no)
+            self.line_markers.update(
+                build_multinode_footprint(self.arrange_block.nodes, self.first_line_no),
+                LineType.arrange_block,
+                self.first_line_no,
+            )
         # ASSERT
         self.assert_block = self.load_assert_block()
         if self.assert_block:
-            self.assert_block.mark_line_types(self.line_types, self.first_line_no)
+            self.line_markers.update(
+                build_multinode_footprint(self.assert_block.nodes, self.first_line_no),
+                LineType.assert_block,
+                self.first_line_no,
+            )
         # SPACING
+        self.mark_bl()
         self.check_arrange_act_spacing()
         self.check_act_assert_spacing()
 
@@ -105,7 +126,8 @@ def get_errors(self) -> List[Tuple[int, int, str, type]]:
     def load_act_block(self) -> ActBlock:
         """
         Raises:
-            ValidationError
+            ValidationError: AAA01 when no act block is found and AAA02 when
+                multiple act blocks are found.
         """
         act_blocks = ActBlock.build_body(self.node.body)
 
@@ -194,14 +216,57 @@ def get_line_relative_to_node(self, target_node: ast.AST, offset: int) -> str:
         """
         return self.lines[target_node.lineno - self.node.lineno + offset]
 
-    def mark_line_types(self) -> None:
+    def mark_def(self) -> int:
         """
-        Mark up the test function with function def and blank lines.
+        Marks up this Function's definition lines (including decorators) into
+        the ``line_markers`` attribute.
+
+        Returns:
+            Number of lines found for the definition.
 
         Note:
-            Mutates the ``line_types`` attribute.
+            Does not spot the closing ``):`` of a function when it occurs on
+            its own line.
+
+        Note:
+            Can not use ``helpers.build_footprint()`` because function nodes
+            cover the whole function. In this case, just the def lines are
+            wanted with any decorators.
         """
-        self.line_types[0] = LineType.func_def
-        for i, line in enumerate(self.lines):
-            if line.strip() == '':
-                self.line_types[i] = LineType.blank_line
+        first_line = get_first_token(self.node).start[0] - self.first_line_no  # Should usually be 0
+        try:
+            end_token = get_last_token(self.node.args.args[-1])
+        except IndexError:
+            # Fn has no args, so end of function is the fn def itself...
+            end_token = get_first_token(self.node)
+        last_line = end_token.end[0] - self.first_line_no
+        lines = range(first_line, last_line + 1)
+        self.line_markers.update(lines, LineType.func_def, self.first_line_no)
+        return len(lines)
+
+    def mark_bl(self) -> int:
+        """
+        Mark unprocessed lines that have no content and no nodes covering them
+        as blank line BL.
+
+        Returns:
+            Number of blank lines found with no parent node.
+        """
+        counter = 0
+        for i, line_marker in enumerate(self.line_markers):
+            if line_marker is not LineType.unprocessed or self.lines[i].strip() != '':
+                continue
+            covered = False
+            for node in self.node.body:
+                # Check if this line is covered by any nodes in the function
+                # and if so, then set the covered flag and bail out
+                if i in build_footprint(node, self.first_line_no):
+                    covered = True
+                    break
+            if covered:
+                continue
+
+            counter += 1
+            self.line_markers[i] = LineType.blank_line
+
+        return counter