Merge rust-bitcoin#4883: Introduce and enforce import statement policy

c0f5db2 CI: Enforce policy (Tobin C. Harding)
1f46b8a docs: Add clause to import policy section (Tobin C. Harding)

Pull request description:

  Add and enforce policy around import statements using `crate::Foo` instead of `units::Foo`.


ACKs for top commit:
  apoelstra:
    ACK c0f5db2; successfully ran local tests; let's give this a shot


Tree-SHA512: c85e47aae7e3992340e71c1cd81a3057fe7eea152c56d91da0a67a557c3724223c2f5dcd7ca572d1b50102eeda442336993752e815043b389e546e91fe5abca7

Author: apoelstra

.github/workflows/README.md

@@ -29,6 +29,7 @@ Run from rust.yml unless stated otherwise. Unfortunately we are now exceeding th
 15. `WASM`
 16. `Kani`
 17. `API`
-18. `release` - run by `release.yml`
-19. `labeler` - run by `manage-pr.yml`
-20. `Shellcheck` - run by `shellcheck.yml`
+18. `Policy` - enforce repository coding policy.
+19. `release` - run by `release.yml`
+20. `labeler` - run by `manage-pr.yml`
+21. `Shellcheck` - run by `shellcheck.yml`

.github/workflows/rust.yml

@@ -322,3 +322,18 @@ jobs:
         run: cargo install --locked cargo-public-api --version 0.49.0
       - name: "Run API checker script"
         run: ./contrib/check-for-api-changes.sh
+
+  Policy:
+    name: Enforce repo policy - stable toolchain
+    runs-on: ubuntu-24.04
+    strategy:
+      fail-fast: false
+    steps:
+      - name: "Checkout repo"
+        uses: actions/checkout@v4
+      - name: "Checkout maintainer tools"
+        uses: actions/checkout@v4
+      - name: "Select toolchain"
+        uses: dtolnay/rust-toolchain@stable
+      - name: "Run policy script"
+        run: ./contrib/check-for-policy-violations.sh

contrib/check-for-policy-violations.sh

@@ -0,0 +1,76 @@
+#!/bin/bash
+#
+# Check if PR abides by our policy.
+
+set -euo pipefail
+
+# When running script locally the name used for the `github.com/rust-bitcoin/rust-bitcoin` remote.
+REMOTE="upstream"
+
+main() {
+    check_required_commands
+
+    if low_level_import_usage; then
+        err "Please do not import directly from low level crates, import using 'use crate::' instead"
+    fi
+}
+
+# Enforces import policy.
+#
+# See `./policy.md` section: `### On re-exports`.
+# Greps patch for imports that violate the policy, returns true if an
+# violations are found.
+low_level_import_usage() {
+    local crates=("units" "primitives")
+    local found_violation=false
+    local violations;
+
+    # Determine the base branch - common CI environment variables.
+    local base_branch="$REMOTE/master"
+    if [[ -n "${GITHUB_BASE_REF:-}" ]]; then
+        base_branch="$GITHUB_BASE_REF"
+    elif [[ -n "${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:-}" ]]; then
+        base_branch="$CI_MERGE_REQUEST_TARGET_BRANCH_NAME"
+    fi
+
+    for crate in "${crates[@]}"; do
+        violations=$(git diff "$base_branch"...HEAD -- '*.rs' | grep "^+" | grep -E "use ${crate}::" | grep -v "pub use ${crate}::" || true)
+        if [[ -n "$violations" ]]; then
+            say_err "invalid import statement: '${violations:1}'"
+            found_violation=true
+        fi
+    done
+
+    $found_violation
+}
+
+# Check all the commands we use are present in the current environment.
+check_required_commands() {
+    need_cmd grep
+    need_cmd git
+}
+
+say() {
+    echo "policy: $1"
+}
+
+say_err() {
+    say "$1" >&2
+}
+
+err() {
+    echo "$1" >&2
+    exit 1
+}
+
+need_cmd() {
+    if ! command -v "$1" > /dev/null 2>&1
+    then err "need '$1' (command not found)"
+    fi
+}
+
+#
+# Main script
+#
+main "$@"
+exit 0

docs/policy.md

@@ -117,6 +117,13 @@ pub use units::{foo, FooBar};
 pub use units::foo::SomeError;
 ```
 
+### Usage of re-exports
+
+As part of the attempt to mirror the `units` and `primitives`, and `bitcoin` APIs, in this codebase,
+policy is to use types from the highest crate available i.e `use crate::Foo` not `units::Foo`.
+
+This is enforced by CI.
+
 ## Return `Self`
 
 Use `Self` as the return type instead of naming the type. When constructing the return value use