From 6886446eb3e59d0abf18630e61d2f226bb69dcce Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 5 Jan 2026 15:20:21 -0500 Subject: [PATCH 1/3] feat: add DALL1xx for __dir__ Signed-off-by: Henry Schreiner --- flake8_dunder_all/__init__.py | 22 ++++++++++++-- tests/common.py | 2 +- tests/test_dir_required.py | 53 +++++++++++++++++++++++++++++++++ tests/test_flake8_dunder_all.py | 16 +++++----- tests/test_subprocess.py | 2 +- 5 files changed, 83 insertions(+), 12 deletions(-) create mode 100644 tests/test_dir_required.py diff --git a/flake8_dunder_all/__init__.py b/flake8_dunder_all/__init__.py index dc2dc89..8a8cd4b 100644 --- a/flake8_dunder_all/__init__.py +++ b/flake8_dunder_all/__init__.py @@ -69,6 +69,8 @@ DALL000 = "DALL000 Module lacks __all__." DALL001 = "DALL001 __all__ not sorted alphabetically" DALL002 = "DALL002 __all__ not a list or tuple of strings." +DALL100 = "DALL100 Top-level __dir__ function definition is required." +DALL101 = "DALL101 Top-level __dir__ function definition is required in __init__.py." class AlphabeticalOptions(Enum): @@ -106,6 +108,8 @@ class Visitor(ast.NodeVisitor): def __init__(self, use_endlineno: bool = False) -> None: self.found_all = False + self.found_lineno = -1 + self.found_dir = False self.members = set() self.last_import = 0 self.use_endlineno = use_endlineno @@ -177,6 +181,10 @@ def handle_def(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef, ast.Clas if not node.name.startswith('_') and "overload" not in decorators: self.members.add(node.name) + if node.name == "__dir__": + self.found_dir = True + self.found_lineno = node.lineno + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: """ Visit ``def foo(): ...``. @@ -306,14 +314,16 @@ class Plugin: A Flake8 plugin which checks to ensure modules have defined ``__all__``. :param tree: The abstract syntax tree (AST) to check. + :param filename: The filename being checked. """ name: str = __name__ version: str = __version__ #: The plugin version dunder_all_alphabetical: AlphabeticalOptions = AlphabeticalOptions.NONE - def __init__(self, tree: ast.AST): + def __init__(self, tree: ast.AST, filename: str): self._tree = tree + self._filename = filename def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: """ @@ -351,11 +361,19 @@ def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: yield visitor.all_lineno, 0, f"{DALL001} (lowercase first).", type(self) elif not visitor.members: - return + pass else: yield 1, 0, DALL000, type(self) + # Require top-level __dir__ function + if not visitor.found_dir: + if self._filename.endswith("__init__.py"): + if visitor.members: + yield 1, 0, DALL101, type(self) + else: + yield 1, 0, DALL100, type(self) + @classmethod def add_options(cls, option_manager: OptionManager) -> None: # noqa: D102 # pragma: no cover diff --git a/tests/common.py b/tests/common.py index 7bc1a81..8585875 100644 --- a/tests/common.py +++ b/tests/common.py @@ -7,7 +7,7 @@ def results(s: str) -> Set[str]: - return {"{}:{}: {}".format(*r) for r in Plugin(ast.parse(s)).run()} + return {"{}:{}: {}".format(*r) for r in Plugin(ast.parse(s), "mod.py").run() if "DALL0" in r[2]} testing_source_a = ''' diff --git a/tests/test_dir_required.py b/tests/test_dir_required.py new file mode 100644 index 0000000..5177780 --- /dev/null +++ b/tests/test_dir_required.py @@ -0,0 +1,53 @@ +from __future__ import annotations + +# stdlib +import ast +import inspect +from typing import Any + +# this package +from flake8_dunder_all import Plugin + + +def from_source(source: str, filename: str) -> list[tuple[int, int, str, type[Any]]]: + source_clean = inspect.cleandoc(source) + plugin = Plugin(ast.parse(source_clean), filename) + return list(plugin.run()) + + +def test_dir_required_non_init(): + source = """ + import foo + """ + results = from_source(source, "module.py") + assert any("DALL100" in r[2] for r in results) + + +def test_dir_required_non_init_with_dir(): + # __dir__ defined, should not yield DALL100 + source_with_dir = """ + def __dir__(): + return []\n""" + results = from_source(source_with_dir, "module.py") + assert not any("DALL100" in r[2] for r in results) + + +def test_dir_required_empty(): + source = """\nimport foo\n""" + # No __dir__ defined but no members present, should not yield DALL101 + results = from_source(source, "__init__.py") + assert not any("DALL101" in r[2] for r in results) + + +def test_dir_required_init(): + source = """\nimport foo\n\nclass Foo: ...\n""" + # No __dir__ defined, should yield DALL101 + results = from_source(source, "__init__.py") + assert any("DALL101" in r[2] for r in results) + + +def test_dir_required_init_with_dir(): + # __dir__ defined, should not yield DALL101 + source_with_dir = """\ndef __dir__():\n return []\n""" + results = from_source(source_with_dir, "__init__.py") + assert not any("DALL101" in r[2] for r in results) diff --git a/tests/test_flake8_dunder_all.py b/tests/test_flake8_dunder_all.py index e2ec149..1b0c560 100644 --- a/tests/test_flake8_dunder_all.py +++ b/tests/test_flake8_dunder_all.py @@ -135,9 +135,9 @@ def test_plugin(source: str, expects: Set[str]): ], ) def test_plugin_alphabetical(source: str, expects: Set[str], dunder_all_alphabetical: AlphabeticalOptions): - plugin = Plugin(ast.parse(source)) + plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical - assert {"{}:{}: {}".format(*r) for r in plugin.run()} == expects + assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == expects @pytest.mark.parametrize( @@ -210,9 +210,9 @@ def test_plugin_alphabetical_ann_assign( expects: Set[str], dunder_all_alphabetical: AlphabeticalOptions, ): - plugin = Plugin(ast.parse(source)) + plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical - assert {"{}:{}: {}".format(*r) for r in plugin.run()} == expects + assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == expects @pytest.mark.parametrize( @@ -229,16 +229,16 @@ def test_plugin_alphabetical_ann_assign( ], ) def test_plugin_alphabetical_not_list(source: str, dunder_all_alphabetical: AlphabeticalOptions): - plugin = Plugin(ast.parse(source)) + plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical msg = "1:0: DALL002 __all__ not a list or tuple of strings." - assert {"{}:{}: {}".format(*r) for r in plugin.run()} == {msg} + assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == {msg} def test_plugin_alphabetical_tuple(): - plugin = Plugin(ast.parse("__all__ = ('bar',\n'foo')")) + plugin = Plugin(ast.parse("__all__ = ('bar',\n'foo')"), "mod.py") plugin.dunder_all_alphabetical = AlphabeticalOptions.IGNORE - assert {"{}:{}: {}".format(*r) for r in plugin.run()} == set() + assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == set() @pytest.mark.parametrize( diff --git a/tests/test_subprocess.py b/tests/test_subprocess.py index bd01fed..a699e07 100644 --- a/tests/test_subprocess.py +++ b/tests/test_subprocess.py @@ -66,7 +66,7 @@ def test_subprocess_noqa(tmp_pathplus: PathPlus, monkeypatch): monkeypatch.delenv("COV_CORE_DATAFILE", raising=False) monkeypatch.setenv("PYTHONWARNINGS", "ignore") - (tmp_pathplus / "demo.py").write_text("# noq" + "a: DALL000\n\n\t\ndef foo():\n\tpass\n\t") + (tmp_pathplus / "demo.py").write_text(" # noqa: DALL000,DALL100 \n\n\t\ndef foo():\n\tpass\n\t") with in_directory(tmp_pathplus): result = subprocess.run( From aa3e4fa19564cbb4b47e6fd339fbdb8c00e86247 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 22 Jun 2026 11:46:12 -0400 Subject: [PATCH 2/3] fix: better scoping of __dir__ check Noticed by a GPT-5.5 review. Assisted-by: ClaudeCode:claude-opus-4.8 Signed-off-by: Henry Schreiner --- README.rst | 8 +++++--- flake8_dunder_all/__init__.py | 8 ++++---- tests/test_dir_required.py | 23 +++++++++++++++++++++++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/README.rst b/README.rst index 632c49c..59ffd69 100644 --- a/README.rst +++ b/README.rst @@ -153,13 +153,15 @@ To install with ``conda``: flake8 codes -------------- -============== ======================================= +============== =============================================================== Code Description -============== ======================================= +============== =============================================================== DALL000 Module lacks __all__. DALL001 __all__ not sorted alphabetically DALL002 __all__ not a list or tuple of strings. -============== ======================================= +DALL100 Top-level __dir__ function definition is required. +DALL101 Top-level __dir__ function definition is required in __init__.py. +============== =============================================================== Use as a pre-commit hook diff --git a/flake8_dunder_all/__init__.py b/flake8_dunder_all/__init__.py index 8a8cd4b..7af0c40 100644 --- a/flake8_dunder_all/__init__.py +++ b/flake8_dunder_all/__init__.py @@ -181,10 +181,6 @@ def handle_def(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef, ast.Clas if not node.name.startswith('_') and "overload" not in decorators: self.members.add(node.name) - if node.name == "__dir__": - self.found_dir = True - self.found_lineno = node.lineno - def visit_FunctionDef(self, node: ast.FunctionDef) -> None: """ Visit ``def foo(): ...``. @@ -195,6 +191,10 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: # Don't generic visit self.handle_def(node) + if node.name == "__dir__": + self.found_dir = True + self.found_lineno = node.lineno + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: """ Visit ``async def foo(): ...``. diff --git a/tests/test_dir_required.py b/tests/test_dir_required.py index 5177780..ee7e46a 100644 --- a/tests/test_dir_required.py +++ b/tests/test_dir_required.py @@ -51,3 +51,26 @@ def test_dir_required_init_with_dir(): source_with_dir = """\ndef __dir__():\n return []\n""" results = from_source(source_with_dir, "__init__.py") assert not any("DALL101" in r[2] for r in results) + + +def test_dir_required_async_def_does_not_satisfy(): + # ``async def __dir__`` can't be used by ``dir(module)``, so DALL100 still applies + source = """ + import foo + + async def __dir__(): + return [] + """ + results = from_source(source, "module.py") + assert any("DALL100" in r[2] for r in results) + + +def test_dir_required_class_does_not_satisfy(): + # ``class __dir__`` can't be used by ``dir(module)``, so DALL100 still applies + source = """ + import foo + + class __dir__: ... + """ + results = from_source(source, "module.py") + assert any("DALL100" in r[2] for r in results) From f04fb353c81f4993bb6b50cb9f8c37db59233e88 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 22 Jun 2026 12:19:49 -0400 Subject: [PATCH 3/3] fix: only require __dir__ when module has public members Address review feedback on the DALL1xx __dir__ checks: - DALL100 no longer fires on memberless modules; the __dir__ requirement now mirrors DALL101 and applies only when public members exist. - Drop the unused Visitor.found_lineno and add the missing found_dir attribute annotation. - Simplify the DALL000 branch and tighten the test code filter (startswith instead of substring); restore the deliberate noqa split in test_subprocess. Assisted-by: ClaudeCode:claude-opus-4.8 Signed-off-by: Henry Schreiner --- flake8_dunder_all/__init__.py | 15 +++++---------- tests/common.py | 2 +- tests/test_dir_required.py | 27 ++++++++++++++++++++++++--- tests/test_flake8_dunder_all.py | 8 ++++---- tests/test_subprocess.py | 2 +- 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/flake8_dunder_all/__init__.py b/flake8_dunder_all/__init__.py index 7af0c40..5457214 100644 --- a/flake8_dunder_all/__init__.py +++ b/flake8_dunder_all/__init__.py @@ -100,6 +100,7 @@ class Visitor(ast.NodeVisitor): """ found_all: bool #: Flag to indicate a ``__all__`` declaration has been found in the AST. + found_dir: bool #: Flag to indicate a top-level ``__dir__`` function has been found in the AST. last_import: int #: The lineno of the last top-level or conditional import members: Set[str] #: List of functions and classed defined in the AST use_endlineno: bool @@ -108,7 +109,6 @@ class Visitor(ast.NodeVisitor): def __init__(self, use_endlineno: bool = False) -> None: self.found_all = False - self.found_lineno = -1 self.found_dir = False self.members = set() self.last_import = 0 @@ -193,7 +193,6 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None: if node.name == "__dir__": self.found_dir = True - self.found_lineno = node.lineno def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: """ @@ -360,17 +359,13 @@ def run(self) -> Generator[Tuple[int, int, str, Type[Any]], None, None]: if list(visitor.all_members) != sorted_alphabetical: yield visitor.all_lineno, 0, f"{DALL001} (lowercase first).", type(self) - elif not visitor.members: - pass - - else: + elif visitor.members: yield 1, 0, DALL000, type(self) - # Require top-level __dir__ function - if not visitor.found_dir: + # Require a top-level __dir__, but only when the module defines public members + if visitor.members and not visitor.found_dir: if self._filename.endswith("__init__.py"): - if visitor.members: - yield 1, 0, DALL101, type(self) + yield 1, 0, DALL101, type(self) else: yield 1, 0, DALL100, type(self) diff --git a/tests/common.py b/tests/common.py index 8585875..dcc0784 100644 --- a/tests/common.py +++ b/tests/common.py @@ -7,7 +7,7 @@ def results(s: str) -> Set[str]: - return {"{}:{}: {}".format(*r) for r in Plugin(ast.parse(s), "mod.py").run() if "DALL0" in r[2]} + return {"{}:{}: {}".format(*r) for r in Plugin(ast.parse(s), "mod.py").run() if r[2].startswith("DALL0")} testing_source_a = ''' diff --git a/tests/test_dir_required.py b/tests/test_dir_required.py index ee7e46a..8d3665e 100644 --- a/tests/test_dir_required.py +++ b/tests/test_dir_required.py @@ -18,6 +18,8 @@ def from_source(source: str, filename: str) -> list[tuple[int, int, str, type[An def test_dir_required_non_init(): source = """ import foo + + class Foo: ... """ results = from_source(source, "module.py") assert any("DALL100" in r[2] for r in results) @@ -26,15 +28,25 @@ def test_dir_required_non_init(): def test_dir_required_non_init_with_dir(): # __dir__ defined, should not yield DALL100 source_with_dir = """ + class Foo: ... + def __dir__(): - return []\n""" + return [] + """ results = from_source(source_with_dir, "module.py") assert not any("DALL100" in r[2] for r in results) def test_dir_required_empty(): + # No public members, so __dir__ is not required + source = """\nimport foo\n""" + results = from_source(source, "module.py") + assert not any("DALL100" in r[2] for r in results) + + +def test_dir_required_empty_init(): + # No public members, so __dir__ is not required in __init__.py either source = """\nimport foo\n""" - # No __dir__ defined but no members present, should not yield DALL101 results = from_source(source, "__init__.py") assert not any("DALL101" in r[2] for r in results) @@ -48,7 +60,12 @@ def test_dir_required_init(): def test_dir_required_init_with_dir(): # __dir__ defined, should not yield DALL101 - source_with_dir = """\ndef __dir__():\n return []\n""" + source_with_dir = """ + class Foo: ... + + def __dir__(): + return [] + """ results = from_source(source_with_dir, "__init__.py") assert not any("DALL101" in r[2] for r in results) @@ -58,6 +75,8 @@ def test_dir_required_async_def_does_not_satisfy(): source = """ import foo + class Foo: ... + async def __dir__(): return [] """ @@ -70,6 +89,8 @@ def test_dir_required_class_does_not_satisfy(): source = """ import foo + class Foo: ... + class __dir__: ... """ results = from_source(source, "module.py") diff --git a/tests/test_flake8_dunder_all.py b/tests/test_flake8_dunder_all.py index 1b0c560..8b378b1 100644 --- a/tests/test_flake8_dunder_all.py +++ b/tests/test_flake8_dunder_all.py @@ -137,7 +137,7 @@ def test_plugin(source: str, expects: Set[str]): def test_plugin_alphabetical(source: str, expects: Set[str], dunder_all_alphabetical: AlphabeticalOptions): plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical - assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == expects + assert {"{}:{}: {}".format(*r) for r in plugin.run() if r[2].startswith("DALL0")} == expects @pytest.mark.parametrize( @@ -212,7 +212,7 @@ def test_plugin_alphabetical_ann_assign( ): plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical - assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == expects + assert {"{}:{}: {}".format(*r) for r in plugin.run() if r[2].startswith("DALL0")} == expects @pytest.mark.parametrize( @@ -232,13 +232,13 @@ def test_plugin_alphabetical_not_list(source: str, dunder_all_alphabetical: Alph plugin = Plugin(ast.parse(source), "mod.py") plugin.dunder_all_alphabetical = dunder_all_alphabetical msg = "1:0: DALL002 __all__ not a list or tuple of strings." - assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == {msg} + assert {"{}:{}: {}".format(*r) for r in plugin.run() if r[2].startswith("DALL0")} == {msg} def test_plugin_alphabetical_tuple(): plugin = Plugin(ast.parse("__all__ = ('bar',\n'foo')"), "mod.py") plugin.dunder_all_alphabetical = AlphabeticalOptions.IGNORE - assert {"{}:{}: {}".format(*r) for r in plugin.run() if "DALL0" in r[2]} == set() + assert {"{}:{}: {}".format(*r) for r in plugin.run() if r[2].startswith("DALL0")} == set() @pytest.mark.parametrize( diff --git a/tests/test_subprocess.py b/tests/test_subprocess.py index a699e07..99c3d13 100644 --- a/tests/test_subprocess.py +++ b/tests/test_subprocess.py @@ -66,7 +66,7 @@ def test_subprocess_noqa(tmp_pathplus: PathPlus, monkeypatch): monkeypatch.delenv("COV_CORE_DATAFILE", raising=False) monkeypatch.setenv("PYTHONWARNINGS", "ignore") - (tmp_pathplus / "demo.py").write_text(" # noqa: DALL000,DALL100 \n\n\t\ndef foo():\n\tpass\n\t") + (tmp_pathplus / "demo.py").write_text(" # noq" + "a: DALL000,DALL100 \n\n\t\ndef foo():\n\tpass\n\t") with in_directory(tmp_pathplus): result = subprocess.run(