Add configuration option to allow comment driven ignores for lint (bu…

…fbuild#73)

internal/buf/bufanalysis/bufanalysistesting/bufanalysistesting.go

@@ -15,6 +15,7 @@
 package bufanalysistesting
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/bufbuild/buf/internal/buf/bufanalysis"
@@ -114,7 +115,7 @@ func AssertFileAnnotationsEqual(
 ) {
 	expected = normalizeFileAnnotations(t, expected)
 	actual = normalizeFileAnnotations(t, actual)
-	assert.Equal(t, len(expected), len(actual))
+	assert.Equal(t, len(expected), len(actual), fmt.Sprint(actual))
 	if len(expected) == len(actual) {
 		for i, a := range actual {
 			e := expected[i]

internal/buf/bufcheck/bufbreaking/handler.go

@@ -34,7 +34,7 @@ func newHandler(
 ) *handler {
 	return &handler{
 		logger: logger,
-		runner: internal.NewRunner(logger),
+		runner: internal.NewRunner(logger, ""),
 	}
 }
 

internal/buf/bufcheck/bufbreaking/internal/util.go

@@ -30,9 +30,9 @@ type addFunc func(bufsrc.Descriptor, bufsrc.Location, string, ...interface{})
 
 func newFilesCheckFunc(
 	f func(addFunc, []bufsrc.File, []bufsrc.File) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
-	return func(id string, previousFiles []bufsrc.File, files []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
-		helper := internal.NewHelper(id)
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+	return func(id string, ignoreFunc internal.IgnoreFunc, previousFiles []bufsrc.File, files []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+		helper := internal.NewHelper(id, ignoreFunc)
 		if err := f(helper.AddFileAnnotationf, previousFiles, files); err != nil {
 			return nil, err
 		}
@@ -42,7 +42,7 @@ func newFilesCheckFunc(
 
 func newFilePairCheckFunc(
 	f func(addFunc, bufsrc.File, bufsrc.File) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newFilesCheckFunc(
 		func(add addFunc, previousFiles []bufsrc.File, files []bufsrc.File) error {
 			previousFilePathToFile, err := bufsrc.FilePathToFile(previousFiles...)
@@ -67,7 +67,7 @@ func newFilePairCheckFunc(
 
 func newEnumPairCheckFunc(
 	f func(addFunc, bufsrc.Enum, bufsrc.Enum) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newFilesCheckFunc(
 		func(add addFunc, previousFiles []bufsrc.File, files []bufsrc.File) error {
 			previousFullNameToEnum, err := bufsrc.FullNameToEnum(previousFiles...)
@@ -94,7 +94,7 @@ func newEnumPairCheckFunc(
 // map is from name to EnumValue for the given number
 func newEnumValuePairCheckFunc(
 	f func(addFunc, map[string]bufsrc.EnumValue, map[string]bufsrc.EnumValue) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newEnumPairCheckFunc(
 		func(add addFunc, previousEnum bufsrc.Enum, enum bufsrc.Enum) error {
 			previousNumberToNameToEnumValue, err := bufsrc.NumberToNameToEnumValue(previousEnum)
@@ -119,7 +119,7 @@ func newEnumValuePairCheckFunc(
 
 func newMessagePairCheckFunc(
 	f func(addFunc, bufsrc.Message, bufsrc.Message) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newFilesCheckFunc(
 		func(add addFunc, previousFiles []bufsrc.File, files []bufsrc.File) error {
 			previousFullNameToMessage, err := bufsrc.FullNameToMessage(previousFiles...)
@@ -144,7 +144,7 @@ func newMessagePairCheckFunc(
 
 func newFieldPairCheckFunc(
 	f func(addFunc, bufsrc.Field, bufsrc.Field) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newMessagePairCheckFunc(
 		func(add addFunc, previousMessage bufsrc.Message, message bufsrc.Message) error {
 			previousNumberToField, err := bufsrc.NumberToMessageField(previousMessage)
@@ -169,7 +169,7 @@ func newFieldPairCheckFunc(
 
 func newServicePairCheckFunc(
 	f func(addFunc, bufsrc.Service, bufsrc.Service) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newFilesCheckFunc(
 		func(add addFunc, previousFiles []bufsrc.File, files []bufsrc.File) error {
 			previousFullNameToService, err := bufsrc.FullNameToService(previousFiles...)
@@ -194,7 +194,7 @@ func newServicePairCheckFunc(
 
 func newMethodPairCheckFunc(
 	f func(addFunc, bufsrc.Method, bufsrc.Method) error,
-) func(string, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
+) func(string, internal.IgnoreFunc, []bufsrc.File, []bufsrc.File) ([]bufanalysis.FileAnnotation, error) {
 	return newServicePairCheckFunc(
 		func(add addFunc, previousService bufsrc.Service, service bufsrc.Service) error {
 			previousNameToMethod, err := bufsrc.NameToMethod(previousService)

internal/buf/bufcheck/buflint/buflint.go

@@ -63,6 +63,7 @@ type Config struct {
 	Checkers            []Checker
 	IgnoreIDToRootPaths map[string]map[string]struct{}
 	IgnoreRootPaths     map[string]struct{}
+	AllowCommentIgnores bool
 }
 
 // GetCheckers returns the checkers for the given categories.
@@ -81,6 +82,7 @@ func NewConfig(externalConfig buflintcfg.ExternalConfig) (*Config, error) {
 		Except:                               externalConfig.Except,
 		IgnoreRootPaths:                      externalConfig.Ignore,
 		IgnoreIDOrCategoryToRootPaths:        externalConfig.IgnoreOnly,
+		AllowCommentIgnores:                  externalConfig.AllowCommentIgnores,
 		EnumZeroValueSuffix:                  externalConfig.EnumZeroValueSuffix,
 		RPCAllowSameRequestResponse:          externalConfig.RPCAllowSameRequestResponse,
 		RPCAllowGoogleProtobufEmptyRequests:  externalConfig.RPCAllowGoogleProtobufEmptyRequests,
@@ -115,6 +117,7 @@ func internalConfigToConfig(internalConfig *internal.Config) *Config {
 		Checkers:            internalCheckersToCheckers(internalConfig.Checkers),
 		IgnoreIDToRootPaths: internalConfig.IgnoreIDToRootPaths,
 		IgnoreRootPaths:     internalConfig.IgnoreRootPaths,
+		AllowCommentIgnores: internalConfig.AllowCommentIgnores,
 	}
 }
 
@@ -123,6 +126,7 @@ func configToInternalConfig(config *Config) *internal.Config {
 		Checkers:            checkersToInternalCheckers(config.Checkers),
 		IgnoreIDToRootPaths: config.IgnoreIDToRootPaths,
 		IgnoreRootPaths:     config.IgnoreRootPaths,
+		AllowCommentIgnores: config.AllowCommentIgnores,
 	}
 }
 

internal/buf/bufcheck/buflint/buflint_ext_test.go

@@ -24,6 +24,7 @@ import (
 	"github.com/bufbuild/buf/internal/buf/bufbuild"
 	"github.com/bufbuild/buf/internal/buf/bufcheck/buflint"
 	"github.com/bufbuild/buf/internal/buf/bufconfig"
+	"github.com/bufbuild/buf/internal/buf/bufimage"
 	"github.com/bufbuild/buf/internal/buf/bufpath"
 	"github.com/bufbuild/buf/internal/pkg/storage"
 	"github.com/bufbuild/buf/internal/pkg/storage/storageos"
@@ -32,6 +33,10 @@ import (
 	"go.uber.org/zap"
 )
 
+// Hint on how to get these:
+// 1. cd into the specific diriectory
+// 2. buf check lint --error-format=json | jq '[.path, ".", .start_line, .start_column, .end_line, .end_column, .type] | @csv' --raw-output
+
 func TestRunComments(t *testing.T) {
 	testLint(
 		t,
@@ -734,6 +739,55 @@ func TestRunIgnores3(t *testing.T) {
 	)
 }
 
+func TestCommentIgnoresOff(t *testing.T) {
+	testLint(
+		t,
+		"comment_ignores",
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 9, 1, 9, 11, "PACKAGE_DIRECTORY_MATCH"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 9, 1, 9, 11, "PACKAGE_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 9, 1, 9, 11, "PACKAGE_VERSION_SUFFIX"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 12, 1, 12, 45, "IMPORT_NO_PUBLIC"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 15, 6, 15, 13, "ENUM_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 17, 3, 17, 29, "ENUM_NO_ALLOW_ALIAS"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 20, 3, 20, 14, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 20, 3, 20, 14, "ENUM_ZERO_VALUE_SUFFIX"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 22, 3, 22, 13, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 24, 3, 24, 13, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 28, 9, 28, 19, "MESSAGE_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 30, 11, 30, 21, "MESSAGE_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 32, 13, 32, 23, "MESSAGE_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 34, 12, 34, 19, "ENUM_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 36, 9, 36, 35, "ENUM_NO_ALLOW_ALIAS"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 39, 9, 39, 20, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 39, 9, 39, 20, "ENUM_ZERO_VALUE_SUFFIX"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 41, 9, 41, 19, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 43, 9, 43, 19, "ENUM_VALUE_UPPER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 46, 13, 46, 16, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 48, 13, 48, 16, "ONEOF_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 50, 15, 50, 18, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 54, 11, 54, 14, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 56, 11, 56, 14, "ONEOF_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 58, 13, 58, 16, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 62, 9, 62, 12, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 64, 9, 64, 12, "ONEOF_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 66, 11, 66, 14, "FIELD_LOWER_SNAKE_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 71, 9, 71, 19, "SERVICE_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 74, 7, 74, 16, "RPC_PASCAL_CASE"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 76, 7, 76, 28, "RPC_REQUEST_STANDARD_NAME"),
+		bufanalysistesting.NewFileAnnotationFunc("a.proto", ".", 79, 7, 79, 28, "RPC_RESPONSE_STANDARD_NAME"),
+	)
+}
+
+func TestCommentIgnoresOn(t *testing.T) {
+	testLintExternalConfigModifier(
+		t,
+		"comment_ignores",
+		func(externalConfig *bufconfig.ExternalConfig) {
+			externalConfig.Lint.AllowCommentIgnores = true
+		},
+	)
+}
+
 func testLint(
 	t *testing.T,
 	relDirPath string,
@@ -797,6 +851,7 @@ func testLintExternalConfigModifier(
 	)
 	require.NoError(t, err)
 	require.Empty(t, fileAnnotations)
+	image = bufimage.ImageWithoutImports(image)
 
 	handler := buflint.NewHandler(logger)
 	fileAnnotations, err = handler.Check(

internal/buf/bufcheck/buflint/buflintcfg/buflintcfg.go

@@ -35,6 +35,7 @@ type ExternalConfig struct {
 	RPCAllowGoogleProtobufEmptyRequests  bool                `json:"rpc_allow_google_protobuf_empty_requests,omitempty" yaml:"rpc_allow_google_protobuf_empty_requests,omitempty"`
 	RPCAllowGoogleProtobufEmptyResponses bool                `json:"rpc_allow_google_protobuf_empty_responses,omitempty" yaml:"rpc_allow_google_protobuf_empty_responses,omitempty"`
 	ServiceSuffix                        string              `json:"service_suffix,omitempty" yaml:"service_suffix,omitempty"`
+	AllowCommentIgnores                  bool                `json:"allow_comment_ignores,omitempty" yaml:"allow_comment_ignores,omitempty"`
 }
 
 // PrintFileAnnotationsLintConfigIgnoreYAML prints the FileAnnotations to the Writer as config-ignore-yaml.

internal/buf/bufcheck/buflint/handler.go

@@ -24,6 +24,8 @@ import (
 	"go.uber.org/zap"
 )
 
+const globalIgnorePrefix = "buf:lint:ignore"
+
 type handler struct {
 	logger *zap.Logger
 	runner *internal.Runner
@@ -32,7 +34,7 @@ type handler struct {
 func newHandler(logger *zap.Logger) *handler {
 	return &handler{
 		logger: logger,
-		runner: internal.NewRunner(logger),
+		runner: internal.NewRunner(logger, globalIgnorePrefix),
 	}
 }
 

internal/buf/bufcheck/buflint/internal/internal.go

@@ -20,6 +20,7 @@ import (
 	"strings"
 
 	"github.com/bufbuild/buf/internal/buf/bufanalysis"
+	"github.com/bufbuild/buf/internal/buf/bufcheck/internal"
 	"github.com/bufbuild/buf/internal/buf/bufsrc"
 	"github.com/bufbuild/buf/internal/pkg/normalpath"
 	"github.com/bufbuild/buf/internal/pkg/stringutil"
@@ -151,12 +152,17 @@ func checkEnumValueUpperSnakeCase(add addFunc, enumValue bufsrc.EnumValue) error
 }
 
 // CheckEnumZeroValueSuffix is a check function.
-var CheckEnumZeroValueSuffix = func(id string, files []bufsrc.File, suffix string) ([]bufanalysis.FileAnnotation, error) {
+var CheckEnumZeroValueSuffix = func(
+	id string,
+	ignoreFunc internal.IgnoreFunc,
+	files []bufsrc.File,
+	suffix string,
+) ([]bufanalysis.FileAnnotation, error) {
 	return newEnumValueCheckFunc(
 		func(add addFunc, enumValue bufsrc.EnumValue) error {
 			return checkEnumZeroValueSuffix(add, enumValue, suffix)
 		},
-	)(id, files)
+	)(id, ignoreFunc, files)
 }
 
 func checkEnumZeroValueSuffix(add addFunc, enumValue bufsrc.EnumValue, suffix string) error {
@@ -461,6 +467,7 @@ func checkRPCPascalCase(add addFunc, method bufsrc.Method) error {
 // CheckRPCRequestResponseUnique is a check function.
 var CheckRPCRequestResponseUnique = func(
 	id string,
+	ignoreFunc internal.IgnoreFunc,
 	files []bufsrc.File,
 	allowSameRequestResponse bool,
 	allowGoogleProtobufEmptyRequests bool,
@@ -476,7 +483,7 @@ var CheckRPCRequestResponseUnique = func(
 				allowGoogleProtobufEmptyResponses,
 			)
 		},
-	)(id, files)
+	)(id, ignoreFunc, files)
 }
 
 func checkRPCRequestResponseUnique(
@@ -560,12 +567,17 @@ func checkRPCRequestResponseUnique(
 }
 
 // CheckRPCRequestStandardName is a check function.
-var CheckRPCRequestStandardName = func(id string, files []bufsrc.File, allowGoogleProtobufEmptyRequests bool) ([]bufanalysis.FileAnnotation, error) {
+var CheckRPCRequestStandardName = func(
+	id string,
+	ignoreFunc internal.IgnoreFunc,
+	files []bufsrc.File,
+	allowGoogleProtobufEmptyRequests bool,
+) ([]bufanalysis.FileAnnotation, error) {
 	return newMethodCheckFunc(
 		func(add addFunc, method bufsrc.Method) error {
 			return checkRPCRequestStandardName(add, method, allowGoogleProtobufEmptyRequests)
 		},
-	)(id, files)
+	)(id, ignoreFunc, files)
 }
 
 func checkRPCRequestStandardName(add addFunc, method bufsrc.Method, allowGoogleProtobufEmptyRequests bool) error {
@@ -590,12 +602,17 @@ func checkRPCRequestStandardName(add addFunc, method bufsrc.Method, allowGoogleP
 }
 
 // CheckRPCResponseStandardName is a check function.
-var CheckRPCResponseStandardName = func(id string, files []bufsrc.File, allowGoogleProtobufEmptyResponses bool) ([]bufanalysis.FileAnnotation, error) {
+var CheckRPCResponseStandardName = func(
+	id string,
+	ignoreFunc internal.IgnoreFunc,
+	files []bufsrc.File,
+	allowGoogleProtobufEmptyResponses bool,
+) ([]bufanalysis.FileAnnotation, error) {
 	return newMethodCheckFunc(
 		func(add addFunc, method bufsrc.Method) error {
 			return checkRPCResponseStandardName(add, method, allowGoogleProtobufEmptyResponses)
 		},
-	)(id, files)
+	)(id, ignoreFunc, files)
 }
 
 func checkRPCResponseStandardName(add addFunc, method bufsrc.Method, allowGoogleProtobufEmptyResponses bool) error {
@@ -632,12 +649,17 @@ func checkServicePascalCase(add addFunc, service bufsrc.Service) error {
 }
 
 // CheckServiceSuffix is a check function.
-var CheckServiceSuffix = func(id string, files []bufsrc.File, suffix string) ([]bufanalysis.FileAnnotation, error) {
+var CheckServiceSuffix = func(
+	id string,
+	ignoreFunc internal.IgnoreFunc,
+	files []bufsrc.File,
+	suffix string,
+) ([]bufanalysis.FileAnnotation, error) {
 	return newServiceCheckFunc(
 		func(add addFunc, service bufsrc.Service) error {
 			return checkServiceSuffix(add, service, suffix)
 		},
-	)(id, files)
+	)(id, ignoreFunc, files)
 }
 
 func checkServiceSuffix(add addFunc, service bufsrc.Service, suffix string) error {