From 5e5285f3dc7af751b797c89f67d6c6ca0fc91731 Mon Sep 17 00:00:00 2001 From: Dominic Davis-Foster Date: Mon, 24 Jan 2022 10:13:42 +0000 Subject: [PATCH] Add check for alphabetical order. --- doc-source/usage.rst | 15 +++ flake8_dunder_all/__init__.py | 117 +++++++++++++++++--- requirements.txt | 1 + tests/test_flake8_dunder_all.py | 182 ++++++++++++++++++++++++++++++-- 4 files changed, 294 insertions(+), 21 deletions(-) diff --git a/doc-source/usage.rst b/doc-source/usage.rst index 6f9c000..866473b 100644 --- a/doc-source/usage.rst +++ b/doc-source/usage.rst @@ -12,6 +12,21 @@ Flake8 codes .. flake8-codes:: flake8_dunder_all DALL000 + DALL001 + DALL002 + + +For the ``DALL001`` option there exists a configuration option (``dunder-all-alphabetical``) +which controls the alphabetical grouping expected of ``__all__``. +The options are: + +* ``ignore`` -- ``__all__`` should be sorted alphabetically ignoring case, e.g. ``['bar', 'Baz', 'foo']`` +* ``lower`` -- group lowercase names first, then uppercase names, e.g. ``['bar', 'foo', 'Baz']`` +* ``upper`` -- group uppercase names first, then uppercase names, e.g. ``['Baz', 'Foo', 'bar']`` + +If the ``dunder-all-alphabetical`` is omitted the ``DALL001`` check is disabled. + +.. versionchanged:: 0.2.0 Added the ``DALL001`` and ``DALL002`` checks. ``ensure-dunder-all`` script diff --git a/flake8_dunder_all/__init__.py b/flake8_dunder_all/__init__.py index 5d52b1e..8990f0f 100644 --- a/flake8_dunder_all/__init__.py +++ b/flake8_dunder_all/__init__.py @@ -32,13 +32,16 @@ # stdlib import ast import sys -from typing import Any, Generator, Set, Tuple, Type, Union +from enum import Enum +from typing import Any, Generator, Optional, Sequence, Set, Tuple, Type, Union, cast # 3rd party +import natsort from consolekit.terminal_colours import Fore from domdf_python_tools.paths import PathPlus from domdf_python_tools.typing import PathLike from domdf_python_tools.utils import stderr_writer +from flake8.options.manager import OptionManager # type: ignore from flake8.style_guide import find_noqa # type: ignore # this package @@ -50,9 +53,32 @@ __version__: str = "0.1.8" __email__: str = "dominic@davis-foster.co.uk" -__all__ = ["Visitor", "Plugin", "check_and_add_all", "DALL000"] +__all__ = [ + "check_and_add_all", + "AlphabeticalOptions", + "DALL000", + "DALL001", + "DALL002", + "Plugin", + "Visitor", + ] -DALL000 = "DALL000 Module lacks __all__." +DALL000 = "DALL000 Module lacks __all__" +DALL001 = "DALL001 __all__ not sorted alphabetically" +DALL002 = "DALL002 __all__ not a list of strings" + + +class AlphabeticalOptions(Enum): + """ + Enum of possible values for the ``--dunder-all-alphabetical`` option. + + .. versionadded:: 0.2.0 + """ + + UPPER = "upper" + LOWER = "lower" + IGNORE = "ignore" + NONE = "none" class Visitor(ast.NodeVisitor): @@ -62,30 +88,56 @@ class Visitor(ast.NodeVisitor): :param use_endlineno: Flag to indicate whether the end_lineno functionality is available. This functionality is available on Python 3.8 and above, or when the tree has been passed through :func:`flake8_dunder_all.utils.mark_text_ranges``. + + .. versionchanged:: 0.2.0 + + Added the ``sorted_upper_first``, ``sorted_lower_first`` and ``all_lineno`` attributes. """ found_all: bool #: Flag to indicate a ``__all__`` declaration has been found in the AST. last_import: int #: The lineno of the last top-level import members: Set[str] #: List of functions and classed defined in the AST use_endlineno: bool + all_members: Optional[Sequence[str]] #: The value of ``__all__``. + all_lineno: int #: The line number where ``__all__`` is defined. def __init__(self, use_endlineno: bool = False) -> None: self.found_all = False self.members = set() self.last_import = 0 self.use_endlineno = use_endlineno + self.all_members = None + self.all_lineno = -1 - def visit_Name(self, node: ast.Name): - """ - Visit a variable. - - :param node: The node being visited. - """ + def visit_Assign(self, node: ast.Assign) -> None: # noqa: D102 + targets = [] + for t in node.targets: + if isinstance(t, ast.Name): + targets.append(t.id) - if node.id == "__all__": + if "__all__" in targets: self.found_all = True - else: - self.generic_visit(node) + self.all_lineno = node.lineno + self.all_members = self._parse_all(cast(ast.List, node.value)) + + def visit_AnnAssign(self, node: ast.AnnAssign) -> None: # noqa: D102 + if isinstance(node.target, ast.Name): + if node.target.id == "__all__": + self.all_lineno = node.lineno + self.found_all = True + self.all_members = self._parse_all(cast(ast.List, node.value)) + + @staticmethod + def _parse_all(all_node: ast.List) -> Optional[Sequence[str]]: + try: + all_ = ast.literal_eval(all_node) + except ValueError: + return None + + if not isinstance(all_, Sequence): + return None + + return all_ def handle_def(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef]): """ @@ -193,6 +245,7 @@ class Plugin: name: str = __name__ version: str = __version__ #: The plugin version + dunder_all_alphabetical: AlphabeticalOptions = AlphabeticalOptions.NONE def __init__(self, tree: ast.AST): self._tree = tree @@ -213,12 +266,50 @@ def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: visitor.visit(self._tree) if visitor.found_all: - return + if visitor.all_members is None: + yield visitor.all_lineno, 0, DALL002, type(self) + + elif self.dunder_all_alphabetical == AlphabeticalOptions.IGNORE: + # Alphabetical, upper or lower don't matter + sorted_alphabetical = natsort.natsorted(visitor.all_members, key=str.lower) + if visitor.all_members != sorted_alphabetical: + yield visitor.all_lineno, 0, f"{DALL001}", type(self) + elif self.dunder_all_alphabetical == AlphabeticalOptions.UPPER: + # Alphabetical, uppercase grouped first + sorted_alphabetical = natsort.natsorted(visitor.all_members) + if visitor.all_members != sorted_alphabetical: + yield visitor.all_lineno, 0, f"{DALL001} (uppercase first)", type(self) + elif self.dunder_all_alphabetical == AlphabeticalOptions.LOWER: + # Alphabetical, lowercase grouped first + sorted_alphabetical = natsort.natsorted(visitor.all_members, alg=natsort.ns.LOWERCASEFIRST) + if visitor.all_members != sorted_alphabetical: + yield visitor.all_lineno, 0, f"{DALL001} (lowercase first)", type(self) + elif not visitor.members: return + else: yield 1, 0, DALL000, type(self) + @classmethod + def add_options(cls, option_manager: OptionManager) -> None: # noqa: D102 # pragma: no cover + + option_manager.add_option( + "--dunder-all-alphabetical", + choices=[member.value for member in AlphabeticalOptions], + parse_from_config=True, + default=AlphabeticalOptions.NONE.value, + help=( + "Require entries in '__all__' to be alphabetical ([upper] or [lower]case first)." + "(Default: %(default)s)" + ), + ) + + @classmethod + def parse_options(cls, options): # noqa: D102 # pragma: no cover + # note: this sets the option on the class and not the instance + cls.dunder_all_alphabetical = AlphabeticalOptions(options.dunder_all_alphabetical) + def check_and_add_all(filename: PathLike, quote_type: str = '"') -> int: """ diff --git a/requirements.txt b/requirements.txt index efa7811..fb3f768 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,4 @@ click>=7.1.2 consolekit>=0.8.1 domdf-python-tools>=2.6.0 flake8>=3.7 +natsort>=8.0.2 diff --git a/tests/test_flake8_dunder_all.py b/tests/test_flake8_dunder_all.py index 6e64a15..fa3ef29 100644 --- a/tests/test_flake8_dunder_all.py +++ b/tests/test_flake8_dunder_all.py @@ -10,7 +10,7 @@ from domdf_python_tools.paths import PathPlus # this package -from flake8_dunder_all import Visitor, check_and_add_all +from flake8_dunder_all import AlphabeticalOptions, Plugin, Visitor, check_and_add_all from flake8_dunder_all.utils import mark_text_ranges from tests.common import ( mangled_source, @@ -36,26 +36,192 @@ pytest.param("import foo", set(), id="just an import"), pytest.param('"""a docstring"""', set(), id="just a docstring"), pytest.param(testing_source_a, set(), id="import and docstring"), - pytest.param(testing_source_b, {"1:0: DALL000 Module lacks __all__."}, id="function no __all__"), - pytest.param(testing_source_c, {"1:0: DALL000 Module lacks __all__."}, id="class no __all__"), + pytest.param(testing_source_b, {"1:0: DALL000 Module lacks __all__"}, id="function no __all__"), + pytest.param(testing_source_c, {"1:0: DALL000 Module lacks __all__"}, id="class no __all__"), pytest.param( - testing_source_d, {"1:0: DALL000 Module lacks __all__."}, - id="function and class no __all__" + testing_source_d, + {"1:0: DALL000 Module lacks __all__"}, + id="function and class no __all__", ), pytest.param(testing_source_e, set(), id="function and class with __all__"), pytest.param(testing_source_f, set(), id="function and class with __all__ and extra variable"), pytest.param( - testing_source_g, {"1:0: DALL000 Module lacks __all__."}, id="async function no __all__" + testing_source_g, + {"1:0: DALL000 Module lacks __all__"}, + id="async function no __all__", ), pytest.param(testing_source_h, set(), id="from import"), - pytest.param(testing_source_i, {"1:0: DALL000 Module lacks __all__."}, id="lots of lines"), - pytest.param(testing_source_j, {"1:0: DALL000 Module lacks __all__."}, id="multiline import"), + pytest.param(testing_source_i, {"1:0: DALL000 Module lacks __all__"}, id="lots of lines"), + pytest.param(testing_source_j, {"1:0: DALL000 Module lacks __all__"}, id="multiline import"), ] ) def test_plugin(source: str, expects: Set[str]): assert results(source) == expects +@pytest.mark.parametrize( + "source, dunder_all_alphabetical, expects", + [ + pytest.param( + "__all__ = ['foo', 'bar', 'Baz']", + AlphabeticalOptions.NONE, + set(), + id="NONE_wrong_order", + ), + pytest.param( + "__all__ = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.NONE, + set(), + id="NONE_right_order_1", + ), + pytest.param( + "__all__ = ['Bar', 'baz', 'foo']", + AlphabeticalOptions.NONE, + set(), + id="NONE_right_order_2", + ), + pytest.param( + "__all__ = ['foo', 'bar', 'Baz']", + AlphabeticalOptions.IGNORE, + {"1:0: DALL001 __all__ not sorted alphabetically"}, + id="IGNORE_wrong_order", + ), + pytest.param( + "__all__ = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.IGNORE, + set(), + id="IGNORE_right_order", + ), + pytest.param( + "__all__ = ['Baz', 'bar', 'foo']", + AlphabeticalOptions.LOWER, + {"1:0: DALL001 __all__ not sorted alphabetically (lowercase first)"}, + id="LOWER_wrong_order", + ), + pytest.param( + "__all__ = ['bar', 'foo', 'Baz']", + AlphabeticalOptions.LOWER, + set(), + id="LOWER_right_order", + ), + pytest.param( + "__all__ = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.UPPER, + {"1:0: DALL001 __all__ not sorted alphabetically (uppercase first)"}, + id="UPPER_wrong_order", + ), + pytest.param( + "__all__ = ['Baz', 'bar', 'foo']", + AlphabeticalOptions.UPPER, + set(), + id="UPPER_right_order_1", + ), + pytest.param( + "__all__ = ['Baz', 'Foo', 'bar']", + AlphabeticalOptions.UPPER, + set(), + id="UPPER_right_order_2", + ), + ] + ) +def test_plugin_alphabetical(source: str, expects: Set[str], dunder_all_alphabetical: AlphabeticalOptions): + plugin = Plugin(ast.parse(source)) + plugin.dunder_all_alphabetical = dunder_all_alphabetical + assert {"{}:{}: {}".format(*r) for r in plugin.run()} == expects + + +@pytest.mark.parametrize( + "source, dunder_all_alphabetical, expects", + [ + pytest.param( + "__all__: List[str] = ['foo', 'bar', 'Baz']", + AlphabeticalOptions.NONE, + set(), + id="NONE_wrong_order", + ), + pytest.param( + "__all__: List[str] = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.NONE, + set(), + id="NONE_right_order_1", + ), + pytest.param( + "__all__: List[str] = ['Bar', 'baz', 'foo']", + AlphabeticalOptions.NONE, + set(), + id="NONE_right_order_1", + ), + pytest.param( + "__all__: List[str] = ['foo', 'bar', 'Baz']", + AlphabeticalOptions.IGNORE, + {"1:0: DALL001 __all__ not sorted alphabetically"}, + id="IGNORE_wrong_order", + ), + pytest.param( + "__all__: List[str] = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.IGNORE, + set(), + id="IGNORE_right_order", + ), + pytest.param( + "__all__: List[str] = ['Baz', 'bar', 'foo']", + AlphabeticalOptions.LOWER, + {"1:0: DALL001 __all__ not sorted alphabetically (lowercase first)"}, + id="LOWER_wrong_order", + ), + pytest.param( + "__all__: List[str] = ['bar', 'foo', 'Baz']", + AlphabeticalOptions.LOWER, + set(), + id="LOWER_right_order", + ), + pytest.param( + "__all__: List[str] = ['bar', 'Baz', 'foo']", + AlphabeticalOptions.UPPER, + {"1:0: DALL001 __all__ not sorted alphabetically (uppercase first)"}, + id="UPPER_wrong_order", + ), + pytest.param( + "__all__: List[str] = ['Baz', 'bar', 'foo']", + AlphabeticalOptions.UPPER, + set(), + id="UPPER_right_order_1", + ), + pytest.param( + "__all__: List[str] = ['Baz', 'Foo', 'bar']", + AlphabeticalOptions.UPPER, + set(), + id="UPPER_right_order_2", + ), + ] + ) +def test_plugin_alphabetical_ann_assign( + source: str, expects: Set[str], dunder_all_alphabetical: AlphabeticalOptions + ): + plugin = Plugin(ast.parse(source)) + plugin.dunder_all_alphabetical = dunder_all_alphabetical + assert {"{}:{}: {}".format(*r) for r in plugin.run()} == expects + + +@pytest.mark.parametrize( + "source, dunder_all_alphabetical", + [ + pytest.param("__all__ = 12345", AlphabeticalOptions.IGNORE, id="IGNORE_123"), + pytest.param("__all__ = 12345", AlphabeticalOptions.LOWER, id="LOWER_123"), + pytest.param("__all__ = 12345", AlphabeticalOptions.NONE, id="NONE_123"), + pytest.param("__all__ = 12345", AlphabeticalOptions.UPPER, id="UPPER_123"), + pytest.param("__all__ = abc", AlphabeticalOptions.IGNORE, id="IGNORE_abc"), + pytest.param("__all__ = abc", AlphabeticalOptions.LOWER, id="LOWER_abc"), + pytest.param("__all__ = abc", AlphabeticalOptions.NONE, id="NONE_abc"), + pytest.param("__all__ = abc", AlphabeticalOptions.UPPER, id="UPPER_abc"), + ] + ) +def test_plugin_alphabetical_not_list(source: str, dunder_all_alphabetical: AlphabeticalOptions): + plugin = Plugin(ast.parse(source)) + plugin.dunder_all_alphabetical = dunder_all_alphabetical + assert {"{}:{}: {}".format(*r) for r in plugin.run()} == {"1:0: DALL002 __all__ not a list of strings"} + + @pytest.mark.parametrize( "source, members, found_all, last_import", [