Skip to content
Open
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
12 changes: 12 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,17 @@ 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. Dynamic expressions and incremental mutation (`+=`, item assignment, or
mutating methods such as `append`, `extend`, and `update`) are rejected because
their final values cannot be verified as single literal declarations.
[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
219 changes: 202 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,176 @@
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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment was generated by Copilot.

Likelihood: low — these are coverage nice-to-haves, not a sign of a current bug; the existing tests already exercise the main paths. Worth adding mainly to pin down intent against future regressions.

Two cases worth adding:

  1. Mixed registry — several lazy keys where only some are in __all__; assert exactly the missing name(s) are reported (guards against an all-or-nothing regression).
  2. The __all__ += [...] / .extend(...) augmented form, asserting whatever behavior you settle on above — so the intent is documented either way.

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__.append("Heavy")', "__all__"),
('__all__.extend(["Heavy"])', "__all__"),
(
'__lazy_imports__ |= {"Other": ("package.other", "Other")}',
"__lazy_imports__",
),
(
'__lazy_imports__.update({"Other": ("package.other", "Other")})',
"__lazy_imports__",
),
(
'__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__ = []
{mutation}
"""

(message,) = _lazy_export_messages(source)

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

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 +191,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 +213,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 +279,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