Bump github.com/golangci/golangci-lint from 1.54.2 to 1.55.1 in /inte…

…rnal/tools (#190)

.golangci.yml

@@ -69,13 +69,15 @@ linters:
     - golint            # deprecated by Go team
     - gomnd             # some unnamed constants are okay
     - ifshort           # deprecated by author
+    - inamedparam       # named params in interface signatures are not always necessary
     - interfacer        # deprecated by author
     - ireturn           # "accept interfaces, return structs" isn't ironclad
     - lll               # don't want hard limits for line length
     - maintidx          # covered by gocyclo
     - maligned          # readability trumps efficient struct packing
     - nlreturn          # generous whitespace violates house style
     - nosnakecase       # deprecated in https://github.com/golangci/golangci-lint/pull/3065
+    - protogetter       # lots of false positives: can't use getter to check if field is present
     - rowserrcheck      # no SQL code in protocompile
     - scopelint         # deprecated by author
     - sqlclosecheck     # no SQL code in protocompile

Makefile

@@ -54,6 +54,7 @@ clean: ## Delete intermediate build artifacts
 .PHONY: test
 test: build ## Run unit tests
 	$(GO) test -race -cover ./...
+	cd internal/benchmarks && SKIP_DOWNLOAD_GOOGLEAPIS=true $(GO) test -race -cover ./...
 
 .PHONY: benchmarks
 benchmarks: build ## Run benchmarks

ast/ast_roundtrip_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/bufbuild/protocompile/ast"
 	"github.com/bufbuild/protocompile/parser"
@@ -38,15 +39,13 @@ func TestASTRoundTrips(t *testing.T) {
 			t.Run(path, func(t *testing.T) {
 				t.Parallel()
 				data, err := os.ReadFile(path)
-				if !assert.Nil(t, err, "%v", err) {
-					return
-				}
+				require.NoError(t, err)
 				testASTRoundTrip(t, path, data)
 			})
 		}
 		return nil
 	})
-	assert.Nil(t, err, "%v", err)
+	assert.NoError(t, err) //nolint:testifylint // we want to continue even if err!=nil
 	t.Run("empty", func(t *testing.T) {
 		t.Parallel()
 		testASTRoundTrip(t, "empty", []byte(`
@@ -58,15 +57,12 @@ func TestASTRoundTrips(t *testing.T) {
 func testASTRoundTrip(t *testing.T, path string, data []byte) {
 	filename := filepath.Base(path)
 	root, err := parser.Parse(filename, bytes.NewReader(data), reporter.NewHandler(nil))
-	if !assert.Nil(t, err) {
-		return
-	}
+	require.NoError(t, err)
 	var buf bytes.Buffer
 	err = printAST(&buf, root)
-	if assert.Nil(t, err, "%v", err) {
-		// see if file survived round trip!
-		assert.Equal(t, string(data), buf.String())
-	}
+	require.NoError(t, err)
+	// see if file survived round trip!
+	assert.Equal(t, string(data), buf.String())
 }
 
 // printAST prints the given AST node to the given output. This operation

ast/items_test.go

@@ -38,15 +38,13 @@ func TestItems(t *testing.T) {
 			t.Run(path, func(t *testing.T) {
 				t.Parallel()
 				data, err := os.ReadFile(path)
-				if !assert.Nil(t, err, "%v", err) {
-					return
-				}
+				require.NoError(t, err)
 				testItemsSequence(t, path, data)
 			})
 		}
 		return nil
 	})
-	assert.Nil(t, err, "%v", err)
+	assert.NoError(t, err) //nolint:testifylint // we want to continue even if err!=nil
 	t.Run("empty", func(t *testing.T) {
 		t.Parallel()
 		testItemsSequence(t, "empty", []byte(`
@@ -58,9 +56,7 @@ func TestItems(t *testing.T) {
 func testItemsSequence(t *testing.T, path string, data []byte) {
 	filename := filepath.Base(path)
 	root, err := parser.Parse(filename, bytes.NewReader(data), reporter.NewHandler(nil))
-	if !assert.Nil(t, err) {
-		return
-	}
+	require.NoError(t, err)
 	tokens := leavesAsSlice(root)
 	require.NoError(t, err)
 	// Make sure sequence matches the actual leaves in the tree

ast/tokens_test.go

@@ -38,15 +38,13 @@ func TestTokens(t *testing.T) {
 			t.Run(path, func(t *testing.T) {
 				t.Parallel()
 				data, err := os.ReadFile(path)
-				if !assert.Nil(t, err, "%v", err) {
-					return
-				}
+				require.NoError(t, err)
 				testTokensSequence(t, path, data)
 			})
 		}
 		return nil
 	})
-	assert.Nil(t, err, "%v", err)
+	assert.NoError(t, err) //nolint:testifylint // we want to continue even if err!=nil
 	t.Run("empty", func(t *testing.T) {
 		t.Parallel()
 		testTokensSequence(t, "empty", []byte(`
@@ -58,9 +56,7 @@ func TestTokens(t *testing.T) {
 func testTokensSequence(t *testing.T, path string, data []byte) {
 	filename := filepath.Base(path)
 	root, err := parser.Parse(filename, bytes.NewReader(data), reporter.NewHandler(nil))
-	if !assert.Nil(t, err) {
-		return
-	}
+	require.NoError(t, err)
 	tokens := leavesAsSlice(root)
 	require.NoError(t, err)
 	// Make sure sequence matches the actual leaves in the tree

compiler_test.go

@@ -45,9 +45,7 @@ func TestParseFilesMessageComments(t *testing.T) {
 	}
 	ctx := context.Background()
 	files, err := comp.Compile(ctx, "internal/testdata/desc_test1.proto")
-	if !assert.Nil(t, err, "%v", err) {
-		t.FailNow()
-	}
+	require.NoError(t, err)
 	comments := ""
 	expected := " Comment for TestMessage\n"
 	for _, fd := range files {
@@ -78,9 +76,7 @@ func TestParseFilesWithImportsNoImportPath(t *testing.T) {
 	}
 	ctx := context.Background()
 	protos, err := comp.Compile(ctx, relFilePaths...)
-	if !assert.Nil(t, err, "%v", err) {
-		t.FailNow()
-	}
+	require.NoError(t, err)
 	assert.Equal(t, len(relFilePaths), len(protos))
 }
 
@@ -125,7 +121,7 @@ func TestParseFilesWithDependencies(t *testing.T) {
 			}),
 		}
 		_, err := compiler.Compile(ctx, "test.proto")
-		assert.Nil(t, err, "%v", err)
+		require.NoError(t, err)
 	})
 	t.Run("DependencyIncludedProto", func(t *testing.T) {
 		t.Parallel()
@@ -139,7 +135,7 @@ func TestParseFilesWithDependencies(t *testing.T) {
 			})),
 		}
 		_, err := compiler.Compile(ctx, "test.proto")
-		assert.Nil(t, err, "%v", err)
+		require.NoError(t, err)
 	})
 
 	// Establish that we *can not* parse the source file if the resolver
@@ -149,7 +145,7 @@ func TestParseFilesWithDependencies(t *testing.T) {
 		// Create a dependency-UNaware parser.
 		compiler := Compiler{Resolver: baseResolver}
 		_, err := compiler.Compile(ctx, "test.proto")
-		assert.NotNil(t, err, "expected parse to fail")
+		require.Error(t, err, "expected parse to fail")
 	})
 
 	t.Run("NoDependencies", func(t *testing.T) {
@@ -171,7 +167,7 @@ func TestParseFilesWithDependencies(t *testing.T) {
 			}),
 		}
 		_, err := compiler.Compile(ctx, "test.proto")
-		assert.Nil(t, err)
+		require.NoError(t, err)
 	})
 }
 
@@ -184,14 +180,10 @@ func findAndLink(t *testing.T, filename string, fdset *descriptorpb.FileDescript
 			for _, dep := range fd.GetDependency() {
 				depDesc, _ := findAndLink(t, dep, fdset, soFar)
 				err := soFar.RegisterFile(depDesc)
-				if !assert.NoError(t, err) {
-					t.FailNow()
-				}
+				require.NoError(t, err)
 			}
 			desc, err := protodesc.NewFile(fd, soFar)
-			if !assert.NoError(t, err) {
-				t.FailNow()
-			}
+			require.NoError(t, err)
 			return desc, fd
 		}
 	}
@@ -217,7 +209,7 @@ message Foo {
 	}
 	ctx := context.Background()
 	fds, err := compiler.Compile(ctx, "test.proto")
-	assert.Nil(t, err)
+	require.NoError(t, err)
 
 	field := fds[0].Messages().Get(0).Fields().Get(0)
 	comment := fds[0].SourceLocations().ByDescriptor(field).LeadingComments
@@ -247,9 +239,7 @@ message Foo {
 	}
 	ctx := context.Background()
 	fds, err := compiler.Compile(ctx, "test.proto")
-	if !assert.Nil(t, err, "%v", err) {
-		t.FailNow()
-	}
+	require.NoError(t, err)
 
 	ext := fds[0].Extensions().ByName("foo")
 	md := fds[0].Messages().Get(0)

internal/benchmarks/benchmark_test.go

@@ -57,6 +57,8 @@ const (
 var (
 	protocPath string
 
+	skipDownload = os.Getenv("SKIP_DOWNLOAD_GOOGLEAPIS") == "true"
+
 	googleapisURI     = fmt.Sprintf("https://github.com/googleapis/googleapis/archive/%s.tar.gz", googleapisCommit)
 	googleapisDir     string
 	googleapisSources []string
@@ -88,44 +90,46 @@ func TestMain(m *testing.M) {
 	// After this point, we can set stat and return instead of directly calling os.Exit.
 	// That allows deferred functions to execute, to perform cleanup, before exiting.
 
-	dir, err := os.MkdirTemp("", "testdownloads")
-	if err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "Could not create temporary directory: %v\n", err)
-		stat = 1
-		return
-	}
-	defer func() {
-		if err := os.RemoveAll(dir); err != nil {
-			_, _ = fmt.Fprintf(os.Stderr, "Failed to cleanup temp directory %s: %v\n", dir, err)
+	if !skipDownload {
+		dir, err := os.MkdirTemp("", "testdownloads")
+		if err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "Could not create temporary directory: %v\n", err)
+			stat = 1
+			return
 		}
-	}()
+		defer func() {
+			if err := os.RemoveAll(dir); err != nil {
+				_, _ = fmt.Fprintf(os.Stderr, "Failed to cleanup temp directory %s: %v\n", dir, err)
+			}
+		}()
 
-	if err := downloadAndExpand(googleapisURI, dir); err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "Failed to download and expand googleapis: %v\n", err)
-		stat = 1
-		return
-	}
+		if err := downloadAndExpand(googleapisURI, dir); err != nil {
+			_, _ = fmt.Fprintf(os.Stderr, "Failed to download and expand googleapis: %v\n", err)
+			stat = 1
+			return
+		}
 
-	googleapisDir = filepath.Join(dir, fmt.Sprintf("googleapis-%s", googleapisCommit)) + "/"
-	var sourceSize int64
-	err = filepath.Walk(googleapisDir, func(path string, info fs.FileInfo, err error) error {
+		googleapisDir = filepath.Join(dir, fmt.Sprintf("googleapis-%s", googleapisCommit)) + "/"
+		var sourceSize int64
+		err = filepath.Walk(googleapisDir, func(path string, info fs.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			if !info.IsDir() && strings.HasSuffix(path, ".proto") {
+				relPath := strings.TrimPrefix(path, googleapisDir)
+				googleapisSources = append(googleapisSources, relPath)
+				sourceSize += info.Size()
+			}
+			return nil
+		})
 		if err != nil {
-			return err
+			_, _ = fmt.Fprintf(os.Stderr, "Failed to enumerate googleapis source files: %v\n", err)
+			stat = 1
+			return
 		}
-		if !info.IsDir() && strings.HasSuffix(path, ".proto") {
-			relPath := strings.TrimPrefix(path, googleapisDir)
-			googleapisSources = append(googleapisSources, relPath)
-			sourceSize += info.Size()
-		}
-		return nil
-	})
-	if err != nil {
-		_, _ = fmt.Fprintf(os.Stderr, "Failed to enumerate googleapis source files: %v\n", err)
-		stat = 1
-		return
+		sort.Strings(googleapisSources)
+		fmt.Printf("%d total source files found in googleapis (%d bytes).\n", len(googleapisSources), sourceSize)
 	}
-	sort.Strings(googleapisSources)
-	fmt.Printf("%d total source files found in googleapis (%d bytes).\n", len(googleapisSources), sourceSize)
 
 	stat = m.Run()
 }
@@ -321,7 +325,7 @@ func benchmarkGoogleapisProtoparse(b *testing.B, factory func() *protoparse.Pars
 			j = chunkEnd
 			total += len(chunks[ch])
 		}
-		require.Equal(b, total, len(googleapisSources))
+		require.Len(b, googleapisSources, total)
 		var wg sync.WaitGroup
 		results := make([][]*desc.FileDescriptor, par)
 		errors := make([]error, par)
@@ -436,6 +440,9 @@ func writeToNull(b *testing.B, fds *descriptorpb.FileDescriptorSet) {
 }
 
 func TestGoogleapisProtocompileResultMemory(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	c := protocompile.Compiler{
 		Resolver: protocompile.WithStandardImports(&protocompile.SourceResolver{
 			ImportPaths: []string{googleapisDir},
@@ -448,6 +455,9 @@ func TestGoogleapisProtocompileResultMemory(t *testing.T) {
 }
 
 func TestGoogleapisProtocompileResultMemoryNoSourceInfo(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	c := protocompile.Compiler{
 		Resolver: protocompile.WithStandardImports(&protocompile.SourceResolver{
 			ImportPaths: []string{googleapisDir},
@@ -460,6 +470,9 @@ func TestGoogleapisProtocompileResultMemoryNoSourceInfo(t *testing.T) {
 }
 
 func TestGoogleapisProtocompileASTMemory(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	var asts []*ast.FileNode
 	for _, file := range googleapisSources {
 		func() {
@@ -480,6 +493,9 @@ func TestGoogleapisProtocompileASTMemory(t *testing.T) {
 }
 
 func TestGoogleapisProtoparseResultMemory(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	p := protoparse.Parser{
 		ImportPaths:           []string{googleapisDir},
 		IncludeSourceCodeInfo: true,
@@ -490,6 +506,9 @@ func TestGoogleapisProtoparseResultMemory(t *testing.T) {
 }
 
 func TestGoogleapisProtoparseResultMemoryNoSourceInfo(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	p := protoparse.Parser{
 		ImportPaths:           []string{googleapisDir},
 		IncludeSourceCodeInfo: false,
@@ -500,6 +519,9 @@ func TestGoogleapisProtoparseResultMemoryNoSourceInfo(t *testing.T) {
 }
 
 func TestGoogleapisProtoparseASTMemory(t *testing.T) {
+	if skipDownload {
+		t.Skip()
+	}
 	p := protoparse.Parser{
 		IncludeSourceCodeInfo: true,
 	}

internal/benchmarks/measure_test.go

@@ -148,8 +148,8 @@ func TestNumBuckets(t *testing.T) {
 		// power of 2
 		assert.Equal(t, 1, bits.OnesCount(uint(b)))
 		// that fits given size (each bucket holds 8 entries)
-		assert.True(t, b*4 < sz)
-		assert.True(t, b*8 >= sz)
+		assert.Less(t, b*4, sz)
+		assert.GreaterOrEqual(t, b*8, sz)
 	}
 	check(7364)
 	check(1234567)