From 69646127df138233e5070930048075e737d58d72 Mon Sep 17 00:00:00 2001 From: Ryan Cragun Date: Thu, 27 Feb 2025 15:56:34 -0700 Subject: [PATCH] fmt: check gosimports during pre-commit hooks (#29520) `gosimports` is the preferred style for module imports and it is enforced via CI. I've found that things often manage to drift so I've taken the liberty to update our pre-commit hook to verify our imports formatting before a change is committed. Along with updating the formatting helper I've also run `make fmt` to resolve any formatting drift that managed to make it into the codebase. Signed-off-by: Ryan Cragun --- builtin/credential/ldap/backend_test.go | 5 +- scripts/go-helper.sh | 88 ++++++++++++------- sdk/database/dbplugin/plugin.go | 3 +- sdk/database/dbplugin/v5/grpc_client.go | 1 - .../dbplugin/v5/grpc_database_plugin_oss.go | 3 +- .../pipeline/internal/cmd/github_list_runs.go | 3 +- .../pkg/generate/enos_dynamic_config_test.go | 3 +- 7 files changed, 63 insertions(+), 43 deletions(-) diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index a8cd549c43..b7f715d89d 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -11,9 +11,6 @@ import ( "testing" "time" - "github.com/hashicorp/vault/sdk/helper/automatedrotationutil" - "github.com/hashicorp/vault/sdk/rotation" - goldap "github.com/go-ldap/ldap/v3" "github.com/go-test/deep" hclog "github.com/hashicorp/go-hclog" @@ -21,10 +18,12 @@ import ( "github.com/hashicorp/vault/helper/namespace" "github.com/hashicorp/vault/helper/testhelpers/ldap" logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical" + "github.com/hashicorp/vault/sdk/helper/automatedrotationutil" "github.com/hashicorp/vault/sdk/helper/ldaputil" "github.com/hashicorp/vault/sdk/helper/policyutil" "github.com/hashicorp/vault/sdk/helper/tokenutil" "github.com/hashicorp/vault/sdk/logical" + "github.com/hashicorp/vault/sdk/rotation" "github.com/mitchellh/mapstructure" ) diff --git a/scripts/go-helper.sh b/scripts/go-helper.sh index 5efaa61e42..1dc1734644 100755 --- a/scripts/go-helper.sh +++ b/scripts/go-helper.sh @@ -8,41 +8,67 @@ set -euo pipefail check_fmt() { echo "==> Checking code formatting..." - declare -a malformed=() - IFS=" " read -r -a files <<< "$(tr '\n' ' ' <<< "$@")" + declare -a malformed_imports=() + declare -a malformed_formatting=() + local failed=0 + IFS=" " read -r -a files <<<"$(tr '\n' ' ' <<<"$@")" if [ -n "${files+set}" ] && [[ "${files[0]}" != "" ]]; then echo "--> Checking changed files..." for file in "${files[@]}"; do if [ ! -f "$file" ]; then - echo "--> $file no longer exists ⚠" + echo "----> $file no longer exists ⚠" continue fi - if echo "$file" | grep -v pb.go | grep -v vendor > /dev/null; then + if echo "$file" | grep -v pb.go | grep -v vendor >/dev/null; then local output + if ! output=$(gosimports -d "$file") || [ "$output" != "" ]; then + echo "----> ${file} ✖ (gosimports)" + malformed_imports+=("$file") + failed=1 + fi if ! output=$(gofumpt -l "$file") || [ "$output" != "" ]; then - echo "--> ${file} ✖" - malformed+=("$file") + echo "----> ${file} ✖ (gofumpt)" + malformed_formatting+=("$file") + failed=1 + fi + + if [ "$failed" == 1 ]; then continue fi fi - echo "--> ${file} ✔" + echo "----> ${file} ✔" done else echo "--> Checking all files..." - IFS=" " read -r -a malformed <<< "$(find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs gofumpt -l)" + IFS=" " read -r -a malformed_imports <<<"$(find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs gosimports -l | tr '\n' ' ')" + IFS=" " read -r -a malformed_formatting <<<"$(find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs gofumpt -l)" fi - if [ "${#malformed[@]}" -ne 0 ] && [ -n "${malformed[0]}" ] ; then - echo "--> The following files need to be reformatted with gofumpt" - printf '%s\n' "${malformed[@]}" + if [ "${#malformed_imports[@]}" -ne 0 ] && [ -n "${malformed_imports[0]}" ]; then + failed=1 + echo "--> The following files need to be reformatted with gosimports" + printf '%s\n' "${malformed_imports[*]}" echo "Run \`make fmt\` to reformat code." - for file in "${malformed[@]}"; do + for file in "${malformed_imports[@]}"; do + gosimports -d "$file" + done + fi + + if [ "${#malformed_formatting[@]}" -ne 0 ] && [ -n "${malformed_formatting[0]}" ]; then + failed=1 + echo "--> The following files need to be reformatted with gofumpt" + printf '%s\n' "${malformed_formatting[@]}" + echo "Run \`make fmt\` to reformat code." + for file in "${malformed_formatting[@]}"; do gofumpt -w "$file" git diff --no-color "$file" done - exit 1 + fi + + if [ $failed == 1 ]; then + return 1 fi } @@ -58,14 +84,14 @@ check_version() { else GO_VERSION=$($GO_CMD version | grep -o 'go[0-9]\+\.[0-9]\+\(\.[0-9]\+\)\?' | tr -d 'go') - IFS="." read -r -a GO_VERSION_ARR <<< "$GO_VERSION" - IFS="." read -r -a GO_VERSION_REQ <<< "$GO_VERSION_MIN" + IFS="." read -r -a GO_VERSION_ARR <<<"$GO_VERSION" + IFS="." read -r -a GO_VERSION_REQ <<<"$GO_VERSION_MIN" if [[ ${GO_VERSION_ARR[0]} -lt ${GO_VERSION_REQ[0]} || - ( ${GO_VERSION_ARR[0]} -eq ${GO_VERSION_REQ[0]} && - ( ${GO_VERSION_ARR[1]} -lt ${GO_VERSION_REQ[1]} || - ( ${GO_VERSION_ARR[1]} -eq ${GO_VERSION_REQ[1]} && ${GO_VERSION_ARR[2]} -lt ${GO_VERSION_REQ[2]} ))) - ]]; then + (${GO_VERSION_ARR[0]} -eq ${GO_VERSION_REQ[0]} && + (${GO_VERSION_ARR[1]} -lt ${GO_VERSION_REQ[1]} || + (${GO_VERSION_ARR[1]} -eq ${GO_VERSION_REQ[1]} && ${GO_VERSION_ARR[2]} -lt ${GO_VERSION_REQ[2]}))) ]] \ + ; then echo "Vault requires go $GO_VERSION_MIN to build; found $GO_VERSION." exit 1 fi @@ -78,19 +104,19 @@ check_version() { mod_download() { while IFS= read -r -d '' mod; do echo "==> Downloading Go modules for $mod to $(go env GOMODCACHE)..." - pushd "$(dirname "$mod")" > /dev/null || (echo "failed to push into module dir" && exit 1) - GOOS=linux GOARCH=amd64 GOPRIVATE=github.com/hashicorp go mod download -x - popd > /dev/null || (echo "failed to pop out of module dir" && exit 1) - done < <(find . -type f -name go.mod -not -path "./tools/pipeline/*" -print0 ) + pushd "$(dirname "$mod")" >/dev/null || (echo "failed to push into module dir" && exit 1) + GOOS=linux GOARCH=amd64 GOPRIVATE=github.com/hashicorp go mod download -x + popd >/dev/null || (echo "failed to pop out of module dir" && exit 1) + done < <(find . -type f -name go.mod -not -path "./tools/pipeline/*" -print0) } # Tidy all the go.mod's defined in the project. mod_tidy() { while IFS= read -r -d '' mod; do echo "==> Tidying $mod..." - pushd "$(dirname "$mod")" > /dev/null || (echo "failed to push into module dir" && exit 1) - GOOS=linux GOARCH=amd64 GOPRIVATE=github.com/hashicorp go mod tidy - popd > /dev/null || (echo "failed to pop out of module dir" && exit 1) + pushd "$(dirname "$mod")" >/dev/null || (echo "failed to push into module dir" && exit 1) + GOOS=linux GOARCH=amd64 GOPRIVATE=github.com/hashicorp go mod tidy + popd >/dev/null || (echo "failed to pop out of module dir" && exit 1) done < <(find . -type f -name go.mod -print0) } @@ -98,20 +124,20 @@ main() { case $1 in mod-download) mod_download - ;; + ;; mod-tidy) mod_tidy - ;; + ;; check-fmt) check_fmt "${@:2}" - ;; + ;; check-version) check_version "$2" - ;; + ;; *) echo "unknown sub-command" >&2 exit 1 - ;; + ;; esac } diff --git a/sdk/database/dbplugin/plugin.go b/sdk/database/dbplugin/plugin.go index 7aacb02b80..717062ac28 100644 --- a/sdk/database/dbplugin/plugin.go +++ b/sdk/database/dbplugin/plugin.go @@ -8,13 +8,12 @@ import ( "fmt" "time" - "google.golang.org/grpc" - "github.com/hashicorp/errwrap" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-plugin" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/pluginutil" + "google.golang.org/grpc" ) // Database is the interface that all database objects must implement. diff --git a/sdk/database/dbplugin/v5/grpc_client.go b/sdk/database/dbplugin/v5/grpc_client.go index 35876216ac..438ab13f6b 100644 --- a/sdk/database/dbplugin/v5/grpc_client.go +++ b/sdk/database/dbplugin/v5/grpc_client.go @@ -12,7 +12,6 @@ import ( "time" "github.com/golang/protobuf/ptypes" - "github.com/hashicorp/vault/sdk/database/dbplugin/v5/proto" "github.com/hashicorp/vault/sdk/helper/pluginutil" "github.com/hashicorp/vault/sdk/logical" diff --git a/sdk/database/dbplugin/v5/grpc_database_plugin_oss.go b/sdk/database/dbplugin/v5/grpc_database_plugin_oss.go index de27710079..437dc99756 100644 --- a/sdk/database/dbplugin/v5/grpc_database_plugin_oss.go +++ b/sdk/database/dbplugin/v5/grpc_database_plugin_oss.go @@ -8,12 +8,11 @@ package dbplugin import ( "context" - "google.golang.org/grpc" - "github.com/hashicorp/go-plugin" "github.com/hashicorp/vault/sdk/database/dbplugin/v5/proto" "github.com/hashicorp/vault/sdk/helper/pluginutil" "github.com/hashicorp/vault/sdk/logical" + "google.golang.org/grpc" ) // GRPCClient (Vault CE edition) initializes and returns a gRPCClient with Database and diff --git a/tools/pipeline/internal/cmd/github_list_runs.go b/tools/pipeline/internal/cmd/github_list_runs.go index 806ccc3c8d..f8d5662529 100644 --- a/tools/pipeline/internal/cmd/github_list_runs.go +++ b/tools/pipeline/internal/cmd/github_list_runs.go @@ -12,9 +12,8 @@ import ( "time" ghclient "github.com/google/go-github/v68/github" - "github.com/spf13/cobra" - "github.com/hashicorp/vault/tools/pipeline/internal/pkg/github" + "github.com/spf13/cobra" ) var listGithubWorkflowRuns = &github.ListWorkflowRunsReq{} diff --git a/tools/pipeline/internal/pkg/generate/enos_dynamic_config_test.go b/tools/pipeline/internal/pkg/generate/enos_dynamic_config_test.go index 5f3b8593b7..a0c4dda582 100644 --- a/tools/pipeline/internal/pkg/generate/enos_dynamic_config_test.go +++ b/tools/pipeline/internal/pkg/generate/enos_dynamic_config_test.go @@ -10,9 +10,8 @@ import ( "slices" "testing" - "github.com/stretchr/testify/require" - "github.com/hashicorp/vault/tools/pipeline/internal/pkg/releases" + "github.com/stretchr/testify/require" ) var testAPIVersions = []string{