Add an option to override the protovalidate validator (#23)

Validator instances are not cheap, so by default use the global
validator instead of allocating a new one in the server handlers. Add
options to allow users to override the validator if needed.

The other changes are code hygiene - updating to support the last two
versions of Go and bumping golangci-lint to the latest v2.

Author: pkwarren

.github/workflows/ci.yaml

@@ -15,7 +15,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go-version: [1.23.x, 1.24.x, 1.25.x]
+        go-version: [1.25.x, 1.26.x]
     steps:
       - name: Checkout Code
         uses: actions/checkout@v6
@@ -32,5 +32,5 @@ jobs:
         # conflicting guidance, run only on the most recent supported version.
         # For the same reason, only check generated code on the most recent
         # supported version.
-        if: matrix.go-version == '1.25.x'
+        if: matrix.go-version == '1.26.x'
         run: make checkgenerate && make lint

.golangci.yml

@@ -1,25 +1,6 @@
-linters-settings:
-  errcheck:
-    check-type-assertions: true
-  forbidigo:
-    forbid:
-      - '^fmt\.Print'
-      - '^log\.'
-      - '^print$'
-      - '^println$'
-      - '^panic$'
-  godox:
-    # TODO, OPT, etc. comments are fine to commit. Use FIXME comments for
-    # temporary hacks, and use godox to prevent committing them.
-    keywords: [FIXME]
-  varnamelen:
-    ignore-decls:
-      - T any
-      - i int
-      - wg sync.WaitGroup
-      - id string
+version: "2"
 linters:
-  enable-all: true
+  default: all
   disable:
     - cyclop            # covered by gocyclo
     - depguard          # unnecessary for small libraries
@@ -28,78 +9,108 @@ linters:
     - funlen            # rely on code review to limit function length
     - gochecknoglobals  # many exceptions
     - gocognit          # dubious "cognitive overhead" quantification
-    - gofumpt           # prefer standard gofmt
-    - goimports         # rely on gci instead
-    - gomnd             # some unnamed constants are okay
     - inamedparam       # not standard style
     - interfacebloat    # many exceptions
     - ireturn           # "accept interfaces, return structs" isn't ironclad
     - lll               # don't want hard limits for line length
     - maintidx          # covered by gocyclo
     - nilnil            # allow this
     - nlreturn          # generous whitespace violates house style
+    - noinlineerr       # excess scope violates house style
     - testifylint       # does not want us to use assert
     - testpackage       # internal tests are fine
     - thelper           # we want to print out the whole stack
     - wrapcheck         # don't _always_ need to wrap errors
     - wsl               # generous whitespace violates house style
-issues:
-  exclude-dirs-use-default: false
-  exclude-rules:
-    - linters:
-        - revive
-      path: check/client.go
-      test: "CheckCallOption"
-    - linters:
-        - revive
-      path: check/check_service_handler.go
-      test: "CheckServiceHandlerOption"
-    - linters:
-        - exhaustive
-      path: option/options.go
-      text: "reflect.Pointer|reflect.Ptr"
-    - linters:
-        - gocritic
-      path: check/file.go
-      text: "commentFormatting"
-    - linters:
-        - gocritic
-      path: check/location.go
-      text: "commentFormatting"
-    - linters:
-        - unparam
-      path: check/category_spec.go
-    - linters:
-        - unparam
-      path: check/annotation.go
-    - linters:
-        - unparam
-      path: check/response.go
-    - linters:
-        - unparam
-      path: info/plugin_info.go
-    - linters:
-        - varnamelen
-      path: check/internal/example
-    - linters:
-        - dupl
-      path: check/checkutil/breaking.go
-    - linters:
-        - varnamelen
-      path: check/checkutil/breaking.go
-    - linters:
-        - varnamelen
-      path: check/checkutil/lint.go
-    - linters:
-        - varnamelen
-      path: check/checkutil/util.go
-    - linters:
-        - varnamelen
-      path: internal/pkg/xslices/xslices.go
-    - linters:
-        - revive
-      path: internal/pkg/compare/compare.go
-    - linters:
-        - gosec
-      path: check/checktest/checktest.go
-      text: "G115:"
+    - wsl_v5            # generous whitespace violates house style
+  settings:
+    errcheck:
+      check-type-assertions: true
+    forbidigo:
+      forbid:
+        - pattern: ^fmt\.Print
+        - pattern: ^log\.
+        - pattern: ^print$
+        - pattern: ^println$
+        - pattern: ^panic$
+    godox:
+      # TODO, OPT, etc. comments are fine to commit. Use FIXME comments for
+      # temporary hacks, and use godox to prevent committing them.
+      keywords:
+        - FIXME
+    varnamelen:
+      ignore-decls:
+        - T any
+        - i int
+        - wg sync.WaitGroup
+        - id string
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    rules:
+      - linters:
+          - revive
+        path: check/client.go
+        text: CheckCallOption
+      - linters:
+          - revive
+        path: check/check_service_handler.go
+        text: stutter
+      - linters:
+          - exhaustive
+        path: option/options.go
+        text: reflect.Pointer|reflect.Ptr
+      - linters:
+          - gocritic
+        path: check/file.go
+        text: commentFormatting
+      - linters:
+          - gocritic
+        path: check/location.go
+        text: commentFormatting
+      - linters:
+          - unparam
+        path: check/category_spec.go
+      - linters:
+          - unparam
+        path: check/annotation.go
+      - linters:
+          - unparam
+        path: check/response.go
+      - linters:
+          - unparam
+        path: info/plugin_info.go
+      - linters:
+          - varnamelen
+        path: check/internal/example
+      - linters:
+          - dupl
+        path: check/checkutil/breaking.go
+      - linters:
+          - varnamelen
+        path: check/checkutil/breaking.go
+      - linters:
+          - varnamelen
+        path: check/checkutil/lint.go
+      - linters:
+          - varnamelen
+        path: check/checkutil/util.go
+      - linters:
+          - varnamelen
+        path: internal/pkg/xslices/xslices.go
+      - linters:
+          - revive
+        path: internal/pkg/compare/compare.go
+      - linters:
+          - gosec
+        path: check/checktest/checktest.go
+        text: 'G115:'
+formatters:
+  enable:
+    - gci
+  exclusions:
+    generated: lax

Makefile

@@ -12,11 +12,8 @@ export GOBIN := $(abspath $(BIN))
 COPYRIGHT_YEARS := 2024-2025
 LICENSE_IGNORE := --ignore testdata/
 
-BUF_VERSION := v1.50.0
-GO_MOD_GOTOOLCHAIN := go1.23.5
-GOLANGCI_LINT_VERSION := v1.63.4
-# https://github.com/golangci/golangci-lint/issues/4837
-GOLANGCI_LINT_GOTOOLCHAIN := $(GO_MOD_GOTOOLCHAIN)
+BUF_VERSION := v1.67.0
+GOLANGCI_LINT_VERSION := v2.11.4
 #GO_GET_PKGS :=
 
 .PHONY: help
@@ -48,11 +45,11 @@ install: ## Install all binaries
 .PHONY: lint
 lint: $(BIN)/golangci-lint ## Lint
 	go vet ./...
-	GOTOOLCHAIN=$(GOLANGCI_LINT_GOTOOLCHAIN) golangci-lint run --modules-download-mode=readonly --timeout=3m0s
+	golangci-lint run --modules-download-mode=readonly --timeout=3m0s
 
 .PHONY: lintfix
 lintfix: $(BIN)/golangci-lint ## Automatically fix some lint errors
-	GOTOOLCHAIN=$(GOLANGCI_LINT_GOTOOLCHAIN) golangci-lint run --fix --modules-download-mode=readonly --timeout=3m0s
+	golangci-lint run --fix --modules-download-mode=readonly --timeout=3m0s
 
 .PHONY: generate
 generate: $(BIN)/buf $(BIN)/protoc-gen-pluginrpc-go $(BIN)/license-header ## Regenerate code and licenses
@@ -65,7 +62,6 @@ generate: $(BIN)/buf $(BIN)/protoc-gen-pluginrpc-go $(BIN)/license-header ## Reg
 
 .PHONY: upgrade
 upgrade: ## Upgrade dependencies
-	go mod edit -toolchain=$(GO_MOD_GOTOOLCHAIN)
 	go get -u -t ./... $(GO_GET_PKGS)
 	go mod tidy -v
 
@@ -84,7 +80,7 @@ $(BIN)/license-header: Makefile
 
 $(BIN)/golangci-lint: Makefile
 	@mkdir -p $(@D)
-	GOTOOLCHAIN=$(GOLANGCI_LINT_GOTOOLCHAIN) go install github.com/golangci/golangci-lint/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
+	go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION)
 
 .PHONY: $(BIN)/protoc-gen-pluginrpc-go
 $(BIN)/protoc-gen-pluginrpc-go:

check/category.go

@@ -35,7 +35,7 @@ type Category interface {
 	//
 	// This uniquely identifies the Category.
 	ID() string
-	// A user-displayable purpose of the category.
+	// Purpose is a user-displayable purpose of the category.
 	//
 	// Always present.
 	Purpose() string

check/check_service_handler.go

@@ -55,6 +55,14 @@ func CheckServiceHandlerWithParallelism(parallelism int) CheckServiceHandlerOpti
 	}
 }
 
+// CheckServiceHandlerWithValidator allows overriding the default validator used by the handler.
+// By default, [protovalidate.GlobalValidator] is used.
+func CheckServiceHandlerWithValidator(validator protovalidate.Validator) CheckServiceHandlerOption {
+	return func(checkServiceHandlerOptions *checkServiceHandlerOptions) {
+		checkServiceHandlerOptions.validator = validator
+	}
+}
+
 // *** PRIVATE ***
 
 type checkServiceHandler struct {
@@ -118,9 +126,9 @@ func newCheckServiceHandler(spec *Spec, options ...CheckServiceHandlerOption) (*
 		ruleIDToRule[id] = rule
 		ruleIDToIndex[id] = i
 	}
-	validator, err := protovalidate.New()
-	if err != nil {
-		return nil, err
+	validator := checkServiceHandlerOptions.validator
+	if validator == nil {
+		validator = protovalidate.GlobalValidator
 	}
 	return &checkServiceHandler{
 		spec:                 spec,
@@ -300,6 +308,7 @@ func (c *checkServiceHandler) getCategoriesAndNextPageToken(pageSize int, pageTo
 
 type checkServiceHandlerOptions struct {
 	parallelism int
+	validator   protovalidate.Validator
 }
 
 func newCheckServiceHandlerOptions() *checkServiceHandlerOptions {

check/check_service_handler_test.go

@@ -15,7 +15,6 @@
 package check
 
 import (
-	"context"
 	"testing"
 
 	checkv1 "buf.build/gen/go/bufbuild/bufplugin/protocolbuffers/go/buf/plugin/check/v1"
@@ -39,7 +38,7 @@ func TestCheckServiceHandlerUniqueFiles(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = checkServiceHandler.Check(
-		context.Background(),
+		t.Context(),
 		&checkv1.CheckRequest{
 			FileDescriptors: []*descriptorv1.FileDescriptor{
 				{
@@ -62,7 +61,7 @@ func TestCheckServiceHandlerUniqueFiles(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = checkServiceHandler.Check(
-		context.Background(),
+		t.Context(),
 		&checkv1.CheckRequest{
 			FileDescriptors: []*descriptorv1.FileDescriptor{
 				{
@@ -85,7 +84,7 @@ func TestCheckServiceHandlerUniqueFiles(t *testing.T) {
 	require.Equal(t, pluginrpc.CodeInvalidArgument, pluginrpcError.Code())
 
 	_, err = checkServiceHandler.Check(
-		context.Background(),
+		t.Context(),
 		&checkv1.CheckRequest{
 			FileDescriptors: []*descriptorv1.FileDescriptor{
 				{
@@ -129,7 +128,7 @@ func TestCheckServiceHandlerNoSourceCodeInfo(t *testing.T) {
 	require.NoError(t, err)
 
 	_, err = checkServiceHandler.Check(
-		context.Background(),
+		t.Context(),
 		&checkv1.CheckRequest{
 			FileDescriptors: []*descriptorv1.FileDescriptor{
 				{

check/checktest/checktest.go

@@ -76,7 +76,7 @@ type CheckTest struct {
 //   - Call Check on the Client.
 //   - Compare the resulting Annotations with the ExpectedAnnotations, failing if there is a mismatch.
 func (c CheckTest) Run(t *testing.T) {
-	ctx := context.Background()
+	ctx := t.Context()
 
 	require.NotNil(t, c.Request)
 	require.NotNil(t, c.Spec)