diff --git a/check/response_writer.go b/check/response_writer.go index 40bb7bf..be80d31 100644 --- a/check/response_writer.go +++ b/check/response_writer.go @@ -339,8 +339,11 @@ func getFileLocationForAddAnnotationOptions( if !ok { return nil, fmt.Errorf("cannot add annotation for unknown file: %q", fileName) } - if len(path) > 0 { + for ; len(path) > 0; path = path[:len(path)-1] { sourceLocation = fileDescriptor.ProtoreflectFileDescriptor().SourceLocations().ByPath(path) + if len(sourceLocation.Path) > 0 { + break + } } return descriptor.NewFileLocation(fileDescriptor, sourceLocation), nil } diff --git a/check/response_writer_test.go b/check/response_writer_test.go new file mode 100644 index 0000000..594fe41 --- /dev/null +++ b/check/response_writer_test.go @@ -0,0 +1,132 @@ +package check_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "buf.build/go/bufplugin/check" + "buf.build/go/bufplugin/check/checktest" + "buf.build/go/bufplugin/check/checkutil" + "buf.build/go/bufplugin/descriptor" + "github.com/stretchr/testify/require" +) + +const protoFile = ` +syntax = "proto3"; +import "google/protobuf/descriptor.proto"; +extend google.protobuf.MessageOptions { + X x = 5000; +} +message X { + string y = 1; +} +message Foo { + option deprecated = true; + option (x).y = "z"; +} +` + +// TestIssue20 checks that an annotation by source path points to the nearest parent if the original one is missing. +func TestIssue20(t *testing.T) { + t.Parallel() + dir := t.TempDir() + const fileName = "file.proto" + const ruleID = "TEST_MISSING_SOURCE_LOCATION" + require.NoError(t, os.WriteFile(filepath.Join(dir, fileName), []byte(protoFile), 0666)) + checktest.CheckTest{ + Request: &checktest.RequestSpec{ + Files: &checktest.ProtoFileSpec{ + DirPaths: []string{dir}, + FilePaths: []string{fileName}, + }, + RuleIDs: []string{ruleID}, + }, + Spec: &check.Spec{ + Rules: []*check.RuleSpec{{ + ID: ruleID, + Purpose: "Purpose.", + Type: check.RuleTypeLint, + Handler: checkutil.NewFileRuleHandler( + func( + _ context.Context, + writer check.ResponseWriter, + _ check.Request, + file descriptor.FileDescriptor, + ) error { + foo := file.ProtoreflectFileDescriptor().Messages().ByName("Foo") + fooPath := file.ProtoreflectFileDescriptor().SourceLocations().ByDescriptor(foo).Path + writer.AddAnnotation( + check.WithMessage("Annotation for message"), + check.WithFileNameAndSourcePath( + fileName, + fooPath, + ), + ) + writer.AddAnnotation( + check.WithMessage("Annotation for option (x).y"), + check.WithFileNameAndSourcePath( + fileName, + append( + fooPath, + 7, // google.protobuf.DescriptorProto.options + 5000, // x + 1, // X.y + ), + ), + ) + writer.AddAnnotation( + check.WithMessage("Annotation for option (x) points to first message option"), + check.WithFileNameAndSourcePath( + fileName, + append( + fooPath, + 7, // google.protobuf.DescriptorProto.options + 5000, // x + ), + ), + ) + return nil + }, + checkutil.WithoutImports(), + ), + }}, + }, + ExpectedAnnotations: []checktest.ExpectedAnnotation{ + { + RuleID: ruleID, + Message: "Annotation for message", + FileLocation: &checktest.ExpectedFileLocation{ + FileName: fileName, + StartLine: 9, + StartColumn: 0, + EndLine: 12, + EndColumn: 1, + }, + }, + { + RuleID: ruleID, + Message: "Annotation for option (x) points to first message option", + FileLocation: &checktest.ExpectedFileLocation{ + FileName: fileName, + StartLine: 10, + StartColumn: 2, + EndLine: 10, + EndColumn: 27, + }, + }, + { + RuleID: ruleID, + Message: "Annotation for option (x).y", + FileLocation: &checktest.ExpectedFileLocation{ + FileName: fileName, + StartLine: 11, + StartColumn: 2, + EndLine: 11, + EndColumn: 21, + }, + }, + }, + }.Run(t) +}