Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
395 lines (320 loc) · 16.9 KB

CONTRIBUTING.md

File metadata and controls

395 lines (320 loc) · 16.9 KB
Raw

Contributing to big-code-analysis

Thanks for considering a contribution. This document covers the essentials; the deeper conventions live in AGENTS.md, CLAUDE.md, and the developer guide under big-code-analysis-book/.

Ground rules

  • By submitting a pull request, you agree to license your contribution under MPL-2.0, matching the rest of the project (declared via license = "MPL-2.0" in the root Cargo.toml).
  • Security-sensitive reports must not go in public issues; see SECURITY.md for the private disclosure channels.

Getting started

git clone https://github.com/dekobon/big-code-analysis
cd big-code-analysis
make worktree-setup
cargo build --workspace
cargo test --workspace --all-features

make worktree-setup checks out the integration corpora and, if uv is installed, the Python-bindings venv. It is idempotent, so re-run it any time the environment looks wrong — including after an interrupted corpus checkout, which leaves the submodule empty at the recorded SHA where a plain git submodule update --init is a silent no-op.

MSRV is 1.94, declared once in the root Cargo.toml ([workspace.package] rust-version = "1.94") and inherited by every member crate.

Integration snapshots live in the big-code-analysis-output submodule under tests/repositories/. Initialize submodules before running the test suite, otherwise 24 tests fail; each says so.

The out-of-band benchmark harness (make bench-*) additionally walks DeepSpeech's own submodules, which the corpus tests exclude and worktree-setup therefore does not fetch. See Benchmarking for its recursive checkout.

The cargo-fuzz targets (make fuzz-*) are likewise out of band and need a nightly toolchain plus cargo install cargo-fuzz --locked; every recipe skips with a printed reason when either is missing, so a stable-only checkout is not blocked. See Fuzzing.

The two binaries shipped with the workspace are:

  • bca, the CLI (big-code-analysis-cli): cargo run -p big-code-analysis-cli --.
  • bca-web, the REST API server (big-code-analysis-web): cargo run -p big-code-analysis-web --.

Optional: a faster linker

Every test binary in this workspace statically links the tree-sitter runtime and all twenty-odd grammars, so linking — not compiling — is the tail of an incremental cargo test after a one-line edit. GNU ld, still the default on most Linux toolchains, is single-threaded and is usually the slowest part of that.

This is deliberately not configured in the repository: which linker is installed, and which one works, varies per machine, and a committed [target.*] rustflags that names a missing linker breaks the build for everyone who does not have it. Install one yourself and add it to a local, untracked .cargo/config.toml:

# mold — https://github.com/rui314/mold (apt: mold, brew: mold)
[target.x86_64-unknown-linux-gnu]
rustflags = ["-C", "link-arg=-fuse-ld=mold"]

For LLVM's lld (apt install lld, or bundled with a Homebrew LLVM), substitute -fuse-ld=lld. On macOS the platform linker is already parallel and neither is needed. Note that .cargo/config.toml is tracked here — it carries the cargo mutants and cargo xtask aliases — so add the stanza without committing it, or put it in your user-level ~/.cargo/config.toml instead.

Local validation gate

make pre-commit is the canonical entry point for the full validation gate. In one parallel pass it runs, among other stages:

  • cargo fmt --check.
  • cargo clippy --workspace --all-targets -- -D warnings in both default-features and --all-features flavours.
  • The full test suite via make test (cargo-nextest when present, otherwise cargo test) plus make test-doc for doctests.
  • cargo doc --no-deps --workspace --all-features with RUSTDOCFLAGS="-D warnings".
  • cargo +nightly udeps.
  • Markdown / TOML / shell / Makefile / GitHub Actions lint families (rumdl, taplo, shellcheck, shfmt, checkmake, actionlint).
  • The man-page drift gate and the bca self-scan threshold gates.
  • ./utils/check-snapshot-anchors.py (see "Snapshot anchors" below).
  • The Python lint / type-check / test stages (see below), skipped per stage when a tool is absent.

The authoritative stage list lives in the "Validation gates" section of AGENTS.md.

make ci runs the same checks without auto-fix, mirroring the GitHub Actions behaviour.

Reading the verdict

Both gates end with exactly one machine-readable line on stdout:

BCA_GATE: pass (gate=pre-commit)
BCA_GATE: fail (gate=pre-commit, exit=2, stage=_pc-fmt)

Grep for a line starting BCA_GATE: — nothing else in the repository emits one, there is exactly one per run, and it is the last thing either gate writes.

Do not read an outcome out of make's Error N lines. The gate is a parallel DAG, so make[2]: *** [… _pc-fmt] Error 2 is printed the instant a stage fails, while other stages are still running — it is routinely followed by several more stages' successful output. In one measured run the failing stage was reported at line 58 of a 236-line log, with 174 lines of green output after it.

stage= lists every stage make reported as failing, comma-separated in the order reported: under -j the first failure stops scheduling, but stages already running finish and can fail too. It reads unknown when the gate died before make named a stage.

No BCA_GATE: line at all is a third state, not a pass. It means the run never finished — it crashed, was killed, or was interrupted. On the failure path make appends its own make: *** [… pre-commit] Error 2 epilogue after the verdict, because the gate exits non-zero and make says so; the verdict line does not change either gate's exit status.

Capture a run like this:

log=$(mktemp /tmp/bca-pre-commit.XXXXXX.log)
make pre-commit >"$log" 2>&1
grep '^BCA_GATE:' "$log"

Two habits that idiom encodes:

  • Give every run its own log path. A fixed /tmp/pc.log is shared by every checkout, worktree, and tool on the host. During one batch a concurrent run's log was read as this one's, and a fmt-check failure was diagnosed against code that had never been touched.
  • Read the verdict from the log, not from a reported exit status. Some tooling reports the status of the last command on the line, so make pre-commit >log 2>&1; echo "EXIT=$?" announces echo's success. That is a property of the caller, not of make; the log is the thing that is true either way.

If GNU Make 4 or any optional tool is unavailable, fall back to the raw cargo trio:

cargo fmt --all -- --check
cargo clippy --workspace --all-targets -- -D warnings
cargo test --workspace --all-features

If pre-commit is installed, also run pre-commit run --all-files; the project's .pre-commit-config.yaml wires clippy, cargo +nightly udeps, the test suite, ruff-check + ruff-format, and a local mypy --strict hook into the standard hook flow.

Python bindings (big-code-analysis-py)

The Python bindings use ruff for lint + format and mypy --strict plus pyright for type checking. make pre-commit and make ci run these alongside the Rust gates when the tools are present.

Canonical install path: make py-bootstrap (requires uv: curl -LsSf https://astral.sh/uv/install.sh | sh, brew install uv, or pipx install uv). This runs uv sync --locked --extra dev against the checked-in uv.lock, so the resolved ruff/mypy/pyright/maturin/pytest versions are identical across every contributor on this path.

After editing the dev extra in pyproject.toml, run make py-relock, which regenerates uv.lock and the hash-pinned exports under big-code-analysis-py/requirements/ (dev.txt, examples.txt). make py-bootstrap will refuse to silently rewrite uv.lock (intentional, so lockfile churn is always a deliberate, reviewable commit). Resolve uv.lock rebase conflicts by re-running make py-relock rather than hand-merging the file. CI consumes uv.lock through the requirements exports (pip install --require-hashes -r … in the workflows), so a uv.lock change and its regenerated exports must land in the same commit.

Note that make py-relock re-resolves but does not upgrade: uv lock keeps a package at its locked version as long as that version still satisfies the requirement, so widening a bound alone changes nothing. Adopting a newer release is a second, explicit step — uv lock --upgrade-package <name> from big-code-analysis-py, followed by the two uv export commands py-relock runs. #1222 raised ruff's ceiling to <0.17 and would have shipped a still-locked 0.15.22 without it.

Alternative install paths (mise install via mise.toml, direct pipx install ruff/mypy/pyright/maturin, python -m venv .venv && pip install -e ".[dev]") still work but bypass uv.lock; resolved versions can drift from peers and from CI. They remain documented for contributors with environment constraints that preclude uv. None of them pins ruff, deliberately — see below.

The ruff version is gated

One ruff version is adopted, and uv.lock is where it is decided. make check-ruff-lockstep (wired into make lint, make pre-commit, make ci, and the pre-commit hooks) fails unless three other declarations agree with it:

  • the ruff-pre-commit rev: in .pre-commit-config.yaml, which must be v + the locked version — otherwise pre-commit run --all-files runs a different ruff than CI;
  • the ruff== pin in big-code-analysis-py/requirements/dev.txt, the hash-pinned export CI installs from;
  • the ruff bound in pyproject.toml, which must match the one uv recorded resolving against — a bound edited without make py-relock leaves CI installing a version the manifest no longer admits.

Adopting a newer ruff is therefore uv lock --upgrade-package ruff plus the uv export pair make py-relock runs, then the rev:, all in one commit. The gate names whichever file has fallen behind.

make py-fmt, py-fmt-check and py-lint run big-code-analysis-py/.venv/bin/ruff when it exists and fall back to PATH otherwise, mirroring how py-typecheck resolves mypy and pyright. So the local gate runs the locked ruff as soon as you have run make py-bootstrap. The alternative install paths above stay unpinned on purpose: an exact version in mise.toml or the Dockerfile would be a fifth copy to bump on every ruff release, in a file no gate reads.

Then build and test:

cd big-code-analysis-py
source .venv/bin/activate
maturin develop
python -m pytest

Or simply make py-test from the repo root, which performs the maturin-develop step implicitly. CI runs the same flow across Linux / macOS / Windows on every supported Python version, gated by a dorny/paths-filter job so Rust-only PRs skip it entirely.

To remove Python build artifacts (.venv, compiled extension, per-tool caches, __pycache__ trees), run make py-clean. For a full wipe of both cargo target/ and Python state, run make distclean.

Snapshot anchors

Per-metric tests under src/metrics/ use insta snapshot assertions. Every insta::assert_json_snapshot! call must be anchored: a bare insta::assert_json_snapshot!(metric.X) records whatever production emitted at acceptance time, including bugs.

Each new snapshot assertion must carry one of:

  • An inline expected block: insta::assert_json_snapshot!(metric.X, @r###"…"###).
  • A positive assert_eq! on the headline value(s) immediately above the snapshot call, using integer-valued accessors (branches(), class_npm_sum(), unique_operators(), …). Float magnitudes, averages, and Halstead volume/difficulty/effort are bit-brittle and not safe for exact equality.
  • A // expected: <derivation> comment explaining what the values should be and why, sufficient for a reviewer to verify without re-deriving the metric from scratch.

The policy is enforced automatically by ./utils/check-snapshot-anchors.py, which make pre-commit, make ci, the pre-commit hooks, and the lint job in .github/workflows/ci.yml all invoke. The per-file baseline of pre-existing unanchored snapshots lives in .snapshot-anchor-baseline.txt; CI fails on any increase against that baseline.

Background reading:

  • AGENTS.md, section "Validation gates": the full policy and the bulk-acceptance rules around grammar bumps.
  • docs/development/lessons_learned.md, lesson 2 ("Tree-sitter aliases one rule across many kind_ids") and lesson 6 ("Snapshot tests pin behaviour, not correctness"): the bug classes the anchor rule exists to catch.

For bulk snapshot refresh after a grammar bump or a deliberate metric-computation change, use cargo insta test --accept per test file. Accepting snapshots one at a time via mv *.snap.new shifts assertion_line fields and cascades stale-snapshot churn.

Integration snapshots and the submodule

Behaviour-changing fixes that touch metric computation, AST traversal, or alterator rules also shift snapshots inside the big-code-analysis-output submodule. A fix is not done until all of the following have happened in the same parent commit:

  1. cargo test --workspace --all-features exits clean from a fresh working tree (no .snap.new files left behind under tests/repositories/big-code-analysis-output/).
  2. The accepted snapshots are committed and pushed to the submodule's remote (dekobon/big-code-analysis-output, main branch).
  3. The parent records the new submodule SHA (git add tests/repositories/big-code-analysis-output) in the same parent commit as the metric/alterator fix.
  4. After any rebase, force-push, or long-running batch fix, re-run the integration tests before declaring done.

See lesson 8 in docs/development/lessons_learned.md for why this matters.

Project conventions

  • Rust style: cargo fmt, clippy clean with --workspace --all-targets -- -D warnings. No unsafe code (one narrow PyO3 FFI exception; see AGENTS.md). Avoid unwrap / expect / panic! / assert! in non-test code; propagate errors with ?.
  • Visibility: prefer pub(crate) over pub; widen visibility only when an item is re-exported from lib.rs.
  • Edition: 2024; let-else, let-chains, and other 2024 features are available.
  • Borrowing: prefer &str over String parameters unless ownership is required downstream. Never use to_string_lossy() on paths used as identifiers (map keys, JSON output, error correlation); use to_str() with explicit error handling.
  • Per-language modules mirror each other: a bug in one big-code-analysis-ast/src/languages/language_<lang>.rs typically exists in several. Fix every affected sibling together.
  • Public API: this is a published library on crates.io. Treat lib.rs re-exports, public traits (ParserTrait, LanguageInfo, …), and public types (Metrics, FuncSpace, language enums) as a stable surface; break them only with an intentional version bump.

Commits and pull requests

  • Follow Conventional Commits: feat(scope): …, fix(scope): …, refactor(scope): …, docs(scope): …, etc.
  • Commit in small, reviewable steps. Each commit should build and pass tests on its own where practical.
  • When fixing a bug, add a regression test that would catch the exact bug if reintroduced.
  • For user-visible changes (API additions, behaviour changes, bug fixes), add an entry to CHANGELOG.md under ## [Unreleased]. Refactors, docs-only, and CI changes don't need a changelog entry.
  • Open the pull request against main. Link the issue with Fixes #NNN in the PR body.

Adding a new language or bumping a grammar

The two workflows most likely to trip a new contributor have dedicated guides under big-code-analysis-book/src/developers/:

External grammar crates are version-pinned (=0.23.x, etc.) in the root Cargo.toml. Do not loosen those pins without explicit approval; a grammar bump is a deliberate, separate change and snapshot tests shift accordingly.

Code review

Criticism is welcome: point out mistakes, suggest better approaches, cite relevant standards. Be skeptical and concise. Reviews focus on correctness, API shape, and test coverage before style.

Questions

Open a GitHub Discussion or a low-priority issue. We'd rather answer a question than review a PR that went the wrong direction.