Compare commits

...

1 Commits

Author SHA1 Message Date
Rodin 0619e2b617 fix: address review feedback on check-deps script
- Accept import paths starting with digit (e.g., 9fans.net/go) by
  relaxing filter from ^[a-zA-Z] to ^[[:alnum:]]
- Fix misleading comment: loop uses Bash process substitution, not POSIX
- Remove redundant \$ from grep regex (literal $ unreachable)
- Capture stderr separately in go list to avoid mixing errors with output
- Clarify CONVENTIONS.md wording on transitive vs direct deps

Reviewed-by: sonnet-review-bot
Reviewed-by: gpt-review-bot
2026-05-10 14:09:03 -07:00
2 changed files with 13 additions and 7 deletions
+1 -1
View File
@@ -14,7 +14,7 @@
**Any import not in this table or the Go standard library is forbidden.**
Transitive dependencies of approved packages are automatically allowed.
Only *direct* dependencies (listed in go.mod without `// indirect`) are checked against this allowlist. Transitive dependencies pulled in by approved packages are implicitly allowed.
To request a new dependency:
1. Open a PR that ONLY updates this table
+12 -6
View File
@@ -23,7 +23,8 @@ if [ ! -f "$CONVENTIONS_FILE" ]; then
exit 1
fi
# Parse approved packages from CONVENTIONS.md table using awk (POSIX-compatible)
# Parse approved packages from CONVENTIONS.md table
# Note: uses Bash process substitution (< <(...)) for the loop
# Format: | `package` | use case | scope |
declare -A ALLOWED_PROD=()
declare -A ALLOWED_TEST=()
@@ -33,7 +34,8 @@ while IFS= read -r line; do
pkg=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]*`|`[[:space:]]*$/, "", $2); print $2}')
scope=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]+|[[:space:]]+$/, "", $4); print tolower($4)}')
if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[a-zA-Z] ]]; then
# Accept packages starting with letter or digit (e.g., 9fans.net/go)
if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[[:alnum:]] ]]; then
if [[ "$scope" == *"test"* ]]; then
ALLOWED_TEST["$pkg"]=1
else
@@ -69,8 +71,12 @@ matches_allowlist() {
}
# Get direct module dependencies from go.mod
DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>&1) || {
echo "❌ Failed to list dependencies: $DIRECT_IMPORTS"
# Capture stderr separately to avoid mixing error messages with package list
GO_LIST_STDERR=$(mktemp)
trap 'rm -f "$GO_LIST_STDERR"' EXIT
DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>"$GO_LIST_STDERR") || {
echo "❌ Failed to list dependencies:"
cat "$GO_LIST_STDERR"
exit 1
}
DIRECT_IMPORTS=$(echo "$DIRECT_IMPORTS" | grep -v '^$' || true)
@@ -106,8 +112,8 @@ PROD_IMPORTS=$(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./.
TEST_ONLY_IN_PROD=""
for test_pkg in "${!ALLOWED_TEST[@]}"; do
# Use word-boundary matching: exact match or followed by /
if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|\$|$)"; then
# Match exact package or subpackages (pkg or pkg/...)
if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|$)"; then
TEST_ONLY_IN_PROD="${TEST_ONLY_IN_PROD} - ${test_pkg} (marked 'test only' but used in production code)"$'\n'
fi
done