Skip to content

Commit

Permalink
Merge pull request #444 from yoheimuta/support-editions
Browse files Browse the repository at this point in the history
Protobuf Editions Support
  • Loading branch information
yoheimuta authored Dec 1, 2024
2 parents a205cc9 + 7bff355 commit d8f3db4
Show file tree
Hide file tree
Showing 16 changed files with 149 additions and 9 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: 1.21
go-version: 1.23

- name: Lint
run: |
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/goreleaser.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
name: Set up Go
uses: actions/setup-go@v5
with:
go-version: 1.21
go-version: 1.23
-
name: Set up QEMU
uses: docker/setup-qemu-action@v3
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish_wheel.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Run go release to create binaries
uses: actions/setup-go@v5
with:
go-version: 1.21
go-version: 1.23
- name: Run GoReleaser
uses: goreleaser/goreleaser-action@v6
with:
Expand Down
27 changes: 27 additions & 0 deletions _example/proto/editions.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
edition = "2023";
import public "other.proto";
option java_package = "com.example.foo";
enum EnumAllowingAlias {
option allow_alias = true;
EAA_UNSPECIFIED = 0;
EAA_STARTED = 1;
EAA_RUNNING = 1;
EAA_FINISHED = 2 [(custom_option) = "hello world"];
}
message Outer {
option (my_option).a = true;
message Inner { // Level 2
int64 ival = 1 [features.field_presence = LEGACY_REQUIRED];
}
repeated Inner inner_message = 2;
EnumAllowingAlias enum_field = 3;
map<int32, string> my_map = 4;
extensions 20 to 30;
reserved reserved_field;
}
message Foo {
message GroupMessage {
bool a = 1;
}
GroupMessage groupmessage = 2 [features.message_encoding = DELIMITED];
}
12 changes: 12 additions & 0 deletions _testdata/rules/order/invalidEditions.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
edition = "2023";

message Outer {
option (my_option).a = true;
// inner is an inner message.
message Inner { // Level 2
int64 ival = 1;
}
repeated Inner inner_messages = 2;
}

package my.package;
12 changes: 12 additions & 0 deletions _testdata/rules/order/orderEditions.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
edition = "2023";

package my.package;

message Outer {
option (my_option).a = true;
// inner is an inner message.
message Inner { // Level 2
int64 ival = 1;
}
repeated Inner inner_messages = 2;
}
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ require (
github.com/golang/protobuf v1.5.4
github.com/hashicorp/go-hclog v1.6.3
github.com/hashicorp/go-plugin v1.6.1
github.com/yoheimuta/go-protoparser/v4 v4.11.0
github.com/yoheimuta/go-protoparser/v4 v4.12.0
google.golang.org/grpc v1.66.0
google.golang.org/protobuf v1.34.2
gopkg.in/yaml.v2 v2.4.0
Expand All @@ -26,4 +26,4 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20240604185151-ef581f913117 // indirect
)

go 1.21
go 1.23
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
github.com/yoheimuta/go-protoparser/v4 v4.11.0 h1:zhP3R1bzopFKOco4YouXR7X126ggQX3nQ12OcW958CA=
github.com/yoheimuta/go-protoparser/v4 v4.11.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8=
github.com/yoheimuta/go-protoparser/v4 v4.12.0 h1:j5J7NqsUB2tUczF11kuLjeWnc+DMF/pQrPe5HkAt1zU=
github.com/yoheimuta/go-protoparser/v4 v4.12.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8=
golang.org/x/net v0.26.0 h1:soB7SVo0PWrY4vPW/+ay0jKDNScG2X9wFeYlXIvJsOQ=
golang.org/x/net v0.26.0/go.mod h1:5YKkiSynbBIh3p6iOc/vibscux0x38BZDkn8sCUPxHE=
golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
8 changes: 8 additions & 0 deletions internal/addon/rules/fileHasCommentRule.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,11 @@ func (v *fileHasCommentVisitor) VisitSyntax(s *parser.Syntax) bool {
}
return false
}

// VisitEdition checks the syntax.
func (v *fileHasCommentVisitor) VisitEdition(s *parser.Edition) bool {
if !hasComment(s.Comments) {
v.AddFailuref(s.Meta.Pos, `File should start with a doc comment`)
}
return false
}
8 changes: 8 additions & 0 deletions internal/addon/rules/indentRule.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,14 @@ func (v indentVisitor) VisitSyntax(s *parser.Syntax) (next bool) {
return false
}

func (v indentVisitor) VisitEdition(s *parser.Edition) (next bool) {
v.validateIndentLeading(s.Meta.Pos)
for _, comment := range s.Comments {
v.validateIndentLeading(comment.Meta.Pos)
}
return false
}

func (v indentVisitor) validateIndentLeading(
pos meta.Position,
) {
Expand Down
28 changes: 24 additions & 4 deletions internal/addon/rules/orderRule.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
)

// OrderRule verifies that all files should be ordered in the following manner:
// 1. Syntax
// 1. Syntax or Edition
// 2. Package
// 3. Imports (sorted)
// 4. File options
Expand Down Expand Up @@ -100,6 +100,16 @@ func (v *orderVisitor) VisitSyntax(s *parser.Syntax) bool {
return false
}

func (v *orderVisitor) VisitEdition(e *parser.Edition) bool {
next := v.machine.transit(v.state, syntaxVisitEvent)
if next == invalidOrderState {
v.AddFailuref(e.Meta.Pos, "Edition should be located at the top. Check if the file is ordered in the correct manner.")
}
v.state = syntaxOrderState
v.formatter.edition = e
return false
}

func (v *orderVisitor) VisitPackage(p *parser.Package) bool {
next := v.machine.transit(v.state, packageVisitEvent)
if next == invalidOrderState {
Expand Down Expand Up @@ -273,6 +283,7 @@ func (i indexedVisitee) isContiguous(a indexedVisitee) bool {

type formatter struct {
syntax *parser.Syntax
edition *parser.Edition
pkg *parser.Package
imports []indexedVisitee
options []indexedVisitee
Expand All @@ -282,7 +293,7 @@ type formatter struct {

func (f formatter) index() int {
idx := 0
if f.syntax != nil {
if f.syntax != nil || f.edition != nil {
idx = 1
}
if f.pkg != nil {
Expand Down Expand Up @@ -329,6 +340,8 @@ func newVisiteeLine(elm parser.Visitee) line {
switch e := elm.(type) {
case *parser.Syntax:
return newLine(e.Meta, e.Comments, e.InlineComment)
case *parser.Edition:
return newLine(e.Meta, e.Comments, e.InlineComment)
case *parser.Package:
return newLine(e.Meta, e.Comments, e.InlineComment)
case *parser.Import:
Expand Down Expand Up @@ -385,8 +398,15 @@ func (w *writer) removeLastRedundantN() {
func (f formatter) format(content []byte) []byte {
w := writer{content: content}

sl := newVisiteeLine(f.syntax)
w.writeNN(sl)
if f.syntax != nil {
sl := newVisiteeLine(f.syntax)
w.writeNN(sl)
}

if f.edition != nil {
el := newVisiteeLine(f.edition)
w.writeNN(el)
}

if f.pkg != nil {
pl := newVisiteeLine(f.pkg)
Expand Down
27 changes: 27 additions & 0 deletions internal/addon/rules/orderRule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,23 @@ func TestOrderRule_Apply(t *testing.T) {
},
},
},
{
name: "no failures with editions for proto including all elements in order",
inputProto: &parser.Proto{
ProtoBody: []parser.Visitee{
&parser.Edition{},
&parser.Package{},
&parser.Import{},
&parser.Import{},
&parser.Option{},
&parser.Option{},
&parser.Message{},
&parser.Enum{},
&parser.Service{},
&parser.Extend{},
},
},
},
{
name: "no failures for proto omitting the syntax",
inputProto: &parser.Proto{
Expand Down Expand Up @@ -366,11 +383,21 @@ func TestOrderRule_Apply_fix(t *testing.T) {
inputFilename: "order.proto",
wantFilename: "order.proto",
},
{
name: "no fix for a correct proto with editions",
inputFilename: "orderEditions.proto",
wantFilename: "orderEditions.proto",
},
{
name: "fix for an incorrect proto",
inputFilename: "invalid.proto",
wantFilename: "order.proto",
},
{
name: "fix for an incorrect proto with editions",
inputFilename: "invalidEditions.proto",
wantFilename: "orderEditions.proto",
},
{
name: "fix for an incorrect proto while keeping contiguous misc elements",
inputFilename: "invalidContiguousMisc.proto",
Expand Down
10 changes: 10 additions & 0 deletions internal/addon/rules/quoteConsistentRule.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ func (v quoteConsistentVisitor) VisitSyntax(s *parser.Syntax) bool {
return false
}

func (v quoteConsistentVisitor) VisitEdition(s *parser.Edition) bool {
str := s.EditionQuote
converted := convertConsistentQuote(str, v.quote)
if str != converted {
v.AddFailuref(s.Meta.Pos, "Quoted string should be %s but was %s.", converted, str)
v.Fixer.ReplaceText(s.Meta.Pos.Line, str, converted)
}
return false
}

func (v quoteConsistentVisitor) VisitImport(i *parser.Import) (next bool) {
str := i.Location
converted := convertConsistentQuote(str, v.quote)
Expand Down
3 changes: 3 additions & 0 deletions linter/visitor/baseVisitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ func (BaseVisitor) Finally() error { return nil }
// VisitComment works noop.
func (BaseVisitor) VisitComment(*parser.Comment) {}

// VisitEdition works noop.
func (BaseVisitor) VisitEdition(*parser.Edition) (next bool) { return true }

// VisitEmptyStatement works noop.
func (BaseVisitor) VisitEmptyStatement(*parser.EmptyStatement) (next bool) { return true }

Expand Down
4 changes: 4 additions & 0 deletions linter/visitor/extendedAutoDisableVisitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ func (v *extendedAutoDisableVisitor) Finally() error {
}
func (v *extendedAutoDisableVisitor) Failures() []report.Failure { return v.inner.Failures() }

func (v *extendedAutoDisableVisitor) VisitEdition(s *parser.Edition) (next bool) {
return v.inner.VisitEdition(s)
}

func (v *extendedAutoDisableVisitor) VisitEmptyStatement(e *parser.EmptyStatement) (next bool) {
return v.inner.VisitEmptyStatement(e)
}
Expand Down
7 changes: 7 additions & 0 deletions linter/visitor/extendedDisableRuleVisitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ func (v extendedDisableRuleVisitor) VisitComment(c *parser.Comment) {
v.inner.VisitComment(c)
}

func (v extendedDisableRuleVisitor) VisitEdition(s *parser.Edition) (next bool) {
if v.interpreter.Interpret(s.Comments, s.InlineComment) {
return true
}
return v.inner.VisitEdition(s)
}

func (v extendedDisableRuleVisitor) VisitEnum(e *parser.Enum) (next bool) {
if v.interpreter.Interpret(e.Comments, e.InlineComment, e.InlineCommentBehindLeftCurly) {
return true
Expand Down

0 comments on commit d8f3db4

Please sign in to comment.