Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# bufplugin-go

[![Build](https://github.com/bufbuild/bufplugin-go/actions/workflows/ci.yaml/badge.svg?branch=main)](https://github.com/bufbuild/bufplugin-go/actions/workflows/ci.yaml)
[![Report Card](https://goreportcard.com/badge/buf.build/go/bufplugin)](https://goreportcard.com/report/buf.build/go/bufplugin)
[![GoDoc](https://pkg.go.dev/badge/buf.build/go/bufplugin.svg)](https://pkg.go.dev/buf.build/go/bufplugin)
[![Slack](https://img.shields.io/badge/slack-buf-%23e01563)](https://buf.build/links/slack)

Expand Down
51 changes: 51 additions & 0 deletions check/check_service_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,54 @@ func TestCheckServiceHandlerNoSourceCodeInfo(t *testing.T) {
)
require.NoError(t, err)
}

func TestCheckServiceHandlerModuleName(t *testing.T) {
t.Parallel()

checkServiceHandler, err := NewCheckServiceHandler(
&Spec{
Rules: []*RuleSpec{
testNewSimpleLintRuleSpec("RULE1", nil, true, false, nil),
},
},
)
require.NoError(t, err)

newCheckRequest := func(moduleName *descriptorv1.ModuleName) *checkv1.CheckRequest {
return checkv1.CheckRequest_builder{
FileDescriptors: []*descriptorv1.FileDescriptor{
descriptorv1.FileDescriptor_builder{
FileDescriptorProto: &descriptorpb.FileDescriptorProto{
Name: new("foo.proto"),
},
ModuleName: moduleName,
}.Build(),
},
}.Build()
}

_, err = checkServiceHandler.Check(
t.Context(),
newCheckRequest(
descriptorv1.ModuleName_builder{
Registry: "buf.build",
Owner: "acme",
Module: "weather",
}.Build(),
),
)
require.NoError(t, err)

_, err = checkServiceHandler.Check(
t.Context(),
newCheckRequest(
descriptorv1.ModuleName_builder{
Owner: "acme",
Module: "weather",
}.Build(),
),
)
pluginrpcError := &pluginrpc.Error{}
require.ErrorAs(t, err, &pluginrpcError)
require.Equal(t, pluginrpc.CodeInvalidArgument, pluginrpcError.Code())
}
30 changes: 27 additions & 3 deletions check/checktest/checktest.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type RequestSpec struct {
//
// Required.
Files *ProtoFileSpec
// AgainstFiles specifies the input against files to test against, if anoy.
// AgainstFiles specifies the input against files to test against, if any.
AgainstFiles *ProtoFileSpec
// RuleIDs are the specific RuleIDs to run.
RuleIDs []string
Expand Down Expand Up @@ -160,6 +160,12 @@ type ProtoFileSpec struct {
//
// This corresponds to arguments passed to protoc.
FilePaths []string
// ModuleName is the name of the module that the FilePaths belong to, in the
// form registry/owner/module.
//
// If set, all files that are not imports will have this ModuleName.
// Optional.
ModuleName string
}

// ToFileDescriptors compiles the files into descriptor.FileDescriptors.
Expand All @@ -172,7 +178,15 @@ func (p *ProtoFileSpec) ToFileDescriptors(ctx context.Context) ([]descriptor.Fil
if err := validateProtoFileSpec(p); err != nil {
return nil, err
}
return compile(ctx, p.DirPaths, p.FilePaths)
var moduleName descriptor.ModuleName
if p.ModuleName != "" {
var err error
moduleName, err = descriptor.ParseModuleName(p.ModuleName)
if err != nil {
return nil, err
}
}
return compile(ctx, p.DirPaths, p.FilePaths, moduleName)
}

// ExpectedAnnotation contains the values expected from an Annotation.
Expand Down Expand Up @@ -315,7 +329,12 @@ func expectedAnnotationForAnnotation(annotation check.Annotation) ExpectedAnnota
return expectedAnnotation
}

func compile(ctx context.Context, dirPaths []string, filePaths []string) ([]descriptor.FileDescriptor, error) {
func compile(
ctx context.Context,
dirPaths []string,
filePaths []string,
moduleName descriptor.ModuleName,
) ([]descriptor.FileDescriptor, error) {
dirPaths = fromSlashPaths(dirPaths)
filePaths = fromSlashPaths(filePaths)
toSlashFilePathMap := make(map[string]struct{}, len(filePaths))
Expand Down Expand Up @@ -361,11 +380,16 @@ func compile(ctx context.Context, dirPaths []string, filePaths []string) ([]desc
fileDescriptorProto,
filePathToUnusedDependencyFilePaths[fileDescriptorProto.GetName()],
)
var protoModuleName *descriptorv1.ModuleName
if isNotImport && moduleName != nil {
protoModuleName = moduleName.ToProto()
}
protoFileDescriptors[i] = descriptorv1.FileDescriptor_builder{
FileDescriptorProto: fileDescriptorProto,
IsImport: !isNotImport,
IsSyntaxUnspecified: isSyntaxUnspecified,
UnusedDependency: unusedDependencyIndexes,
ModuleName: protoModuleName,
}.Build()
}
return descriptor.FileDescriptorsForProtoFileDescriptors(protoFileDescriptors)
Expand Down
47 changes: 20 additions & 27 deletions check/compare.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,20 @@ import (

// CompareAnnotations returns -1 if one < two, 1 if one > two, 0 otherwise.
func CompareAnnotations(one Annotation, two Annotation) int {
if one == nil && two == nil {
switch {
case one == nil && two == nil:
return 0
}
if one == nil && two != nil {
case one == nil:
return -1
}
if one != nil && two == nil {
case two == nil:
return 1
}
if compare := strings.Compare(one.RuleID(), two.RuleID()); compare != 0 {
return compare
}

if compare := descriptor.CompareFileLocations(one.FileLocation(), two.FileLocation()); compare != 0 {
return compare
}

if compare := descriptor.CompareFileLocations(one.AgainstFileLocation(), two.AgainstFileLocation()); compare != 0 {
return compare
}
Expand All @@ -47,27 +44,25 @@ func CompareAnnotations(one Annotation, two Annotation) int {

// CompareRules returns -1 if one < two, 1 if one > two, 0 otherwise.
func CompareRules(one Rule, two Rule) int {
if one == nil && two == nil {
switch {
case one == nil && two == nil:
return 0
}
if one == nil && two != nil {
case one == nil:
return -1
}
if one != nil && two == nil {
case two == nil:
return 1
}
return strings.Compare(one.ID(), two.ID())
}

// CompareCategories returns -1 if one < two, 1 if one > two, 0 otherwise.
func CompareCategories(one Category, two Category) int {
if one == nil && two == nil {
switch {
case one == nil && two == nil:
return 0
}
if one == nil && two != nil {
case one == nil:
return -1
}
if one != nil && two == nil {
case two == nil:
return 1
}
return strings.Compare(one.ID(), two.ID())
Expand All @@ -77,27 +72,25 @@ func CompareCategories(one Category, two Category) int {

// compareRuleSpecs returns -1 if one < two, 1 if one > two, 0 otherwise.
func compareRuleSpecs(one *RuleSpec, two *RuleSpec) int {
if one == nil && two == nil {
switch {
case one == nil && two == nil:
return 0
}
if one == nil && two != nil {
case one == nil:
return -1
}
if one != nil && two == nil {
case two == nil:
return 1
}
return strings.Compare(one.ID, two.ID)
}

// compareCategorySpecs returns -1 if one < two, 1 if one > two, 0 otherwise.
func compareCategorySpecs(one *CategorySpec, two *CategorySpec) int {
if one == nil && two == nil {
switch {
case one == nil && two == nil:
return 0
}
if one == nil && two != nil {
case one == nil:
return -1
}
if one != nil && two == nil {
case two == nil:
return 1
}
return strings.Compare(one.ID, two.ID)
Expand Down
88 changes: 88 additions & 0 deletions check/module_name_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Copyright 2024-2025 Buf Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package check_test

import (
"context"
"testing"

"buf.build/go/bufplugin/check"
"buf.build/go/bufplugin/check/checktest"
"github.com/stretchr/testify/require"
)

func TestModuleName(t *testing.T) {
t.Parallel()
const ruleID = "TEST_MODULE_NAME"
spec := &check.Spec{
Rules: []*check.RuleSpec{
{
ID: ruleID,
Default: true,
Purpose: "Purpose.",
Type: check.RuleTypeLint,
Handler: check.RuleHandlerFunc(
func(_ context.Context, responseWriter check.ResponseWriter, request check.Request) error {
for _, fileDescriptor := range request.FileDescriptors() {
message := "<none>"
if moduleName := fileDescriptor.ModuleName(); moduleName != nil {
message = moduleName.String()
}
responseWriter.AddAnnotation(
check.WithMessage(message),
check.WithFileName(fileDescriptor.ProtoreflectFileDescriptor().Path()),
)
}
return nil
},
),
},
},
}

checktest.CheckTest{
Request: &checktest.RequestSpec{
Files: &checktest.ProtoFileSpec{
DirPaths: []string{"testdata/module_name"},
FilePaths: []string{"a.proto"},
ModuleName: "buf.build/acme/weather",
},
},
Spec: spec,
ExpectedAnnotations: []checktest.ExpectedAnnotation{
{
RuleID: ruleID,
Message: "buf.build/acme/weather",
FileLocation: &checktest.ExpectedFileLocation{FileName: "a.proto", EndLine: 8, EndColumn: 1},
},
{
RuleID: ruleID,
Message: "<none>",
FileLocation: &checktest.ExpectedFileLocation{FileName: "b.proto", EndLine: 4, EndColumn: 12},
},
},
}.Run(t)
}

func TestModuleNameInvalidProtoFileSpec(t *testing.T) {
t.Parallel()

_, err := (&checktest.ProtoFileSpec{
DirPaths: []string{"testdata/module_name"},
FilePaths: []string{"a.proto"},
ModuleName: "buf.build/acme",
}).ToFileDescriptors(t.Context())
require.ErrorContains(t, err, `invalid module name "buf.build/acme"`)
}
2 changes: 1 addition & 1 deletion check/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Request interface {
// If empty, all default Rules will be used.
// The returned RuleIDs will be sorted.
//
// This may return more than 250 IDs; the underlying Client implemention is required to do
// This may return more than 250 IDs; the underlying Client implementation is required to do
// any necessary chunking.
//
// RuleHandlers can safely ignore this - the handling of RuleIDs will have already
Expand Down
2 changes: 1 addition & 1 deletion check/response_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (

var errCannotReuseResponseWriter = errors.New("cannot reuse ResponseWriter")

// ResponseWriter is used by plugin implmentations to add Annotations to responses.
// ResponseWriter is used by plugin implementations to add Annotations to responses.
//
// A ResponseWriter is tied to a specific rule, and is passed to a RuleHandler.
// The ID of the Rule will be automatically populated for any added Annotations.
Expand Down
9 changes: 9 additions & 0 deletions check/testdata/module_name/a.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
syntax = "proto3";

package a;

import "b.proto";

message A {
b.B b = 1;
}
5 changes: 5 additions & 0 deletions check/testdata/module_name/b.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
syntax = "proto3";

package b;

message B {}
Loading
Loading