Skip to content

Commit 35b64f2

Browse files
leliaclaude
andcommitted
Enforce ruff in CI and pre-commit, and clear the existing violations
Ruff already ran in CI, but only as a job inside the Unit Tests workflow, so it inherited that workflow's path filter and never saw .hooks/, benchmarks/, or tests/e2e/. Move it to its own unconditional Lint workflow, which also keeps it usable as a required status check. Add ruff to pre-commit so violations surface before CI. The hook runs ruff out of the project environment rather than the upstream mirror, so the version stays pinned in one place; Dependabot has no pre-commit ecosystem and would never update a mirror's rev. Expand the rule set beyond E/F/I to cover bug classes that matter for a CLI other people run in their pipelines, and fix every resulting violation so the baseline is clean rather than suppressed. Behaviour changes worth calling out: - Package.created_at used str.strip(" (Coordinated Universal Time)"), which treats its argument as a set of characters, not a suffix. It ate a leading "T" from "Tue ..." and a trailing "T" from timestamps that carried no suffix at all. Now uses removesuffix. - Every requests call in the plugins and the GitLab client now passes an explicit timeout. requests blocks forever by default, so a hung notification could wedge the pipeline the CLI reports into. - Two asserts became real checks. assert is stripped under python -O, so neither guard survived an optimised interpreter. - config.py logs through the socketcli logger instead of the root logger, so its messages honour the configured level and format. - A stray debug print in the SBOM artifact loop became a log.debug call; it was writing to stdout, which carries machine-readable output. - Closures defined inside loops in alert_selection and messages were hoisted and now take their inputs explicitly. Complexity is bounded by C901 (max 12) and PLR0913 (max 8). The 20 functions over the limit today carry an explicit noqa; RUF100 fails the build once a suppression goes stale, so the list can only shrink. E501 and W291/W293 are left to ruff format rather than duplicated in the linter: everything the formatter cannot reflow is a string literal, and the PR-comment markup depends on trailing double-spaces as Markdown hard line breaks. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent eb3e46a commit 35b64f2

82 files changed

Lines changed: 4116 additions & 4117 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.git-blame-ignore-revs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Revisions that only reformat or mechanically re-lint code.
2+
# Configure once per clone:
3+
# git config blame.ignoreRevsFile .git-blame-ignore-revs
4+
# (GitHub applies this file automatically in its blame view.)

.github/workflows/lint.yml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
name: Lint
2+
3+
env:
4+
PYTHON_VERSION: "3.12"
5+
6+
# Deliberately not path-filtered. Ruff finishes in well under a minute, and its
7+
# trigger surface is every Python file in the repository -- including the ones
8+
# outside the Unit Tests filters (.hooks/, benchmarks/, tests/e2e/). Running
9+
# unconditionally also keeps this usable as a required status check: a
10+
# path-filtered workflow reports as "not run" rather than "passed", which blocks
11+
# any pull request that does not happen to touch the filtered paths.
12+
on:
13+
push:
14+
branches: [main]
15+
pull_request:
16+
workflow_dispatch:
17+
18+
permissions:
19+
contents: read
20+
21+
concurrency:
22+
group: lint-${{ github.event.pull_request.number || github.ref }}
23+
cancel-in-progress: true
24+
25+
jobs:
26+
ruff:
27+
runs-on: ubuntu-latest
28+
timeout-minutes: 10
29+
steps:
30+
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
31+
with:
32+
fetch-depth: 1
33+
persist-credentials: false
34+
- name: 🐍 setup python
35+
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
36+
with:
37+
python-version: ${{ env.PYTHON_VERSION }}
38+
- name: 🛠️ install deps
39+
run: |
40+
python -m pip install --upgrade pip
41+
pip install uv
42+
uv sync --extra dev
43+
# Same ruff version the pre-commit hook uses (pinned in pyproject.toml,
44+
# locked in uv.lock), so a clean commit locally stays clean here.
45+
- name: 🧹 ruff check
46+
run: uv run ruff check
47+
- name: 🎨 ruff format
48+
run: uv run ruff format --check

.github/workflows/python-tests.yml

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -67,26 +67,6 @@ jobs:
6767
uv export --no-hashes --no-emit-project --format requirements-txt > /tmp/req-audit.txt
6868
uvx pip-audit --strict --progress-spinner off --disable-pip --no-deps -r /tmp/req-audit.txt
6969
70-
ruff:
71-
runs-on: ubuntu-latest
72-
timeout-minutes: 10
73-
steps:
74-
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
75-
with:
76-
fetch-depth: 1
77-
persist-credentials: false
78-
- name: 🐍 setup python
79-
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
80-
with:
81-
python-version: ${{ env.PYTHON_VERSION }}
82-
- name: 🛠️ install deps
83-
run: |
84-
python -m pip install --upgrade pip
85-
pip install uv
86-
uv sync --extra dev
87-
- name: 🧹 run ruff
88-
run: uv run ruff check
89-
9070
unsupported-python-install:
9171
runs-on: ubuntu-latest
9272
timeout-minutes: 10

.hooks/sync_version.py

100644100755
Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
PYPI_PROD_API = "https://pypi.org/pypi/socketsecurity/json"
1717
PYPI_TEST_API = "https://test.pypi.org/pypi/socketsecurity/json"
1818

19+
1920
def read_version_from_init(path: pathlib.Path) -> str:
2021
content = path.read_text()
2122
match = VERSION_PATTERN.search(content)
@@ -24,6 +25,7 @@ def read_version_from_init(path: pathlib.Path) -> str:
2425
sys.exit(1)
2526
return match.group(1)
2627

28+
2729
def read_version_from_git(path: str) -> str:
2830
try:
2931
output = subprocess.check_output(["git", "show", f"HEAD:{path}"], text=True)
@@ -34,13 +36,15 @@ def read_version_from_git(path: str) -> str:
3436
except subprocess.CalledProcessError:
3537
return None
3638

39+
3740
def bump_patch_version(version: str) -> str:
3841
if ".dev" in version:
3942
version = version.split(".dev")[0]
4043
parts = version.split(".")
4144
parts[-1] = str(int(parts[-1]) + 1)
4245
return ".".join(parts)
4346

47+
4448
def parse_stable_version(version: str):
4549
if not STABLE_VERSION_PATTERN.fullmatch(version):
4650
return None
@@ -72,6 +76,7 @@ def fetch_latest_stable_pypi_version():
7276
return None
7377
return max(stable_versions)
7478

79+
7580
def find_next_available_dev_version(base_version: str) -> str:
7681
existing_versions = fetch_existing_versions(PYPI_TEST_API)
7782
for i in range(1, 100):
@@ -94,6 +99,7 @@ def find_next_stable_patch_version(current_version: str) -> str:
9499
next_parts = (base_parts[0], base_parts[1], base_parts[2] + 1)
95100
return format_stable_version(next_parts)
96101

102+
97103
def inject_version(version: str):
98104
print(f"🔁 Updating version to: {version}")
99105

@@ -190,16 +196,21 @@ def main():
190196
inject_version(new_version)
191197
uv_lock_changed = run_uv_lock()
192198
lock_hint = " and uv.lock" if uv_lock_changed else ""
193-
print(f"⚠️ Version {current_version} is already published on PyPI — auto-bumped to {new_version}. Please git add{lock_hint} + commit again.")
199+
print(
200+
f"⚠️ Version {current_version} is already published on PyPI — auto-bumped to {new_version}. Please git add{lock_hint} + commit again."
201+
)
194202
sys.exit(1)
195203

196204
uv_lock_changed = run_uv_lock()
197205
if uv_lock_changed:
198-
print("⚠️ Version already bumped, but uv.lock was out of date and has been updated. Please git add uv.lock + commit again.")
206+
print(
207+
"⚠️ Version already bumped, but uv.lock was out of date and has been updated. Please git add uv.lock + commit again."
208+
)
199209
sys.exit(1)
200210

201211
print("✅ Version already bumped and uv.lock is up to date — proceeding.")
202212
sys.exit(0)
203213

214+
204215
if __name__ == "__main__":
205216
main()

.pre-commit-config.yaml

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,29 @@ repos:
66
entry: python .hooks/sync_version.py
77
language: python
88
always_run: true
9-
pass_filenames: false
9+
pass_filenames: false
10+
11+
# Ruff runs out of the project environment rather than the upstream
12+
# astral-sh/ruff-pre-commit mirror so its version is pinned in exactly one
13+
# place: `ruff==0.16.4` under [project.optional-dependencies].dev, locked
14+
# in uv.lock and used verbatim by the Lint workflow. Dependabot has no
15+
# pre-commit ecosystem and will not touch a mirror's `rev:`, so a mirror
16+
# would drift out of step with CI and produce the worst failure mode for a
17+
# hook -- clean locally, red on the pull request.
18+
#
19+
# `--fix` applies only ruff's fixes marked safe. When it changes a file
20+
# pre-commit aborts the commit and leaves the edit in the working tree, so
21+
# nothing lands without being looked at.
22+
- id: ruff-check
23+
name: ruff check
24+
entry: uv run --extra dev ruff check --force-exclude --fix
25+
language: system
26+
types_or: [python, pyi]
27+
require_serial: true
28+
29+
- id: ruff-format
30+
name: ruff format
31+
entry: uv run --extra dev ruff format --force-exclude
32+
language: system
33+
types_or: [python, pyi]
34+
require_serial: true

CONTRIBUTING.md

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,16 @@ dependencies:
1111
uv sync --all-extras
1212
```
1313

14+
Install the git hooks once per clone:
15+
16+
```bash
17+
make hooks
18+
```
19+
1420
Before opening a pull request, run:
1521

1622
```bash
23+
make lint
1724
make test
1825
uv run hatch build
1926
uv run python -m twine check dist/*
@@ -22,6 +29,62 @@ uv run python -m twine check dist/*
2229
To develop against a local SDK checkout, set `SOCKET_SDK_PATH` if it is not at
2330
`../socketdev`, then run `make first-time-local-setup`.
2431

32+
## Linting
33+
34+
Ruff is the only linter. It runs in three places, all reading the same
35+
configuration from `pyproject.toml` and the same version pinned in
36+
`[project.optional-dependencies].dev`:
37+
38+
- `make lint` locally,
39+
- the `ruff-check` pre-commit hook, on the files a commit touches,
40+
- the `Lint` workflow, on every pull request and every push to `main`.
41+
42+
The pre-commit hook applies ruff's safe fixes and then fails the commit, leaving
43+
the edits unstaged so they get read before they land. CI is the backstop for
44+
commits made with `--no-verify` or without hooks installed.
45+
46+
`ruff format` is enforced the same way. It owns line length (120) and
47+
whitespace, so the linter does not duplicate those checks: `E501` and `W291`/
48+
`W293` are deliberately not selected. Everything the formatter cannot reflow is
49+
a string literal -- argparse help text, log messages, the Markdown used to build
50+
pull request comments -- where rewrapping risks silently changing user-visible
51+
text. The PR-comment markup in particular relies on trailing double-spaces as
52+
Markdown hard line breaks.
53+
54+
### One trap worth knowing
55+
56+
Never run `ruff check --select <narrow-list> --fix` with `RUF100` in the select.
57+
With a narrow select, RUF100 considers every `# noqa` for a *non-selected* rule
58+
to be unused and deletes it -- silently stripping the complexity suppressions
59+
across the repository. Run `make lint-fix`, which uses the full configured rule
60+
set, instead of hand-rolling a `--select`.
61+
62+
### Complexity limits
63+
64+
Two rules bound how large a single function may get:
65+
66+
| Rule | Limit | What it measures |
67+
| --- | --- | --- |
68+
| `C901` | 12 | Cyclomatic complexity: independent paths through a function, which is also the number of tests needed to cover it. |
69+
| `PLR0913` | 8 | Arguments in a function definition. |
70+
71+
Functions that already exceed these limits carry an explicit
72+
`# noqa: C901` / `# noqa: PLR0913` on their `def` line. That list is a backlog,
73+
not a precedent:
74+
75+
- **Do not add a new suppression.** If a function you are writing trips the
76+
limit, split it. This matters most for generated or model-assisted code, where
77+
branches accumulate quickly and nothing pushes back.
78+
- **Suppressions clean themselves up.** `RUF100` fails the build on a `# noqa`
79+
that no longer applies, so refactoring a function back under the limit forces
80+
the marker to be removed. The backlog can only shrink.
81+
82+
To see what is left:
83+
84+
```bash
85+
grep -rn 'noqa: C901\|noqa: PLR0913' socketsecurity/ tests/
86+
```
87+
2588
## Pull request validation
2689

2790
The `Package Check` workflow runs automatically for pull requests. It builds

Makefile

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
.PHONY: setup sync clean test lint update-lock local-dev first-time-setup dev-setup sync-all first-time-local-setup
1+
.PHONY: setup sync clean test lint lint-fix format format-check hooks update-lock local-dev first-time-setup dev-setup sync-all first-time-local-setup
22

33
# Environment variable for local SDK path (optional)
44
SOCKET_SDK_PATH ?= ../socketdev
@@ -57,6 +57,20 @@ clean:
5757
test:
5858
uv run pytest
5959

60+
# Installs the git pre-commit hooks (ruff + version sync).
61+
hooks:
62+
uv run --extra dev pre-commit install
63+
64+
# Exactly what the Lint workflow runs, so a green `make lint` means a green CI.
6065
lint:
61-
uv run ruff check .
62-
uv run ruff format --check .
66+
uv run --extra dev ruff check
67+
uv run --extra dev ruff format --check
68+
69+
lint-fix:
70+
uv run --extra dev ruff check --fix
71+
72+
format:
73+
uv run --extra dev ruff format
74+
75+
format-check:
76+
uv run --extra dev ruff format --check

benchmarks/manifest_discovery.py

100644100755
Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@ def legacy_discover(root: Path) -> set[str]:
4747
insensitive = Core.to_case_insensitive_regex(pattern)
4848
for candidate in root.rglob(insensitive):
4949
if candidate.is_file() and not Core.is_excluded(
50-
str(candidate),
51-
excluded_dirs,
50+
str(candidate),
51+
excluded_dirs,
5252
):
5353
results.add(candidate.as_posix())
5454
return results
@@ -85,10 +85,7 @@ def main() -> None:
8585
)
8686

8787
if legacy_results != new_results:
88-
raise SystemExit(
89-
"Manifest result mismatch: "
90-
f"legacy={len(legacy_results)}, single_pass={len(new_results)}"
91-
)
88+
raise SystemExit(f"Manifest result mismatch: legacy={len(legacy_results)}, single_pass={len(new_results)}")
9289

9390
speedup = legacy_seconds / new_seconds if new_seconds else float("inf")
9491
print(f"Manifests: {len(new_results)}")

0 commit comments

Comments
 (0)