Skip to content

Commit c2390ce

Browse files
committed
Fix test files to work without jhump/protoreflect
Created testutils helpers for lint package tests and fixed all test files to use the new Google protobuf library: - Added lint/testutils_test.go with builder-style helpers for test proto creation - Fixed lint test files to use new testutils (rule_test.go, rule_enabled_test.go, problem_test.go) - Updated rules/internal/testutils/problems_test.go to use ParseProto3String - Added timestamp import to support well-known types in tests - Fixed common_lints_test.go and resource_test.go to use descriptorpb types - Added WithStandardImports to protocompile for well-known types support - Partially implemented registry resolver for aep/api and google/api imports Test status: - lint package: all tests pass - rules/internal/testutils: all tests pass - rules/internal/utils: build error (registry resolver needs completion) - Other rule packages: need registry resolver to be completed The registry resolver implementation is in progress to handle imports of aep/api/* and google/api/* proto files that are registered but not available as source files.
1 parent b9216a7 commit c2390ce

9 files changed

Lines changed: 473 additions & 52 deletions

File tree

internal/protoparse/parser.go

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/bufbuild/protocompile"
2929
"github.com/bufbuild/protocompile/reporter"
3030
"google.golang.org/protobuf/reflect/protoreflect"
31+
"google.golang.org/protobuf/reflect/protoregistry"
3132
)
3233

3334
// ErrorWithPos represents a parse error with position information.
@@ -87,17 +88,35 @@ func (m mapAccessor) Open(filename string) (io.ReadCloser, error) {
8788

8889
// ParseFiles parses the given proto files.
8990
func (p *Parser) ParseFiles(filenames ...string) ([]*desc.FileDescriptor, error) {
90-
resolver := protocompile.SourceResolver{
91+
resolver := &protocompile.SourceResolver{
9192
ImportPaths: p.ImportPaths,
9293
}
9394

9495
if p.Accessor != nil {
95-
adapter := &accessorAdapter{accessor: p.Accessor, importPaths: p.ImportPaths}
96+
adapter := &accessorAdapter{accessor: p.Accessor, importPaths: p.ImportPaths, lookupImport: p.LookupImport}
9697
resolver.Accessor = adapter.Open
9798
}
9899

100+
// Wrap resolver with standard imports (well-known types)
101+
resolverWithStdImports := protocompile.WithStandardImports(resolver)
102+
103+
// Create a composite resolver that checks the global registry first
104+
compositeResolver := protocompile.CompositeResolver{
105+
protocompile.ResolverFunc(func(path string) (protocompile.SearchResult, error) {
106+
// Try to find in global registry
107+
fd, err := protoregistry.GlobalFiles.FindFileByPath(path)
108+
if err == nil {
109+
// Found in registry, return it
110+
return &registrySearchResult{fd: fd}, nil
111+
}
112+
// Not in registry, fall through to next resolver
113+
return nil, fs.ErrNotExist
114+
}),
115+
resolverWithStdImports,
116+
}
117+
99118
compiler := &protocompile.Compiler{
100-
Resolver: &resolver,
119+
Resolver: compositeResolver,
101120
SourceInfoMode: protocompile.SourceInfoStandard,
102121
}
103122

@@ -138,8 +157,9 @@ func (er *errorReporter) Warning(reporter.ErrorWithPos) {
138157
}
139158

140159
type accessorAdapter struct {
141-
accessor FileAccessor
142-
importPaths []string
160+
accessor FileAccessor
161+
importPaths []string
162+
lookupImport func(string) (*desc.FileDescriptor, error)
143163
}
144164

145165
func (a *accessorAdapter) Open(path string) (io.ReadCloser, error) {
@@ -156,10 +176,32 @@ func (a *accessorAdapter) Open(path string) (io.ReadCloser, error) {
156176
}
157177
}
158178

179+
// Try lookup import (for well-known types)
180+
if a.lookupImport != nil {
181+
if fd, err := a.lookupImport(path); err == nil {
182+
// Convert the FileDescriptor to a proto and return it as a reader
183+
proto := fd.AsProto()
184+
// Note: protocompile needs the source, but we have the compiled descriptor.
185+
// This won't work for source info. For well-known types, we'll skip them
186+
// by returning an error and let protocompile use its built-in resolution.
187+
_ = proto
188+
// Fall through to filesystem
189+
}
190+
}
191+
159192
// Fall back to file system
160193
return os.Open(path)
161194
}
162195

196+
// registrySearchResult implements protocompile.SearchResult for registry lookups.
197+
type registrySearchResult struct {
198+
fd protoreflect.FileDescriptor
199+
}
200+
201+
func (r *registrySearchResult) Desc() protoreflect.FileDescriptor {
202+
return r.fd
203+
}
204+
163205
// ResolveFilenames resolves file paths relative to import paths.
164206
func ResolveFilenames(importPaths []string, filenames ...string) ([]string, error) {
165207
resolved := make([]string, len(filenames))

lint/problem_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,12 @@ func TestProblemYAML(t *testing.T) {
8484

8585
func TestProblemDescriptor(t *testing.T) {
8686
mb := testutils.NewMessage(t, "Foo")
87-
testutils.NewFile(t, "foo.proto").AddMessage(mb)
88-
89-
m, err := mb.Build()
87+
fd, err := testutils.NewFile(t, "foo.proto").AddMessage(mb).Build()
9088
if err != nil {
9189
t.Fatalf("%v", err)
9290
}
91+
92+
m := fd.GetMessageTypes()[0]
9393
m.GetSourceInfo().Span = []int32{42, 0, 79}
9494
problem := &Problem{
9595
Message: "foo bar",

lint/rule_enabled_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,10 @@ func TestRuleIsEnabled(t *testing.T) {
7070
// Run the specific tests individually.
7171
for _, test := range tests {
7272
t.Run(test.testName, func(t *testing.T) {
73-
f, err := testutils.NewFile(t, "test.proto").SetSyntaxComments(builder.Comments{
73+
f, err := testutils.NewFile(t, "test.proto").SetSyntaxComments(Comments{
7474
LeadingComment: test.fileComment,
7575
}).AddMessage(
76-
testutils.NewMessage(t, "MyMessage").SetComments(builder.Comments{
76+
testutils.NewMessage(t, "MyMessage").SetComments(Comments{
7777
LeadingComment: test.messageComment,
7878
}),
7979
).Build()
@@ -93,10 +93,10 @@ func TestRuleIsEnabled(t *testing.T) {
9393

9494
for _, test := range tests {
9595
t.Run(fmt.Sprintf("MustRule/%s", test.testName), func(t *testing.T) {
96-
f, err := testutils.NewFile(t, "test.proto").SetSyntaxComments(builder.Comments{
96+
f, err := testutils.NewFile(t, "test.proto").SetSyntaxComments(Comments{
9797
LeadingComment: test.fileComment,
9898
}).AddMessage(
99-
testutils.NewMessage(t, "MyMessage").SetComments(builder.Comments{
99+
testutils.NewMessage(t, "MyMessage").SetComments(Comments{
100100
LeadingComment: test.messageComment,
101101
}),
102102
).Build()
@@ -121,7 +121,7 @@ func TestRuleIsEnabledFirstMessage(t *testing.T) {
121121

122122
// Build a proto and check that ruleIsEnabled does the right thing.
123123
f, err := testutils.NewFile(t, "test.proto").AddMessage(
124-
testutils.NewMessage(t, "FirstMessage").SetComments(builder.Comments{
124+
testutils.NewMessage(t, "FirstMessage").SetComments(Comments{
125125
LeadingComment: "api-linter: test=disabled",
126126
}),
127127
).AddMessage(
@@ -149,11 +149,11 @@ func TestRuleIsEnabledParent(t *testing.T) {
149149

150150
// Build a proto with two messages, one of which disables the rule.
151151
f, err := testutils.NewFile(t, "test.proto").AddMessage(
152-
testutils.NewMessage(t, "Foo").SetComments(builder.Comments{
152+
testutils.NewMessage(t, "Foo").SetComments(Comments{
153153
LeadingComment: "api-linter: test=disabled",
154-
}).AddField(/* field: "foo", testutils.FieldTypeBool( */)),
154+
}).AddField(newField("foo", "bool", 1)),
155155
).AddMessage(
156-
testutils.NewMessage(t, "Bar").AddField(/* field: "bar", testutils.FieldTypeBool( */)),
156+
testutils.NewMessage(t, "Bar").AddField(newField("bar", "bool", 1)),
157157
).Build()
158158
if err != nil {
159159
t.Fatalf("Error building test file: %q", err)
@@ -191,7 +191,7 @@ func TestRuleIsEnabledDeprecated(t *testing.T) {
191191
f, err := testutils.NewFile(t, "test.proto").AddMessage(
192192
testutils.NewMessage(t, "Foo").SetOptions(&dpb.MessageOptions{
193193
Deprecated: &test.msgDeprecated,
194-
}).AddField(/* field: "bar", testutils.FieldTypeBool( */).SetOptions(
194+
}).AddField(newField("bar", "bool", 1).SetOptions(
195195
&dpb.FieldOptions{Deprecated: &test.fieldDeprecated},
196196
)),
197197
).Build()

lint/rule_test.go

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,9 @@ func TestFieldRule(t *testing.T) {
113113
// Create a file descriptor with one message and two fields in that message.
114114
fd, err := testutils.NewFile(t, "test.proto").AddMessage(
115115
testutils.NewMessage(t, "Book").AddField(
116-
/* field: "title", testutils.FieldTypeString( */),
116+
newField("title", "string", 1),
117117
).AddField(
118-
/* field: "edition_count", testutils.FieldTypeInt32( */),
118+
newField("edition_count", "int32", 2),
119119
),
120120
).Build()
121121
if err != nil {
@@ -170,19 +170,22 @@ func TestServiceRule(t *testing.T) {
170170

171171
func TestMethodRule(t *testing.T) {
172172
// Create a file descriptor with a service.
173-
book := builder.RpcTypeMessage(testutils.NewMessage(t, "Book"), false)
174-
fd, err := testutils.NewFile(t, "test.proto").AddService(
173+
book := testutils.NewMessage(t, "Book")
174+
getBookRequest := testutils.NewMessage(t, "GetBookRequest")
175+
createBookRequest := testutils.NewMessage(t, "CreateBookRequest")
176+
177+
fd, err := testutils.NewFile(t, "test.proto").AddMessage(book).AddMessage(getBookRequest).AddMessage(createBookRequest).AddService(
175178
builder.NewService("Library").AddMethod(
176179
builder.NewMethod(
177180
"GetBook",
178-
builder.RpcTypeMessage(testutils.NewMessage(t, "GetBookRequest"), false),
179-
book,
181+
builder.RpcTypeMessage(getBookRequest, false),
182+
builder.RpcTypeMessage(book, false),
180183
),
181184
).AddMethod(
182185
builder.NewMethod(
183186
"CreateBook",
184-
builder.RpcTypeMessage(testutils.NewMessage(t, "CreateBookRequest"), false),
185-
book,
187+
builder.RpcTypeMessage(createBookRequest, false),
188+
builder.RpcTypeMessage(book, false),
186189
),
187190
),
188191
).Build()
@@ -213,9 +216,9 @@ func TestMethodRule(t *testing.T) {
213216
func TestEnumRule(t *testing.T) {
214217
// Create a file descriptor with top-level enums.
215218
fd, err := testutils.NewFile(t, "test.proto").AddEnum(
216-
/* enum: "Format" */.AddValue(/* enum_value: "PDF" */),
219+
newEnum("Format").AddValue(newEnumValue("PDF", 0)),
217220
).AddEnum(
218-
/* enum: "Edition" */.AddValue(/* enum_value: "PUBLISHER_ONLY" */),
221+
newEnum("Edition").AddValue(newEnumValue("PUBLISHER_ONLY", 0)),
219222
).Build()
220223
if err != nil {
221224
t.Fatalf("Error building test proto:%s ", err)
@@ -243,7 +246,7 @@ func TestEnumRule(t *testing.T) {
243246
func TestEnumValueRule(t *testing.T) {
244247
// Create a file descriptor with a top-level enum with values.
245248
fd, err := testutils.NewFile(t, "test.proto").AddEnum(
246-
/* enum: "Format" */.AddValue(/* enum_value: "YAML" */).AddValue(/* enum_value: "JSON" */),
249+
newEnum("Format").AddValue(newEnumValue("YAML", 0)).AddValue(newEnumValue("JSON", 1)),
247250
).Build()
248251
if err != nil {
249252
t.Fatalf("Error building test proto:%s ", err)
@@ -272,9 +275,9 @@ func TestEnumRuleNested(t *testing.T) {
272275
// Create a file descriptor with top-level enums.
273276
fd, err := testutils.NewFile(t, "test.proto").AddMessage(
274277
testutils.NewMessage(t, "Book").AddNestedEnum(
275-
/* enum: "Format" */.AddValue(/* enum_value: "PDF" */),
278+
newEnum("Format").AddValue(newEnumValue("PDF", 0)),
276279
).AddNestedEnum(
277-
/* enum: "Edition" */.AddValue(/* enum_value: "PUBLISHER_ONLY" */),
280+
newEnum("Edition").AddValue(newEnumValue("PUBLISHER_ONLY", 0)),
278281
),
279282
).Build()
280283
if err != nil {
@@ -303,10 +306,10 @@ func TestEnumRuleNested(t *testing.T) {
303306
func TestDescriptorRule(t *testing.T) {
304307
// Create a file with one of everything in it.
305308
book := testutils.NewMessage(t, "Book").AddNestedEnum(
306-
/* enum: "Format" */.AddValue(
307-
/* enum_value: "FORMAT_UNSPECIFIED" */,
308-
).AddValue(/* enum_value: "PAPERBACK" */),
309-
).AddField(/* field: "name", testutils.FieldTypeString( */)).AddNestedMessage(
309+
newEnum("Format").AddValue(
310+
newEnumValue("FORMAT_UNSPECIFIED", 0),
311+
).AddValue(newEnumValue("PAPERBACK", 1)),
312+
).AddField(newField("name", "string", 1)).AddNestedMessage(
310313
testutils.NewMessage(t, "Author"),
311314
)
312315
fd, err := testutils.NewFile(t, "library.proto").AddMessage(book).AddService(
@@ -317,7 +320,7 @@ func TestDescriptorRule(t *testing.T) {
317320
builder.RpcTypeMessage(book, false),
318321
),
319322
),
320-
).AddEnum(/* enum: "State" */.AddValue(/* enum_value: "AVAILABLE" */)).Build()
323+
).AddEnum(newEnum("State").AddValue(newEnumValue("AVAILABLE", 0))).Build()
321324
if err != nil {
322325
t.Fatalf("%v", err)
323326
}

0 commit comments

Comments
 (0)