Skip to content

Commit

Permalink
Make "buf lint" work correctly with top-level extensions (#2920)
Browse files Browse the repository at this point in the history 
This fixes a potential panic introduced in a previous PR and
fleshes out support (including tests) for running lint checks
over top-level extensions (which in past versions were
inadvertently excluded from lint checks).
  • Loading branch information
@jhump
jhump committed Apr 29, 2024
1 parent 08b93e0 commit db1d424
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 33 deletions.
62 changes: 40 additions & 22 deletions private/bufpkg/bufcheck/buflint/buflint_test.go
View file
Expand Up @@ -329,6 +329,8 @@ func TestRunFieldLowerSnakeCase(t *testing.T) {
bufanalysistesting.NewFileAnnotation(t, "a.proto", 30, 11, 30, 20, "FIELD_LOWER_SNAKE_CASE"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 31, 11, 31, 21, "FIELD_LOWER_SNAKE_CASE"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 32, 11, 32, 21, "FIELD_LOWER_SNAKE_CASE"),
bufanalysistesting.NewFileAnnotation(t, "b.proto", 10, 23, 10, 24, "FIELD_LOWER_SNAKE_CASE"),
bufanalysistesting.NewFileAnnotation(t, "b.proto", 17, 19, 17, 20, "FIELD_LOWER_SNAKE_CASE"),
)
}

Expand All @@ -345,30 +347,46 @@ func TestRunFieldNoDescriptor(t *testing.T) {
bufanalysistesting.NewFileAnnotation(t, "a.proto", 11, 19, 11, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 12, 19, 12, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 13, 19, 13, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 19, 23, 19, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 20, 23, 20, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 21, 23, 21, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 22, 23, 22, 34, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 23, 23, 23, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 24, 23, 24, 34, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 25, 23, 25, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 26, 23, 26, 37, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 28, 21, 28, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 29, 21, 29, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 30, 21, 30, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 31, 21, 31, 32, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 32, 21, 32, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 33, 21, 33, 32, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 34, 21, 34, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 35, 21, 35, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 37, 19, 37, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 38, 19, 38, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 39, 19, 39, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 40, 19, 40, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 41, 19, 41, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 42, 19, 42, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 43, 19, 43, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 44, 19, 44, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 22, 23, 22, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 23, 23, 23, 34, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 24, 23, 24, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 25, 23, 25, 34, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 26, 23, 26, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 27, 23, 27, 37, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 30, 27, 30, 38, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 31, 27, 31, 39, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 32, 27, 32, 38, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 33, 27, 33, 39, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 34, 27, 34, 41, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 35, 27, 35, 39, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 36, 27, 36, 38, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 37, 27, 37, 39, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 41, 21, 41, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 42, 21, 42, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 43, 21, 43, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 44, 21, 44, 32, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 45, 21, 45, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 46, 21, 46, 32, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 47, 21, 47, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 48, 21, 48, 35, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 50, 19, 50, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 51, 19, 51, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 52, 19, 52, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 53, 19, 53, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 54, 19, 54, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 55, 19, 55, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 56, 19, 56, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 57, 19, 57, 33, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 61, 19, 61, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 62, 19, 62, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 63, 19, 63, 29, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 64, 19, 64, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 65, 19, 65, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 66, 19, 66, 30, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 67, 19, 67, 31, "FIELD_NO_DESCRIPTOR"),
bufanalysistesting.NewFileAnnotation(t, "a.proto", 68, 19, 68, 33, "FIELD_NO_DESCRIPTOR"),
)
}

Expand Down
27 changes: 16 additions & 11 deletions private/bufpkg/bufcheck/buflint/internal/buflintcheck/buflintcheck.go
View file
Expand Up @@ -268,21 +268,24 @@ func checkEnumZeroValueSuffix(add addFunc, enumValue bufprotosource.EnumValue, s
var CheckFieldLowerSnakeCase = newFieldCheckFunc(checkFieldLowerSnakeCase)

func checkFieldLowerSnakeCase(add addFunc, field bufprotosource.Field) error {
if message := field.ParentMessage(); message != nil && message.IsMapEntry() {
message := field.ParentMessage()
if message != nil && message.IsMapEntry() {
// this check should always pass anyways but just in case
return nil
}
name := field.Name()
expectedName := fieldToLowerSnakeCase(name)
if name != expectedName {
var otherLocs []bufprotosource.Location
if message != nil {
// also check the message for this comment ignore
// this allows users to set this "globally" for a message
otherLocs = []bufprotosource.Location{message.Location()}
}
add(
field,
field.NameLocation(),
// also check the message for this comment ignore
// this allows users to set this "globally" for a message
[]bufprotosource.Location{
field.ParentMessage().Location(),
},
otherLocs,
"Field name %q should be lower_snake_case, such as %q.",
name,
expectedName,
Expand All @@ -297,14 +300,16 @@ var CheckFieldNoDescriptor = newFieldCheckFunc(checkFieldNoDescriptor)
func checkFieldNoDescriptor(add addFunc, field bufprotosource.Field) error {
name := field.Name()
if strings.ToLower(strings.Trim(name, "_")) == "descriptor" {
var otherLocs []bufprotosource.Location
if message := field.ParentMessage(); message != nil {
// also check the message for this comment ignore
// this allows users to set this "globally" for a message
otherLocs = []bufprotosource.Location{message.Location()}
}
add(
field,
field.NameLocation(),
// also check the message for this comment ignore
// this allows users to set this "globally" for a message
[]bufprotosource.Location{
field.ParentMessage().Location(),
},
otherLocs,
`Field name %q cannot be any capitalization of "descriptor" with any number of prefix or suffix underscores.`,
name,
)
Expand Down
18 changes: 18 additions & 0 deletions private/bufpkg/bufcheck/buflint/testdata/field_lower_snake_case/b.proto
View file
@@ -0,0 +1,18 @@
syntax = "proto2";

package b;

message Foo {
extensions 1 to 100;
message Bar {
extend Foo {
optional string a = 1;
optional string B = 2;
}
}
}

extend Foo {
optional string c = 3;
optional string D = 4;
}
24 changes: 24 additions & 0 deletions private/bufpkg/bufcheck/buflint/testdata/field_no_descriptor/a.proto
View file
Expand Up @@ -11,6 +11,7 @@ message One {
optional string descriptor_ = 6;
optional string descriptor__ = 7;
optional string __descriptor__ = 8;
extensions 10 to 100;
}

message Two {
Expand All @@ -24,6 +25,18 @@ message Two {
optional string descriptor_ = 6;
optional string descriptor__ = 7;
optional string __descriptor__ = 8;
message Five {
extend One {
optional string descriptor = 10;
optional string Descriptor = 11;
optional string descRiptor = 12;
optional string _descriptor = 13;
optional string __descriptor = 14;
optional string descriptor_ = 15;
optional string descriptor__ = 16;
optional string __descriptor__ = 17;
}
}
}
optional string descriptor = 1;
optional string Descriptor = 2;
Expand All @@ -43,3 +56,14 @@ message Two {
optional string descriptor__ = 7;
optional string __descriptor__ = 8;
}

extend One {
optional string descriptor = 20;
optional string Descriptor = 21;
optional string descRiptor = 22;
optional string _descriptor = 23;
optional string __descriptor = 24;
optional string descriptor_ = 25;
optional string descriptor__ = 26;
optional string __descriptor__ = 27;
}

0 comments on commit db1d424

Please sign in to comment.