Merge pull request #157 from jamescooke/tokenize

Use tokens from Flake8 in analysis of blank lines

CHANGELOG.rst

@@ -21,13 +21,19 @@ Added
   guarantee compatibility. `#120
   <https://github.com/jamescooke/flake8-aaa/issues/120>`_.
 
+* Tokens now received from Flake8 to help with comment analysis. `#148
+  <https://github.com/jamescooke/flake8-aaa/issues/148>`_.
+
 Changed
 .......
 
 * Stringy line analysis adjusted to use Constant visitor since Str visitor is
   deprecated as of Python 3.8. `#145
   <https://github.com/jamescooke/flake8-aaa/issues/145>`_.
 
+* Blank line analysis now carried out using tokens rather than tokenised AST.
+  `#157 <https://github.com/jamescooke/flake8-aaa/pull/157>`_.
+
 0.10.0_ - 2020/05/24
 --------------------
 

src/flake8_aaa/block.py

@@ -2,7 +2,7 @@
 from typing import Iterable, List, Tuple, Type, TypeVar
 
 from .exceptions import EmptyBlock
-from .helpers import filter_arrange_nodes, filter_assert_nodes, get_first_token, get_last_token
+from .helpers import filter_arrange_nodes, get_first_token, get_last_token
 from .types import LineType
 
 _Block = TypeVar('_Block', bound='Block')
@@ -14,8 +14,7 @@ class Block:
 
     Note:
         This may just become the Act Block *AND* since the Act Block is just a
-        single node, this might not even be required. The get_span() method
-        could be extracted to be a helper function.
+        single node, this might not even be required.
 
     Args:
         nodes: Nodes that make up this block.
@@ -49,18 +48,6 @@ def build_arrange(cls: Type[_Block], nodes: List[ast.stmt], act_block_first_line
         """
         return cls(filter_arrange_nodes(nodes, act_block_first_line), LineType.arrange)
 
-    @classmethod
-    def build_assert(cls: Type[_Block], nodes: List[ast.stmt], min_line_number: int) -> _Block:
-        """
-        Assert block is all nodes that are after the Act node.
-
-        Note:
-            The filtering is *still* running off the line number of the Act
-            node, when instead it should be using the last line of the Act
-            block.
-        """
-        return cls(filter_assert_nodes(nodes, min_line_number), LineType._assert)
-
     def get_span(self, first_line_no: int) -> Tuple[int, int]:
         """
         Args:

src/flake8_aaa/checker.py

@@ -1,48 +1,56 @@
 from ast import AST
-from typing import Generator, List, Tuple
+from typing import Generator, List, Optional, Tuple
 
 import asttokens
 
 from .__about__ import __short_name__, __version__
-from .exceptions import ValidationError
+from .exceptions import TokensNotLoaded, ValidationError
 from .function import Function
 from .helpers import find_test_functions, is_test_file
 
 
 class Checker:
     """
     Attributes:
-        ast_tokens (asttokens.ASTTokens): Tokens for the file.
-        filename (str): Name of file under check.
-        lines (list (str))
-        tree (ast.AST): Tree passed from flake8.
+        ast_tokens: Tokens for the file.
+        filename: Name of file under check.
+        lines
+        tree: Tree passed from flake8.
     """
 
     name = __short_name__
     version = __version__
 
     def __init__(self, tree: AST, lines: List[str], filename: str):
-        """
-        Args:
-            tree
-            lines (list (str))
-            filename (str)
-        """
         self.tree = tree
         self.lines = lines
         self.filename = filename
-        self.ast_tokens = None
+        self.ast_tokens: Optional[asttokens.ASTTokens] = None
 
     def load(self) -> None:
         self.ast_tokens = asttokens.ASTTokens(''.join(self.lines), tree=self.tree)
 
     def all_funcs(self, skip_noqa: bool = False) -> Generator[Function, None, None]:
-        return (Function(f, self.lines) for f in find_test_functions(self.tree, skip_noqa=skip_noqa))
+        """
+        Note:
+            Checker is responsible for slicing the tokens passed to the
+            Function, BUT the function is reponsible for slicing the lines.
+            This is a bit strange - instead the lines should be sliced here and
+            passed in so that the Function only receives data about itself.
+
+        Raises:
+            TokensNotLoaded: On fetching first value, when `load()` has not
+                been called to populate `ast_tokens`.
+        """
+        if self.ast_tokens is None:
+            raise TokensNotLoaded("ast_tokens is `None`")
+        for f in find_test_functions(self.tree, skip_noqa=skip_noqa):
+            yield Function(f, self.lines, self.ast_tokens.get_tokens(f, include_extra=True))
 
     def run(self) -> Generator[Tuple[int, int, str, type], None, None]:
         """
         Yields:
-            tuple (line_number: int, offset: int, text: str, check: type)
+            tuple (line_number, offset, text, check)
         """
         if is_test_file(self.filename):
             self.load()

src/flake8_aaa/exceptions.py

@@ -20,6 +20,13 @@ class Flake8AAAException(Exception):
     pass
 
 
+class TokensNotLoaded(Flake8AAAException):
+    """
+    `Checker.all_funcs()` was called before `ast_tokens` was populated. Usually
+    this is done by `Checker.load()`.
+    """
+
+
 class EmptyBlock(Flake8AAAException):
     """
     Block has no nodes.

src/flake8_aaa/function.py

@@ -1,17 +1,11 @@
 import ast
+import tokenize
 from typing import Generator, List, Optional
 
 from .act_node import ActNode
 from .block import Block
 from .exceptions import AAAError, EmptyBlock, ValidationError
-from .helpers import (
-    find_stringy_lines,
-    format_errors,
-    function_is_noop,
-    get_first_token,
-    get_last_token,
-    line_is_comment,
-)
+from .helpers import format_errors, function_is_noop, get_first_token, get_last_token, line_is_comment
 from .line_markers import LineMarkers
 from .types import ActNodeType, LineType
 
@@ -35,18 +29,20 @@ class Function:
             method.
         line_markers: Line-wise marking for this function.
         node: AST for the test function / method.
+        tokens: Slice of the file's tokens that make up this test function.
 
     Note:
         "line number" means the number of the line in the file (the usual
         definition). "offset" means the number of the line in the test relative
         to the test definition.
     """
 
-    def __init__(self, node: ast.FunctionDef, file_lines: List[str]):
+    def __init__(self, node: ast.FunctionDef, file_lines: List[str], file_tokens: List[tokenize.TokenInfo]):
         """
         Args:
             node
             file_lines: Lines of file under test.
+            file_tokens: Tokens for file passed by Flake8.
         """
         self.node = node
         self.first_line_no: int = get_first_token(self.node).start[0]
@@ -57,6 +53,7 @@ def __init__(self, node: ast.FunctionDef, file_lines: List[str]):
         self.act_block: Optional[Block] = None
         self.assert_block: Optional[Block] = None
         self.line_markers = LineMarkers(self.lines, self.first_line_no)
+        self.tokens = file_tokens
 
     def __str__(self, errors: Optional[List[AAAError]] = None) -> str:
         out = '------+------------------------------------------------------------------------\n'
@@ -221,11 +218,6 @@ def mark_def(self) -> int:
         Note:
             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.
         """
         first_index = get_first_token(self.node).start[0] - self.first_line_no  # Should usually be 0
         try:
@@ -243,13 +235,16 @@ def mark_bl(self) -> int:
         covering them as blank line BL.
 
         Returns:
-            Number of blank lines found with no stringy parent node.
+            Number of blank lines found.
         """
         counter = 0
-        stringy_lines = find_stringy_lines(self.node, self.first_line_no)
-        for offset, line in enumerate(self.lines):
-            if offset not in stringy_lines and line.strip() == '':
-                counter += 1
-                self.line_markers.types[offset] = LineType.blank_line
+        previous = None
+        for t in self.tokens:
+            if t.type == tokenize.NL:
+                assert previous is not None, "Unexpected NL token before any other tokens seen"
+                if previous.type == tokenize.NL or previous.type == tokenize.NEWLINE:
+                    self.line_markers.types[t.start[0] - self.first_line_no] = LineType.blank_line
+                    counter += 1
+            previous = t
 
         return counter

src/flake8_aaa/helpers.py

@@ -2,9 +2,8 @@
 import io
 import os
 import re
-import sys
 import tokenize
-from typing import List, Set
+from typing import List
 
 from asttokens.util import Token
 
@@ -139,30 +138,6 @@ def get_last_token(node: ast.AST) -> Token:
     return node.last_token  # type: ignore
 
 
-def add_node_parents(root: ast.AST) -> None:
-    """
-    Adds "parent" attribute to all child nodes of passed node.
-
-    Code taken from https://stackoverflow.com/a/43311383/1286705
-    """
-    for node in ast.walk(root):
-        for child in ast.iter_child_nodes(node):
-            child.parent = node  # type: ignore
-
-
-def build_footprint(node: ast.AST, first_line_no: int) -> Set[int]:
-    """
-    Generates a list of lines that the passed node covers, relative to the
-    marked lines list - i.e. start of function is line 0.
-    """
-    return set(
-        range(
-            get_first_token(node).start[0] - first_line_no,
-            get_last_token(node).end[0] - first_line_no + 1,
-        )
-    )
-
-
 def filter_arrange_nodes(nodes: List[ast.stmt], act_block_first_line_number: int) -> List[ast.stmt]:
     """
     Args:
@@ -178,73 +153,13 @@ def filter_arrange_nodes(nodes: List[ast.stmt], act_block_first_line_number: int
     ]
 
 
-def filter_assert_nodes(nodes: List[ast.stmt], min_line_number: int) -> List[ast.stmt]:
-    """
-    Finds all nodes that are after the ``min_line_number``
-    """
-    return [node for node in nodes if node.lineno > min_line_number]
-
-
-class StringyLineVisitor(ast.NodeVisitor):
-    """
-    Find lines that look like strings. For each found, calculate its footprint.
-
-    Note:
-        Python 3.8.0 is starting to give warnings about `visit_Str` being
-        deprecated, but it's still required for 3.6. So some patching is used
-        to adjust this visitor for versions before 3.8.
-    """
-
-    def __init__(self, first_line_no: int):
-        super().__init__()
-        self.first_line_no: int = first_line_no
-        self.footprints: Set[int] = set()
-
-    def visit_Constant(self, node) -> None:
-        """
-        Does not produce values until Python 3.8. See
-        https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Constant
-        """
-        if isinstance(node.value, str):
-            self.add_footprint(node)
-
-    def visit_JoinedStr(self, node) -> None:
-        self.add_footprint(node)
-
-    def add_footprint(self, node) -> None:
-        self.footprints.update(build_footprint(node, self.first_line_no))
-
-
-if sys.version_info < (3, 8, 0):
-
-    def visit_Str(self, node) -> None:
-        self.add_footprint(node)
-
-    StringyLineVisitor.visit_Str = visit_Str  # type: ignore
-
-
-def find_stringy_lines(tree: ast.AST, first_line_no: int) -> Set[int]:
-    """
-    Finds all lines that contain a string in a tree, usually a function. These
-    lines will be ignored when searching for blank lines.
-
-    JoinedStr can contain Str nodes and FormattedValue nodes - the inner nodes
-    are not tokenised, so cause build_footprint() to raise AttributeErrors when
-    it attempts to inspect the tokens on these nodes.
-
-    See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#JoinedStr
-    """
-    str_visitor = StringyLineVisitor(first_line_no)
-    str_visitor.visit(tree)
-    return str_visitor.footprints
-
-
 def line_is_comment(line: str) -> bool:
     """
     Helper for checking that a single line is a comment. Will be replaced by a
     complete `find_comment_lines()` helper in #148. Could also use `tokens`
     from Flake8.
     """
+    # TODO use existing tokens
     try:
         first_token = next(tokenize.generate_tokens(io.StringIO(line).readline))
     except tokenize.TokenError:

tests/checker/conftest.py

@@ -0,0 +1,13 @@
+import ast
+
+import pytest
+
+from flake8_aaa import Checker
+
+
+@pytest.fixture
+def checker(tmpdir) -> Checker:
+    target_file = tmpdir.join('test.py')
+    target_file.write('assert 1 + 2 == 3\n')
+    tree = ast.parse(target_file.read())
+    return Checker(tree, ['assert 1 + 2 == 3\n'], target_file.strpath)

tests/checker/test_all_funcs.py

@@ -0,0 +1,22 @@
+from typing import Generator
+
+import pytest
+
+from flake8_aaa.exceptions import TokensNotLoaded
+
+
+def test(checker) -> None:
+    checker.load()
+
+    result = checker.all_funcs()
+
+    assert isinstance(result, Generator)
+    assert list(result) == []
+
+
+def test_not_loaded(checker) -> None:
+    result = checker.all_funcs()
+
+    assert isinstance(result, Generator)
+    with pytest.raises(TokensNotLoaded):
+        next(result)