From b0fb3b14206c63c01041fe3f561b147a3d41de74 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Mon, 10 Jul 2023 11:49:47 -0500 Subject: [PATCH] pre-commit hook for formatting backend code (#21682) * Add backend format linting to pre-commit hook By taking a slight penalty with each commit, we can ensure that contributors follow the format behavior by default (if they run hooks), making accidental PRs without proper formatting less likely. Additionally, fix gofmtcheck to align with the Makefile, fixing the corresponding fmtcheck target for use with the hook. Signed-off-by: Alexander Scheel * Fix formatting errors Signed-off-by: Alexander Scheel --------- Signed-off-by: Alexander Scheel --- .hooks/pre-commit | 11 ++++++++++- Makefile | 3 +-- builtin/logical/pki/path_acme_test.go | 20 ++++++++++---------- scripts/gofmtcheck.sh | 6 +++--- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/.hooks/pre-commit b/.hooks/pre-commit index f40519e53516..18857f33f3a6 100755 --- a/.hooks/pre-commit +++ b/.hooks/pre-commit @@ -35,7 +35,7 @@ block() { # Add all check functions to this space separated list. # They are executed in this order (see end of file). -CHECKS="ui_lint" +CHECKS="ui_lint backend_lint" # Run ui linter if changes in that dir detected. ui_lint() { @@ -60,6 +60,15 @@ ui_lint() { $LINTER || block "UI lint failed" } +backend_lint() { + # Silently succeed if no changes staged for Go code files. + if git diff --name-only --cached --exit-code -- '*.go'; then + return 0 + fi + + ./scripts/gofmtcheck.sh || block "Backend linting failed; run 'make fmt' to fix." +} + for CHECK in $CHECKS; do # Force each check into a subshell to avoid crosstalk. ( $CHECK ) || exit $? diff --git a/Makefile b/Makefile index c468d6abd928..1418c7b1c95c 100644 --- a/Makefile +++ b/Makefile @@ -238,8 +238,7 @@ proto: bootstrap protoc-go-inject-tag -input=./helper/identity/mfa/types.pb.go fmtcheck: - @true -#@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'" + @sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'" fmt: ci-bootstrap find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs go run mvdan.cc/gofumpt -w diff --git a/builtin/logical/pki/path_acme_test.go b/builtin/logical/pki/path_acme_test.go index d9a2c5743700..335200f903af 100644 --- a/builtin/logical/pki/path_acme_test.go +++ b/builtin/logical/pki/path_acme_test.go @@ -1021,23 +1021,23 @@ func TestACMESubjectFieldsAndExtensionsIgnored(t *testing.T) { acmeClient := getAcmeClientForCluster(t, cluster, directory, nil) cr := &x509.CertificateRequest{ Subject: pkix.Name{CommonName: domains[0], OrganizationalUnit: []string{"DadgarCorp IT"}}, - DNSNames: domains, - } + DNSNames: domains, + } cert := doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr) t.Logf("Got certificate: %v", cert) require.Empty(t, cert.Subject.OrganizationalUnit) - // Use the default sign-verbatim policy and ensure extension does not get set. - domains = []string{"no-ext.dadgarcorp.com"} + // Use the default sign-verbatim policy and ensure extension does not get set. + domains = []string{"no-ext.dadgarcorp.com"} extension, err := certutil.CreateDeltaCRLIndicatorExt(12345) require.NoError(t, err) - cr = &x509.CertificateRequest{ - Subject: pkix.Name{CommonName: domains[0]}, - DNSNames: domains, + cr = &x509.CertificateRequest{ + Subject: pkix.Name{CommonName: domains[0]}, + DNSNames: domains, ExtraExtensions: []pkix.Extension{extension}, - } - cert = doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr) - t.Logf("Got certificate: %v", cert) + } + cert = doACMEForCSRWithDNS(t, dns, acmeClient, domains, cr) + t.Logf("Got certificate: %v", cert) for _, ext := range cert.Extensions { require.False(t, ext.Id.Equal(certutil.DeltaCRLIndicatorOID)) } diff --git a/scripts/gofmtcheck.sh b/scripts/gofmtcheck.sh index 5c58f178558b..886f1968db30 100755 --- a/scripts/gofmtcheck.sh +++ b/scripts/gofmtcheck.sh @@ -5,9 +5,9 @@ echo "==> Checking that code complies with gofmt requirements..." -gofmt_files=$(gofmt -l `find . -name '*.go' | grep -v vendor`) -if [[ -n ${gofmt_files} ]]; then - echo 'gofmt needs running on the following files:' +gofmt_files="$(find . -name '*.go' | grep -v pb.go | grep -v vendor | xargs go run mvdan.cc/gofumpt -l)" +if [[ -n "${gofmt_files}" ]]; then + echo 'gofumpt needs running on the following files:' echo "${gofmt_files}" echo "You can use the command: \`make fmt\` to reformat code." exit 1