Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .flake8
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ extend-exclude = .venv,build,dist

[flake8:local-plugins]
extension =
RMP = flake8_rampart:RampartChecker
RMP001 = flake8_rampart:AsyncSuffixChecker
RMP002 = flake8_rampart:LazyExportChecker
paths =
./tools
11 changes: 11 additions & 0 deletions .github/instructions/coding-standards.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ Before committing code, ensure:
| Convention | Code | Enforced by |
|---|---|---|
| Async `_async` suffix | `RMP001` | `tools/flake8_rampart.py`, a flake8 local plugin |
| Lazy exports listed in `__all__` | `RMP002` | `tools/flake8_rampart.py`, a flake8 local plugin |

### Async naming (`RMP001`)

Expand All @@ -625,6 +626,16 @@ that ruff's `RUF102` accepts it instead of rejecting it as an unknown code.

`RMP001` applies repo-wide, including to tests: the test standards require the
`_async` suffix on async test names too.

### Lazy public exports (`RMP002`)

Modules that declare a literal `__lazy_imports__` dictionary MUST include every
literal key in their literal `__all__` collection. Eager public exports may also
appear in `__all__`; the lazy names are a subset, not an exhaustive public API.
Both declarations MUST use literal forms: `__lazy_imports__` requires a dictionary
with string keys, and `__all__` requires a list, tuple, or set containing only
strings. Each name MUST have exactly one direct module-level assignment. Additional
writes and direct method calls are rejected; reads remain allowed.
[flake8-local]: https://flake8.pycqa.org/en/latest/user/configuration.html#using-local-plugins

---
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ ignore = [

# RMP comes from the flake8 local plugin in tools/. Declaring it here stops
# RUF102 (invalid-rule-code) from rejecting `# noqa: RMPXXX` as unknown.
external = ["RMP001"]
external = ["RMP001", "RMP002"]

[tool.ruff.lint.per-file-ignores]
"scripts/hatch_build.py" = [
Expand Down
244 changes: 227 additions & 17 deletions tests/unit/tools/test_flake8_rampart.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Copyright (c) Microsoft Corporation.
# Licensed under the MIT license.

"""Tests for the flake8-rampart local plugin (RMP001)."""
"""Tests for the flake8-rampart local plugin."""

from __future__ import annotations

Expand All @@ -11,45 +11,201 @@
from pathlib import Path

import pytest
from flake8_rampart import RampartChecker
from flake8_rampart import AsyncSuffixChecker, LazyExportChecker

_REPO_ROOT = Path(__file__).resolve().parents[3]


def _messages(source: str) -> list[str]:
"""Return the messages the checker reports for a source snippet."""
return [msg for _, _, msg, _ in RampartChecker(ast.parse(source)).run()]
def _async_suffix_messages(source: str) -> list[str]:
"""Return async-suffix messages for a source snippet."""
return [msg for _, _, msg, _ in AsyncSuffixChecker(ast.parse(source)).run()]


def _lazy_export_messages(source: str) -> list[str]:
"""Return lazy-export messages for a source snippet."""
return [msg for _, _, msg, _ in LazyExportChecker(ast.parse(source)).run()]


class TestAsyncSuffixRule:
def test_flags_async_function_without_suffix(self) -> None:
(message,) = _messages("async def fetch(): ...")
(message,) = _async_suffix_messages("async def fetch(): ...")
assert message.startswith("RMP001")
assert "`fetch`" in message

def test_accepts_async_function_with_suffix(self) -> None:
assert _messages("async def fetch_async(): ...") == []
assert _async_suffix_messages("async def fetch_async(): ...") == []

def test_ignores_sync_function(self) -> None:
assert _messages("def fetch(): ...") == []
assert _async_suffix_messages("def fetch(): ...") == []

def test_exempts_dunder(self) -> None:
assert _messages("async def __aenter__(self): ...") == []
assert _async_suffix_messages("async def __aenter__(self): ...") == []

def test_flags_method_inside_class(self) -> None:
source = "class A:\n async def fetch(self): ...\n"
assert len(_messages(source)) == 1
assert len(_async_suffix_messages(source)) == 1

def test_flags_nested_function(self) -> None:
source = "def outer():\n async def inner(): ...\n"
assert len(_messages(source)) == 1
assert len(_async_suffix_messages(source)) == 1

def test_reports_position_of_definition(self) -> None:
checker = RampartChecker(ast.parse("\n\nasync def fetch(): ..."))
checker = AsyncSuffixChecker(ast.parse("\n\nasync def fetch(): ..."))
((line, col, _, _),) = checker.run()
assert (line, col) == (3, 0)


class TestLazyExportRule:
def test_flags_lazy_export_missing_from_all(self) -> None:
source = """
__lazy_imports__: dict[str, tuple[str, str]] = {
"Heavy": ("package.heavy", "Heavy"),
}
__all__ = []
"""

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")
assert "`Heavy`" in message

def test_accepts_lazy_export_in_all(self) -> None:
Comment thread
spencrr marked this conversation as resolved.
source = """
__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}
__all__ = ["Heavy"]
"""

assert _lazy_export_messages(source) == []

def test_allows_eager_exports_in_all(self) -> None:
source = """
__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}
__all__ = ["Eager", "Heavy"]
"""

assert _lazy_export_messages(source) == []

def test_reports_only_lazy_exports_missing_from_all(self) -> None:
source = """
__lazy_imports__ = {
"Included": ("package.included", "Included"),
"MissingOne": ("package.missing", "MissingOne"),
"MissingTwo": ("package.missing", "MissingTwo"),
}
__all__ = ["Included"]
"""

messages = _lazy_export_messages(source)

assert len(messages) == 2
assert "`MissingOne`" in messages[0]
assert "`MissingTwo`" in messages[1]

def test_flags_lazy_export_when_all_is_absent(self) -> None:
source = '__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}'

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")

def test_rejects_dynamic_lazy_registry(self) -> None:
source = """
__lazy_imports__ = build_lazy_imports()
__all__ = []
"""

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")
assert "`__lazy_imports__`" in message

def test_rejects_dynamic_all(self) -> None:
source = """
__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}
__all__ = build_public_names()
"""

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")
assert "`__all__`" in message

@pytest.mark.parametrize(
("mutation", "expected_name"),
[
('__all__ += ["Heavy"]', "__all__"),
('__all__.extend(["Heavy"])', "__all__"),
(
'__lazy_imports__["Other"] = ("package.other", "Other")',
"__lazy_imports__",
),
],
)
def test_rejects_incremental_construction(
self,
*,
mutation: str,
expected_name: str,
) -> None:
source = f"""
__lazy_imports__ = {{"Heavy": ("package.heavy", "Heavy")}}
__all__ = ["Heavy"]
{mutation}
"""

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")
assert f"`{expected_name}`" in message

@pytest.mark.parametrize("name", ["__all__", "__lazy_imports__"])
def test_rejects_multiple_assignments(self, *, name: str) -> None:
replacement = "{}" if name == "__lazy_imports__" else "[]"
source = f"""
__lazy_imports__ = {{"Heavy": ("package.heavy", "Heavy")}}
__all__ = ["Heavy"]
{name} = {replacement}
"""

(message,) = _lazy_export_messages(source)

assert message.startswith("RMP002")
assert f"`{name}`" in message

def test_allows_reads(self) -> None:
source = """
__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}
__all__ = ["Heavy"]

observed_exports = __all__
observed_registry = __lazy_imports__

def resolve(name):
return __lazy_imports__[name]

def public_names():
return __all__

class Namespace:
public = __all__
"""

assert _lazy_export_messages(source) == []

def test_reports_position_of_missing_key(self) -> None:
source = """
__lazy_imports__ = {
"Heavy": ("package.heavy", "Heavy"),
}
__all__ = []
"""
checker = LazyExportChecker(ast.parse(source))

((line, col, _, _),) = checker.run()

assert (line, col) == (3, 4)


@pytest.mark.slow
class TestPluginWiring:
"""Guard against the plugin silently failing to load.
Expand All @@ -60,9 +216,18 @@ class TestPluginWiring:
otherwise.
"""

def _run_flake8(self, target: Path) -> subprocess.CompletedProcess[str]:
def _run_flake8(
self,
*,
target: Path,
select: str | None = None,
) -> subprocess.CompletedProcess[str]:
args = [sys.executable, "-m", "flake8"]
if select is not None:
args.extend(["--select", select])
args.append(str(target))
return subprocess.run( # ruff: ignore[subprocess-without-shell-equals-true]
[sys.executable, "-m", "flake8", str(target)],
args,
cwd=_REPO_ROOT,
capture_output=True,
text=True,
Expand All @@ -73,19 +238,64 @@ def test_reports_violation_through_flake8(self, tmp_path: Path) -> None:
target = tmp_path / "sample.py"
target.write_text("async def fetch():\n pass\n", encoding="utf-8")

result = self._run_flake8(target)
result = self._run_flake8(target=target)

assert result.returncode == 1
assert "RMP001" in result.stdout

def test_reports_lazy_export_violation_through_flake8(
self,
*,
tmp_path: Path,
) -> None:
target = tmp_path / "sample.py"
target.write_text(
'__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}\n__all__ = []\n',
encoding="utf-8",
)

result = self._run_flake8(target=target)

assert result.returncode == 1
assert "RMP002" in result.stdout

@pytest.mark.parametrize(
("select", "expected", "unexpected"),
[
("RMP001", "RMP001", "RMP002"),
("RMP002", "RMP002", "RMP001"),
],
)
def test_selects_registered_checker_independently(
self,
*,
tmp_path: Path,
select: str,
expected: str,
unexpected: str,
) -> None:
target = tmp_path / "sample.py"
target.write_text(
"async def fetch(): ...\n"
'__lazy_imports__ = {"Heavy": ("package.heavy", "Heavy")}\n'
"__all__ = []\n",
encoding="utf-8",
)

result = self._run_flake8(target=target, select=select)

assert result.returncode == 1
assert expected in result.stdout
assert unexpected not in result.stdout

def test_honors_noqa_through_flake8(self, tmp_path: Path) -> None:
target = tmp_path / "sample.py"
target.write_text(
"async def fetch(): # noqa: RMP001\n pass\n",
encoding="utf-8",
)

result = self._run_flake8(target)
result = self._run_flake8(target=target)

assert result.returncode == 0, result.stdout

Expand All @@ -94,6 +304,6 @@ def test_runs_no_rules_other_than_rmp(self, tmp_path: Path) -> None:
target = tmp_path / "sample.py"
target.write_text("import os\nx=1\n", encoding="utf-8")

result = self._run_flake8(target)
result = self._run_flake8(target=target)

assert result.returncode == 0, result.stdout
Loading
Loading