feat: add DALL1xx for __dir__ (#92)

Co-authored-by: Dominic Davis-Foster <dominic@davis-foster.co.uk>

Author: henryiii

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

doc-source/usage.rst

@@ -14,6 +14,8 @@ Flake8 codes
 	DALL000
 	DALL001
 	DALL002
+	DALL100
+	DALL101
 
 
 For the ``DALL001`` option there exists a configuration option (``dunder-all-alphabetical``)
@@ -27,6 +29,7 @@ The options are:
 If the ``dunder-all-alphabetical`` option is omitted the ``DALL001`` check is disabled.
 
 .. versionchanged:: 0.5.0  Added the ``DALL001`` and ``DALL002`` checks.
+.. versionchanged:: 0.6.0  Added the ``DALL100`` and ``DALL101`` checks.
 
 .. note::
 

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):
@@ -95,9 +97,12 @@ class Visitor(ast.NodeVisitor):
 	.. versionchanged:: 0.5.0
 
 		Added the ``sorted_upper_first``, ``sorted_lower_first`` and ``all_lineno`` attributes.
+
+	.. versionchanged:: 0.6.0  Added the ``found_dir`` attribute.
 	"""
 
 	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
@@ -106,6 +111,7 @@ class Visitor(ast.NodeVisitor):
 
 	def __init__(self, use_endlineno: bool = False) -> None:
 		self.found_all = False
+		self.found_dir = False
 		self.members = set()
 		self.last_import = 0
 		self.use_endlineno = use_endlineno
@@ -187,6 +193,9 @@ def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
 		# Don't generic visit
 		self.handle_def(node)
 
+		if node.name == "__dir__":
+			self.found_dir = True
+
 	def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None:
 		"""
 		Visit ``async def foo(): ...``.
@@ -306,14 +315,18 @@ 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.
+
+	.. versionchanged:: 0.6.0  Added ``filename`` argument.
 	"""
 
 	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]:
 		"""
@@ -350,12 +363,16 @@ 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:
-			return
-
-		else:
+		elif visitor.members:
 			yield 1, 0, DALL000, type(self)
 
+		# 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"):
+				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
 

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 r[2].startswith("DALL0")}
 
 
 testing_source_a = '''

tests/test_dir_required.py

@@ -0,0 +1,97 @@
+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
+
+	class 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 = """
+	class Foo: ...
+
+	def __dir__():
+		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"""
+	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 = """
+	class Foo: ...
+
+	def __dir__():
+		return []
+	"""
+	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
+
+	class 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 Foo: ...
+
+	class __dir__: ...
+	"""
+	results = from_source(source, "module.py")
+	assert any("DALL100" in r[2] for r in results)

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 r[2].startswith("DALL0")} == 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 r[2].startswith("DALL0")} == 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 r[2].startswith("DALL0")} == {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 r[2].startswith("DALL0")} == set()
 
 
 @pytest.mark.parametrize(

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("  # noq" + "a: DALL000,DALL100  \n\n\t\ndef foo():\n\tpass\n\t")
 
 	with in_directory(tmp_pathplus):
 		result = subprocess.run(