Fix support for unused imports linting in protoc-gen-buf-lint (bufbui…

…ld#1835)

This will address bufbuild/rules_buf#32. The
problem is that the code generator request does not include the stored
unused dependencies. So, since the check doesn't see that information,
it never complains (even if the file does have unused imports, and the
compiler that originally produced the descriptor set warned about them).

So, just for `protoc-gen-buf-lint`, we introduce a new step to
re-compute the unused dependencies and store them in the image proto.
This is woven into `bufimage`, because that's the place where we have
the information needed to do this.

This basically emulates the checks that the compiler does. The compiler
populates the set of used imports while resolving all symbols in the
file, during linking. So we basically do that, too, by visiting all
references and marking associated imports as used.

CHANGELOG.md

@@ -4,6 +4,8 @@
 
 - Add `buf beta price` command to help users of the BSR figure out how much a module
   will cost to store on the BSR under the Team or Pro plans.
+- Fix an issue in `protoc-gen-buf-lint` that prevented it from reporting lint
+  errors for unused imports.
 
 ## [v1.15.1] - 2023-03-08
 

cmd/protoc-gen-buf-lint/main.go

@@ -14,7 +14,9 @@
 
 package main
 
-import lint "github.com/bufbuild/buf/private/buf/cmd/protoc-gen-buf-lint"
+import (
+	lint "github.com/bufbuild/buf/private/buf/cmd/protoc-gen-buf-lint"
+)
 
 func main() {
 	lint.Main()

private/buf/bufcurl/bufcurl.go

@@ -16,28 +16,39 @@ package bufcurl
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
 
+	"github.com/bufbuild/buf/private/pkg/protoencoding"
 	"google.golang.org/protobuf/reflect/protoreflect"
 	"google.golang.org/protobuf/reflect/protoregistry"
 )
 
-// Resolver is an interface for resolving symbols into descriptors and for
-// looking up extensions.
-//
-// Note that resolver implementations must be thread-safe because they could be
-// used by two goroutines concurrently during bidirectional streaming calls.
-type Resolver interface {
-	FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error)
-	protoregistry.MessageTypeResolver
-	protoregistry.ExtensionTypeResolver
-}
-
 // Invoker provides the ability to invoke RPCs dynamically.
 type Invoker interface {
 	// Invoke invokes an RPC method using the given input data and request headers.
 	// The dataSource is a string that describes the input data (e.g. a filename).
 	// The actual contents of the request data is read from the given reader.
 	Invoke(ctx context.Context, dataSource string, data io.Reader, headers http.Header) error
 }
+
+// ResolveMethodDescriptor uses the given resolver to find a descriptor for
+// the requested service and method. The service name must be fully-qualified.
+func ResolveMethodDescriptor(res protoencoding.Resolver, service, method string) (protoreflect.MethodDescriptor, error) {
+	descriptor, err := res.FindDescriptorByName(protoreflect.FullName(service))
+	if err == protoregistry.NotFound {
+		return nil, fmt.Errorf("failed to find service named %q in schema", service)
+	} else if err != nil {
+		return nil, err
+	}
+	serviceDescriptor, ok := descriptor.(protoreflect.ServiceDescriptor)
+	if !ok {
+		return nil, fmt.Errorf("URL indicates service name %q, but that name is a %s", service, descriptorKind(descriptor))
+	}
+	methodDescriptor := serviceDescriptor.Methods().ByName(protoreflect.Name(method))
+	if methodDescriptor == nil {
+		return nil, fmt.Errorf("URL indicates method name %q, but service %q contains no such method", method, service)
+	}
+	return methodDescriptor, nil
+}

private/buf/bufcurl/invoker.go

@@ -71,7 +71,7 @@ type invokeClient = connect.Client[dynamicpb.Message, deferredMessage]
 
 type invoker struct {
 	md        protoreflect.MethodDescriptor
-	res       Resolver
+	res       protoencoding.Resolver
 	client    *invokeClient
 	output    io.Writer
 	errOutput io.Writer
@@ -83,7 +83,7 @@ type invoker struct {
 // in JSON format. The given resolver is used to resolve Any messages and
 // extensions that appear in the input or output. Other parameters are used
 // to create a Connect client, for issuing the RPC.
-func NewInvoker(container appflag.Container, md protoreflect.MethodDescriptor, res Resolver, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker {
+func NewInvoker(container appflag.Container, md protoreflect.MethodDescriptor, res protoencoding.Resolver, httpClient connect.HTTPClient, opts []connect.ClientOption, url string, out io.Writer) Invoker {
 	opts = append(opts, connect.WithCodec(protoCodec{}))
 	// TODO: could also provide custom compressor implementations that could give us
 	//  optics into when request and response messages are compressed (which could be
@@ -381,15 +381,15 @@ func (inv *invoker) handleErrorResponse(connErr *connect.Error) error {
 	return app.NewError(int(connErr.Code()*8), "")
 }
 
-func newStreamMessageProvider(dataSource string, data io.Reader, res Resolver) messageProvider {
+func newStreamMessageProvider(dataSource string, data io.Reader, res protoencoding.Resolver) messageProvider {
 	if data == nil {
 		// if no data provided, treat as empty input
 		data = bytes.NewBuffer(nil)
 	}
 	return &streamMessageProvider{name: dataSource, dec: json.NewDecoder(data), res: res}
 }
 
-func newMessageProvider(dataSource string, data io.Reader, res Resolver) messageProvider {
+func newMessageProvider(dataSource string, data io.Reader, res protoencoding.Resolver) messageProvider {
 	if data == nil {
 		// if no data provider, treat as if single empty message
 		return &singleEmptyMessageProvider{}
@@ -417,7 +417,7 @@ func (s *singleEmptyMessageProvider) next(_ proto.Message) error {
 type streamMessageProvider struct {
 	name string
 	dec  *json.Decoder
-	res  Resolver
+	res  protoencoding.Resolver
 }
 
 func (s *streamMessageProvider) next(msg proto.Message) error {

private/buf/bufcurl/reflection_resolver.go

@@ -101,7 +101,7 @@ func NewServerReflectionResolver(
 	reflectProtocol ReflectProtocol,
 	headers http.Header,
 	printer verbose.Printer,
-) (r Resolver, closeResolver func()) {
+) (r protoencoding.Resolver, closeResolver func()) {
 	baseURL = strings.TrimSuffix(baseURL, "/")
 	var v1Client, v1alphaClient *reflectClient
 	if reflectProtocol != ReflectProtocolGRPCV1 {
@@ -149,6 +149,31 @@ type reflectionResolver struct {
 	cachedExts              protoregistry.Types
 }
 
+func (r *reflectionResolver) FindFileByPath(path string) (protoreflect.FileDescriptor, error) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	d, err := r.cachedFiles.FindFileByPath(path)
+	if d != nil {
+		return d, nil
+	}
+	if err != protoregistry.NotFound {
+		return nil, err
+	}
+	// if not found in existing files, fetch more
+	fileDescriptorProtos, err := r.fileByNameLocked(path)
+	if err != nil {
+		// intentionally not using "%w" because, depending on the code, the bufcli
+		// app framework might incorrectly interpret it and report a bad error message.
+		return nil, fmt.Errorf("failed to resolve filename %q: %v", path, err)
+	}
+	if err := r.cacheFilesLocked(fileDescriptorProtos); err != nil {
+		return nil, err
+	}
+	// now it should definitely be in there!
+	return r.cachedFiles.FindFileByPath(path)
+}
+
 func (r *reflectionResolver) FindDescriptorByName(name protoreflect.FullName) (protoreflect.Descriptor, error) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
@@ -174,14 +199,26 @@ func (r *reflectionResolver) FindDescriptorByName(name protoreflect.FullName) (p
 	return r.cachedFiles.FindDescriptorByName(name)
 }
 
+func (r *reflectionResolver) FindEnumByName(enum protoreflect.FullName) (protoreflect.EnumType, error) {
+	d, err := r.FindDescriptorByName(enum)
+	if err != nil {
+		return nil, err
+	}
+	ed, ok := d.(protoreflect.EnumDescriptor)
+	if !ok {
+		return nil, fmt.Errorf("element %s is a %s, not an enum", enum, descriptorKind(d))
+	}
+	return dynamicpb.NewEnumType(ed), nil
+}
+
 func (r *reflectionResolver) FindMessageByName(message protoreflect.FullName) (protoreflect.MessageType, error) {
 	d, err := r.FindDescriptorByName(message)
 	if err != nil {
 		return nil, err
 	}
 	md, ok := d.(protoreflect.MessageDescriptor)
 	if !ok {
-		return nil, fmt.Errorf("element %s is a %s, not a message", message, DescriptorKind(d))
+		return nil, fmt.Errorf("element %s is a %s, not a message", message, descriptorKind(d))
 	}
 	return dynamicpb.NewMessageType(md), nil
 }
@@ -199,7 +236,7 @@ func (r *reflectionResolver) FindExtensionByName(field protoreflect.FullName) (p
 	}
 	fd, ok := d.(protoreflect.FieldDescriptor)
 	if !ok || !fd.IsExtension() {
-		return nil, fmt.Errorf("element %s is a %s, not an extension", field, DescriptorKind(d))
+		return nil, fmt.Errorf("element %s is a %s, not an extension", field, descriptorKind(d))
 	}
 	return dynamicpb.NewExtensionType(fd), nil
 }
@@ -438,3 +475,47 @@ func reset(stream *reflectStream) {
 	_, _ = stream.Receive()
 	_ = stream.CloseResponse()
 }
+
+type extensionContainer interface {
+	Messages() protoreflect.MessageDescriptors
+	Extensions() protoreflect.ExtensionDescriptors
+}
+
+func registerExtensions(reg *protoregistry.Types, descriptor extensionContainer) {
+	exts := descriptor.Extensions()
+	for i := 0; i < exts.Len(); i++ {
+		extType := dynamicpb.NewExtensionType(exts.Get(i))
+		_ = reg.RegisterExtension(extType)
+	}
+	msgs := descriptor.Messages()
+	for i := 0; i < msgs.Len(); i++ {
+		registerExtensions(reg, msgs.Get(i))
+	}
+}
+
+// descriptorKind returns a succinct description of the type of the given descriptor.
+func descriptorKind(d protoreflect.Descriptor) string {
+	switch d := d.(type) {
+	case protoreflect.FileDescriptor:
+		return "file"
+	case protoreflect.MessageDescriptor:
+		return "message"
+	case protoreflect.FieldDescriptor:
+		if d.IsExtension() {
+			return "extension"
+		}
+		return "field"
+	case protoreflect.OneofDescriptor:
+		return "oneof"
+	case protoreflect.EnumDescriptor:
+		return "enum"
+	case protoreflect.EnumValueDescriptor:
+		return "enum value"
+	case protoreflect.ServiceDescriptor:
+		return "service"
+	case protoreflect.MethodDescriptor:
+		return "method"
+	default:
+		return fmt.Sprintf("%T", d)
+	}
+}