Skip to content

[CI]: Add flake8 local plugin enforcing the async naming convention - #158

Merged
Spencer Schoenberg (spencrr) merged 3 commits into
microsoft:mainfrom
spencrr:dev/spencrr/linter-tool-v2-flake8
Aug 14, 2026
Merged

[CI]: Add flake8 local plugin enforcing the async naming convention#158
Spencer Schoenberg (spencrr) merged 3 commits into
microsoft:mainfrom
spencrr:dev/spencrr/linter-tool-v2-flake8

Conversation

@spencrr

Copy link
Copy Markdown
Contributor

Description

Follow-up to #22 (closed unmerged), taking Nina Chikanov (@nina-msft)'s suggestion to use a flake8 plugin instead of a custom linter. Covers the _async naming rule only.

69 lines instead of 811. No source files change.

flake8 supplies the noqa parser, CLI, file discovery and rule dispatch that #22 hand-built. What remains is the rule itself, ~9 lines.

Two deviations from the suggested snippet:

  • Prefix is RMP, not RAMPART. flake8 validates codes against ^[A-Z]{1,3}[0-9]{0,3}$ and refuses to start otherwise.
  • No packaging. Local plugins drop the entry point: .flake8 points at tools/. Nothing ships in the rampart wheel.

On running two linters: ruff has no plugin system and no rule for name suffixes. select = RMP means flake8 runs only this rule, so it never overlaps ruff.

Baseline

159 pre-existing violations are recorded as per-file-ignores in .flake8 rather than fixed here, keeping this diff reviewable. The rule is live now; follow-up commits shrink the baseline to nothing.

Silent no-op guard

flake8 --select=RMP exits 0 if nothing owns the prefix. Mitigated two ways: any load failure is a hard error (exit 1), and 3 subprocess tests run the real .flake8 config to assert the rule actually fires.

Notes

  • RMP001 is in [tool.ruff.lint] external so RUF102 accepts # noqa: RMP001. Split: # ruff: ignore[rule] for ruff, # noqa: for flake8.
  • Pre-commit hook triggers on .flake8 too, so config edits can't skip it and fail CI later.

Breaking changes

None.

Checklist

  • pre-commit run --all-files passes
  • Tests added or updated for changes — 10 in tests/unit/tools/test_flake8_rampart.py: 7 unit (dunder exemption, nested/class-level defs, sync ignored, position) + 3 subprocess (registration, noqa, select scoping)
  • Documentation updated — Automated Enforcement section in coding-standards.instructions.md; code-style.md (which still said this rule needed "a human eye") and development-setup.md

Replaces the bespoke AST linter proposed in microsoft#22 with
existing tooling, per review feedback on that PR.

Ruff has no plugin system and no rule that can require a name suffix, so the
`_async` convention lives in a flake8 *local plugin*: one module under tools/,
wired up by `.flake8`. Local plugins need no packaging or entry point, and
flake8 supplies `# noqa` handling, file discovery, config and exit codes for
free. The code prefix is `RMP` because flake8 validates plugin codes against
`^[A-Z]{1,3}[0-9]{0,3}$`, which rejects a longer prefix. `select = RMP` keeps
flake8 from running any general-purpose check, leaving those to ruff.

73 lines of plugin instead of 811 lines of linter.

This commit changes no source files. The 159 pre-existing violations are
recorded as a `per-file-ignores` baseline so the rule is enforced from day
one; follow-up commits fix the files and remove their baseline entries.
@spencrr
Spencer Schoenberg (spencrr) requested a review from a team August 8, 2026 01:13
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Comment thread docs/contributing/code-style.md
Comment thread docs/contributing/code-style.md Outdated
Comment thread docs/contributing/code-style.md Outdated
Comment thread docs/contributing/code-style.md Outdated
@spencrr
Spencer Schoenberg (spencrr) force-pushed the dev/spencrr/linter-tool-v2-flake8 branch from 3f9b417 to c419f11 Compare August 13, 2026 23:29
Comment thread .pre-commit-config.yaml Outdated
Comment thread docs/contributing/code-style.md Outdated
Comment thread pyproject.toml Outdated

@nina-msft Nina Chikanov (nina-msft) left a comment

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.

yay!

@spencrr
Spencer Schoenberg (spencrr) merged commit 9f64544 into microsoft:main Aug 14, 2026
14 checks passed
@spencrr
Spencer Schoenberg (spencrr) deleted the dev/spencrr/linter-tool-v2-flake8 branch August 14, 2026 01:24
Spencer Schoenberg (spencrr) added a commit that referenced this pull request Aug 14, 2026
## Description

Second commit of the #22 follow-up. Clears the `.flake8` baseline added
in #158 (merge that first), so `RMP001` is enforced everywhere with no
exemptions.

147 renames: 8 in `rampart/`, 139 async test functions, plus call sites,
tests and docs.

| Old | New |
|---|---|
| `InjectionHandle.wait_until_ready` | `wait_until_ready_async` |
| `ExecutionEventHandler.on_event` | `on_event_async` |
| `sleep_until_ready` | `sleep_until_ready_async` |
| `BaseExecution._fire` | `_fire_async` |
| `_send_via_normalizer` | `_send_via_normalizer_async` |
| `_send_and_parse` | `_send_and_parse_async` |

## Breaking changes

`InjectionHandle.wait_until_ready` and `ExecutionEventHandler.on_event`
are public. External surfaces and handlers must rename to `*_async`.
Handles that don't now raise a clear `TypeError` from `Attacks.xpia`
instead of failing obscurely.

## Checklist

- [x] `pre-commit run --all-files` passes
- [x] Tests added or updated for changes -- 20 test files updated for
renamed symbols and test names
- [x] Documentation updated -- `core-protocols.md`, `xpia.md`,
`extending-rampart.md`, `authoring-tests.md`,
`unit-tests-standards.instructions.md`
Mahdi Alhakim (mahdi-al-hakim) added a commit to mahdi-al-hakim/RAMPART that referenced this pull request Aug 15, 2026
Rebased onto main, which now enforces RMP001 from microsoft#158 and microsoft#159. The tests
this PR adds were written before that rule landed, so they are renamed to
match it. Seven names are also shortened to stay inside the line limit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants