Skip to content
Merged
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
19 changes: 18 additions & 1 deletion .github/workflows/formatting.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,22 @@ jobs:
cache: true
cache-dependency-path: go.sum

# `gofmt -l` walks the filesystem rather than Go's package wildcard, so
# unlike `go vet ./...` it has to be told which files are the module's own.
# scripts/go_files.sh is the one place that answers, here and locally, by
# asking the toolchain — which is what keeps vendor/ and testdata/ out.
#
# pipefail matters more than usual: without it, go_files.sh failing
# mid-pipeline is masked by xargs exiting 0 on no input, and the step
# passes having checked nothing at all.
- name: check formatting
run: if [ $(gofmt -l . | grep -Ev '^vendor\/' | head -c1 | wc -c) -ne 0 ]; then exit 1; fi
run: |
set -euo pipefail

unformatted=$(scripts/go_files.sh . | xargs -0 gofmt -l)

if [ -n "${unformatted}" ]; then
echo "::error::gofmt reports these files as unformatted:"
echo "${unformatted}"
exit 1
fi
14 changes: 14 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,20 @@ This template does **not** vendor dependencies (platform-go's dependency tree is
tests run against the module cache. Vendoring targets (`make vendor` / `make revendor`) exist for
consumers who want them.

`scripts/go_files.sh` is the one place that decides which Go files the formatters see, and
`format_golang.sh`, `format_imports.sh`, `goimports.sh`, and the `gofmt` check in
`.github/workflows/formatting.yaml` all take their list from it. It asks the Go toolchain
(`go list -e -f '{{.Dir}}' ./...`, then `find -maxdepth 1`) rather than writing out an exclusion
list, so `vendor/`, `testdata/`, and any `_`- or `.`-prefixed directory are skipped for the same
reason `go test ./...` skips them. Point new filesystem-walking tooling at it rather than adding a
fourth spelling of the same exclusion.

Two things about it are load-bearing. It **fails loudly rather than emitting an empty list** — an
out-of-sync `vendor/modules.txt` makes `go list` exit non-zero, and a formatter that quietly formats
nothing (or a CI check that quietly checks nothing) is worse than a stop. And its callers read it
**through a file, not `< <(...)`**, because process substitution discards the exit status of what it
runs, which is exactly how that empty list would go unnoticed.

## Import Ordering

Import ordering uses `gci` with four sections, separated by blank lines:
Expand Down
13 changes: 12 additions & 1 deletion scripts/format_golang.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,22 @@ set -euo pipefail

# Format all Go files using gofmt
# Usage: format_golang.sh <project_root> <gofmt_command>
#
# Which files those are is go_files.sh's question, not this one's.

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PROJECT_ROOT="${1:-$(pwd)}"

# Through a file rather than `< <(go_files.sh)`: process substitution discards
# the exit status of what it runs, so a failure to produce the list would read
# here as a list of no files, and formatting nothing would look like success.
file_list="$(mktemp)"
trap 'rm -f "${file_list}"' EXIT

"${SCRIPT_DIR}/go_files.sh" "${PROJECT_ROOT}" >"${file_list}"

while IFS= read -r -d '' file; do
# GO_FORMAT contains a command with arguments, so we use eval
# shellcheck disable=SC2086
eval "gofmt -s -w \"${file}\""
done < <(find "${PROJECT_ROOT}" -type f -not -path '*/vendor/*' -name "*.go" -print0)
done <"${file_list}"
15 changes: 13 additions & 2 deletions scripts/format_imports.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,26 @@ set -euo pipefail

# Format Go imports using gci
# Usage: format_imports.sh <package_prefix> <project_root>
#
# Which files those are is go_files.sh's question, not this one's.

SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PACKAGE_PREFIX="${1:-github.com/primandproper/template-go}"
PROJECT_ROOT="${2:-$(pwd)}"

# Find all Go files and pass them to gci
# Through a file rather than `< <(go_files.sh)`: process substitution discards
# the exit status of what it runs, so a failure to produce the list would read
# here as a list of no files, and formatting nothing would look like success.
file_list="$(mktemp)"
trap 'rm -f "${file_list}"' EXIT

"${SCRIPT_DIR}/go_files.sh" "${PROJECT_ROOT}" >"${file_list}"

# Every Go file the module owns, passed to gci
go_files=()
while IFS= read -r -d '' file; do
go_files+=("${file}")
done < <(find "${PROJECT_ROOT}" -type f -not -path '*/vendor/*' -name "*.go" -print0)
done <"${file_list}"

if [ ${#go_files[@]} -gt 0 ]; then
go tool gci write \
Expand Down
52 changes: 52 additions & 0 deletions scripts/go_files.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
#!/usr/bin/env bash
set -euo pipefail

# Emit every Go file this module owns, NUL-separated, for the formatters to
# consume with `read -r -d ''`.
# Usage: go_files.sh [project_root]
#
# The list comes from the Go toolchain rather than from a find(1) exclusion
# list, because the toolchain already knows the answer. `./...` does not
# descend into vendor/ or testdata/, nor into any directory whose name begins
# with _ or . — which is why every wildcard-driven target here (test, lint,
# go fix, fieldalignment, tagalign) needs no exclusions at all, and only the
# tools that walk the filesystem ever did.
#
# Those tools used to answer the question themselves, three different ways:
# format_golang.sh and format_imports.sh each carried their own
# `-not -path '*/vendor/*'`, the formatting workflow filtered gofmt's output
# with `grep -Ev '^vendor\/'`, and goimports.sh ran `goimports -w .` with no
# exclusion whatsoever — which rewrote every vendored file in the tree. One
# question deserves one answer, and this is it.
#
# -e keeps a package that does not compile in the list. Formatting a file is
# most useful exactly when it is still broken, so a syntax error somewhere in
# the module must not empty the whole list.
#
# -maxdepth 1 because what go list names are package directories, and a package
# is entitled to a testdata directory of its own.

PROJECT_ROOT="${1:-$(pwd)}"

cd "${PROJECT_ROOT}"

# go list is asked for the directories separately from walking them, so that a
# failure to list is a failure of this script rather than an empty answer.
# `go list` exits non-zero for module-level problems that -e does not cover — an
# out-of-sync vendor/modules.txt being the one to expect — and a formatter that
# quietly formats nothing, or a CI check that quietly checks nothing, is a worse
# outcome than either a wrong file list or a loud stop.
package_dirs="$(go list -e -f '{{.Dir}}' ./...)"

if [ -z "${package_dirs}" ]; then
echo "go_files.sh: go list named no packages under ${PROJECT_ROOT}" >&2
exit 1
fi

printf '%s\n' "${package_dirs}" | while IFS= read -r dir; do
# A package with no directory on disk is not something to walk; -e means the
# list can carry entries the loader could not resolve.
[ -n "${dir}" ] && [ -d "${dir}" ] || continue

find "${dir}" -maxdepth 1 -type f -name '*.go' -print0
done
27 changes: 25 additions & 2 deletions scripts/goimports.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@
set -euo pipefail

# Format Go imports using goimports
# Usage: goimports.sh
# Usage: goimports.sh [project_root]
#
# This used to be `go tool goimports -w .`, which walks the filesystem from the
# working directory and so rewrote every vendored file in the tree whenever one
# was present. It takes its file list from go_files.sh now, like the other
# formatters.

go tool goimports -w .
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
PROJECT_ROOT="${1:-$(pwd)}"

# Through a file rather than `< <(go_files.sh)`: process substitution discards
# the exit status of what it runs, so a failure to produce the list would read
# here as a list of no files, and formatting nothing would look like success.
file_list="$(mktemp)"
trap 'rm -f "${file_list}"' EXIT

"${SCRIPT_DIR}/go_files.sh" "${PROJECT_ROOT}" >"${file_list}"

go_files=()
while IFS= read -r -d '' file; do
go_files+=("${file}")
done <"${file_list}"

if [ ${#go_files[@]} -gt 0 ]; then
go tool goimports -w "${go_files[@]}"
fi