diff --git a/README.md b/README.md index be544fb5..6e08409c 100644 --- a/README.md +++ b/README.md @@ -155,6 +155,7 @@ Some rules support a feature that automatically fixed the problems. - IMPORTS_SORTED - INDENT - QUOTE_CONSISTENT +- ENUM_NAMES_UPPER_CAMEL_CASE | Official | ID | Purpose | |----------|-----------------------------------|--------------------------------------------------------------------------| diff --git a/_testdata/rules/enumNamesUpperCamelCase/invalid.proto b/_testdata/rules/enumNamesUpperCamelCase/invalid.proto new file mode 100644 index 00000000..9d390a54 --- /dev/null +++ b/_testdata/rules/enumNamesUpperCamelCase/invalid.proto @@ -0,0 +1,23 @@ +syntax = "proto3"; + +enum enum { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + +enum Enum_allowing_alias { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + +enum enumAllowing { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + diff --git a/_testdata/rules/enumNamesUpperCamelCase/upperCamelCase.proto b/_testdata/rules/enumNamesUpperCamelCase/upperCamelCase.proto new file mode 100644 index 00000000..eb356287 --- /dev/null +++ b/_testdata/rules/enumNamesUpperCamelCase/upperCamelCase.proto @@ -0,0 +1,23 @@ +syntax = "proto3"; + +enum Enum { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + +enum EnumAllowingAlias { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + +enum EnumAllowing { + option allow_alias = true; + UNKNOWN = 0; + STARTED = 1; + RUNNING = 2 [(custom_option) = "hello world"]; +} + diff --git a/go.mod b/go.mod index b25df6e7..067ee3af 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ require ( github.com/golang/protobuf v1.5.2 github.com/hashicorp/go-hclog v1.0.0 github.com/hashicorp/go-plugin v1.4.3 - github.com/yoheimuta/go-protoparser/v4 v4.4.0 + github.com/yoheimuta/go-protoparser/v4 v4.5.0 google.golang.org/grpc v1.42.0 gopkg.in/yaml.v2 v2.4.0 ) diff --git a/go.sum b/go.sum index d5962957..50ba337d 100644 --- a/go.sum +++ b/go.sum @@ -82,8 +82,8 @@ github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UV github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/yoheimuta/go-protoparser/v4 v4.4.0 h1:aFvfPXEOAuxguLQAlRaSo9hyI1E8VuF+iZVAyP8hc0s= -github.com/yoheimuta/go-protoparser/v4 v4.4.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= +github.com/yoheimuta/go-protoparser/v4 v4.5.0 h1:Qna+FG1vYTtgPTNsPlwLu74aDhUiW7/UwXvAeu5EFvY= +github.com/yoheimuta/go-protoparser/v4 v4.5.0/go.mod h1:AHNNnSWnb0UoL4QgHPiOAg2BniQceFscPI5X/BZNHl8= go.opentelemetry.io/proto/otlp v0.7.0/go.mod h1:PqfVotwruBrMGOCsRd/89rSnXhoiJIqeYNgFYFoEGnI= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= diff --git a/internal/addon/rules/enumNamesUpperCamelCaseRule.go b/internal/addon/rules/enumNamesUpperCamelCaseRule.go index a6e0df59..74d1b2f4 100644 --- a/internal/addon/rules/enumNamesUpperCamelCaseRule.go +++ b/internal/addon/rules/enumNamesUpperCamelCaseRule.go @@ -1,7 +1,9 @@ package rules import ( + "github.com/yoheimuta/go-protoparser/v4/lexer" "github.com/yoheimuta/go-protoparser/v4/parser" + "github.com/yoheimuta/protolint/linter/fixer" "github.com/yoheimuta/protolint/linter/report" "github.com/yoheimuta/protolint/linter/strs" @@ -10,11 +12,17 @@ import ( // EnumNamesUpperCamelCaseRule verifies that all enum names are CamelCase (with an initial capital). // See https://developers.google.com/protocol-buffers/docs/style#enums. -type EnumNamesUpperCamelCaseRule struct{} +type EnumNamesUpperCamelCaseRule struct { + fixMode bool +} // NewEnumNamesUpperCamelCaseRule creates a new EnumNamesUpperCamelCaseRule. -func NewEnumNamesUpperCamelCaseRule() EnumNamesUpperCamelCaseRule { - return EnumNamesUpperCamelCaseRule{} +func NewEnumNamesUpperCamelCaseRule( + fixMode bool, +) EnumNamesUpperCamelCaseRule { + return EnumNamesUpperCamelCaseRule{ + fixMode: fixMode, + } } // ID returns the ID of this rule. @@ -34,20 +42,40 @@ func (r EnumNamesUpperCamelCaseRule) IsOfficial() bool { // Apply applies the rule to the proto. func (r EnumNamesUpperCamelCaseRule) Apply(proto *parser.Proto) ([]report.Failure, error) { + base, err := visitor.NewBaseFixableVisitor(r.ID(), r.fixMode, proto) + if err != nil { + return nil, err + } + v := &enumNamesUpperCamelCaseVisitor{ - BaseAddVisitor: visitor.NewBaseAddVisitor(r.ID()), + BaseFixableVisitor: base, } return visitor.RunVisitor(v, proto, r.ID()) } type enumNamesUpperCamelCaseVisitor struct { - *visitor.BaseAddVisitor + *visitor.BaseFixableVisitor } // VisitEnum checks the enum. func (v *enumNamesUpperCamelCaseVisitor) VisitEnum(enum *parser.Enum) bool { - if !strs.IsUpperCamelCase(enum.EnumName) { - v.AddFailuref(enum.Meta.Pos, "Enum name %q must be UpperCamelCase", enum.EnumName) + name := enum.EnumName + if !strs.IsUpperCamelCase(name) { + expected := strs.ToUpperCamelCase(name) + v.AddFailuref(enum.Meta.Pos, "Enum name %q must be UpperCamelCase like %q", name, expected) + + err := v.Fixer.SearchAndReplace(enum.Meta.Pos, func(lex *lexer.Lexer) fixer.TextEdit { + lex.NextKeyword() + lex.Next() + return fixer.TextEdit{ + Pos: lex.Pos.Offset, + End: lex.Pos.Offset + len(lex.Text) - 1, + NewText: []byte(expected), + } + }) + if err != nil { + panic(err) + } } return false } diff --git a/internal/addon/rules/enumNamesUpperCamelCaseRule_test.go b/internal/addon/rules/enumNamesUpperCamelCaseRule_test.go index 39a67094..2c575bc0 100644 --- a/internal/addon/rules/enumNamesUpperCamelCaseRule_test.go +++ b/internal/addon/rules/enumNamesUpperCamelCaseRule_test.go @@ -77,7 +77,7 @@ func TestEnumNamesUpperCamelCaseRule_Apply(t *testing.T) { Column: 10, }, "ENUM_NAMES_UPPER_CAMEL_CASE", - `Enum name "enumName" must be UpperCamelCase`, + `Enum name "enumName" must be UpperCamelCase like "EnumName"`, ), report.Failuref( meta.Position{ @@ -87,7 +87,7 @@ func TestEnumNamesUpperCamelCaseRule_Apply(t *testing.T) { Column: 20, }, "ENUM_NAMES_UPPER_CAMEL_CASE", - `Enum name "Enum_name" must be UpperCamelCase`, + `Enum name "Enum_name" must be UpperCamelCase like "EnumName"`, ), }, }, @@ -96,7 +96,7 @@ func TestEnumNamesUpperCamelCaseRule_Apply(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - rule := rules.NewEnumNamesUpperCamelCaseRule() + rule := rules.NewEnumNamesUpperCamelCaseRule(false) got, err := rule.Apply(test.inputProto) if err != nil { @@ -109,3 +109,30 @@ func TestEnumNamesUpperCamelCaseRule_Apply(t *testing.T) { }) } } + +func TestEnumNamesUpperCamelCaseRule_Apply_fix(t *testing.T) { + tests := []struct { + name string + inputFilename string + wantFilename string + }{ + { + name: "no fix for a correct proto", + inputFilename: "upperCamelCase.proto", + wantFilename: "upperCamelCase.proto", + }, + { + name: "fix for an incorrect proto", + inputFilename: "invalid.proto", + wantFilename: "upperCamelCase.proto", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + r := rules.NewEnumNamesUpperCamelCaseRule(true) + testApplyFix(t, r, test.inputFilename, test.wantFilename) + }) + } +} diff --git a/internal/addon/rules/quoteConsistentRule_test.go b/internal/addon/rules/quoteConsistentRule_test.go index 3867d457..bbdfbe69 100644 --- a/internal/addon/rules/quoteConsistentRule_test.go +++ b/internal/addon/rules/quoteConsistentRule_test.go @@ -4,6 +4,9 @@ import ( "reflect" "testing" + "github.com/yoheimuta/protolint/linter/rule" + "github.com/yoheimuta/protolint/linter/strs" + "github.com/yoheimuta/protolint/internal/linter/file" "github.com/yoheimuta/protolint/internal/setting_test" @@ -252,6 +255,53 @@ func TestQuoteConsistentRule_Apply(t *testing.T) { } } +func testApplyFix( + t *testing.T, + r rule.Rule, + inputFilename string, + wantFilename string, +) { + dataDir := strs.ToLowerCamelCase(r.ID()) + + input, err := newTestData(setting_test.TestDataPath("rules", dataDir, inputFilename)) + if err != nil { + t.Errorf("got err %v", err) + return + } + + want, err := newTestData(setting_test.TestDataPath("rules", dataDir, wantFilename)) + if err != nil { + t.Errorf("got err %v", err) + return + } + + proto, err := file.NewProtoFile(input.filePath, input.filePath).Parse(false) + if err != nil { + t.Errorf(err.Error()) + return + } + + _, err = r.Apply(proto) + if err != nil { + t.Errorf("got err %v, but want nil", err) + return + } + + got, err := input.data() + if !reflect.DeepEqual(got, want.originData) { + t.Errorf( + "got %s(%v), but want %s(%v)", + string(got), got, + string(want.originData), want.originData, + ) + } + + err = input.restore() + if err != nil { + t.Errorf("got err %v", err) + } +} + func TestQuoteConsistentRule_Apply_fix(t *testing.T) { tests := []struct { name string @@ -282,48 +332,11 @@ func TestQuoteConsistentRule_Apply_fix(t *testing.T) { for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { - rule := rules.NewQuoteConsistentRule( + r := rules.NewQuoteConsistentRule( test.inputQuote, true, ) - - input, err := newTestData(setting_test.TestDataPath("rules", "quoteConsistent", test.inputFilename)) - if err != nil { - t.Errorf("got err %v", err) - return - } - - want, err := newTestData(setting_test.TestDataPath("rules", "quoteConsistent", test.wantFilename)) - if err != nil { - t.Errorf("got err %v", err) - return - } - - proto, err := file.NewProtoFile(input.filePath, input.filePath).Parse(false) - if err != nil { - t.Errorf(err.Error()) - return - } - - _, err = rule.Apply(proto) - if err != nil { - t.Errorf("got err %v, but want nil", err) - return - } - - got, err := input.data() - if !reflect.DeepEqual(got, want.originData) { - t.Errorf( - "got %s(%v), but want %s(%v)", - string(got), got, - string(want.originData), want.originData, - ) - } - - err = input.restore() - if err != nil { - t.Errorf("got err %v", err) - } + testApplyFix(t, r, test.inputFilename, test.wantFilename) }) } } diff --git a/internal/cmd/subcmds/rules.go b/internal/cmd/subcmds/rules.go index de414714..436d42ae 100644 --- a/internal/cmd/subcmds/rules.go +++ b/internal/cmd/subcmds/rules.go @@ -80,7 +80,7 @@ func newAllInternalRules( enumFieldsHaveComment.ShouldFollowGolangStyle, ), - rules.NewEnumNamesUpperCamelCaseRule(), + rules.NewEnumNamesUpperCamelCaseRule(fixMode), rules.NewEnumsHaveCommentRule( enumsHaveComment.ShouldFollowGolangStyle, ), diff --git a/linter/fixer/fixer.go b/linter/fixer/fixer.go index 2c838c95..a9314e3d 100644 --- a/linter/fixer/fixer.go +++ b/linter/fixer/fixer.go @@ -1,20 +1,33 @@ package fixer import ( + "bytes" "io/ioutil" "strings" + "github.com/yoheimuta/go-protoparser/v4/lexer" + "github.com/yoheimuta/go-protoparser/v4/parser/meta" + "github.com/yoheimuta/go-protoparser/v4/parser" "github.com/yoheimuta/protolint/internal/osutil" ) +// TextEdit represents the replacement of the code between Pos and End with the new text. +type TextEdit struct { + Pos int + End int + NewText []byte +} + // Fixer provides the ways to operate the proto content. type Fixer interface { // NOTE: This method is insufficient to process unexpected multi-line contents. ReplaceText(line int, old, new string) ReplaceAll(proc func(lines []string) []string) + SearchAndReplace(startPos meta.Position, lex func(lex *lexer.Lexer) TextEdit) error + Lines() []string } @@ -37,6 +50,7 @@ type BaseFixing struct { content []byte lineEnding string fileName string + textEdits []TextEdit } func newBaseFixing(protoFileName string) (*BaseFixing, error) { @@ -72,6 +86,22 @@ func (f *BaseFixing) ReplaceAll(proc func(lines []string) []string) { f.content = []byte(strings.Join(lines, f.lineEnding)) } +// SearchAndReplace specifies test edits and replaces with them. +func (f *BaseFixing) SearchAndReplace(startPos meta.Position, lex func(lex *lexer.Lexer) TextEdit) error { + r := bytes.NewReader(f.content) + _, err := r.Seek(int64(startPos.Offset), 0) + if err != nil { + return err + } + + l := lexer.NewLexer(r) + t := lex(l) + t.Pos += startPos.Offset + t.End += startPos.Offset + f.textEdits = append(f.textEdits, t) + return nil +} + // Lines returns the line format of f.content. func (f *BaseFixing) Lines() []string { return strings.Split(string(f.content), f.lineEnding) @@ -79,6 +109,13 @@ func (f *BaseFixing) Lines() []string { // Finally writes the fixed content to the file. func (f *BaseFixing) Finally() error { + diff := 0 + for _, t := range f.textEdits { + t.Pos += diff + t.End += diff + f.content = append(f.content[:t.Pos], append(t.NewText, f.content[t.End+1:]...)...) + diff += len(t.NewText) - (t.End - t.Pos + 1) + } return osutil.WriteExistingFile(f.fileName, f.content) } @@ -91,6 +128,11 @@ func (f NopFixing) ReplaceText(line int, old, new string) {} // ReplaceAll noop func (f NopFixing) ReplaceAll(proc func(lines []string) []string) {} +// SearchAndReplace noop +func (f NopFixing) SearchAndReplace(startPos meta.Position, lex func(lexer *lexer.Lexer) TextEdit) error { + return nil +} + // Lines noop. func (f NopFixing) Lines() []string { return []string{} } diff --git a/linter/strs/strs.go b/linter/strs/strs.go index c3087240..d58dcbd6 100644 --- a/linter/strs/strs.go +++ b/linter/strs/strs.go @@ -111,6 +111,36 @@ func ToUpperSnakeCaseFromCamelCase(s string) (string, error) { ), nil } +// ToUpperCamelCase converts s to UpperCamelCase. +func ToUpperCamelCase(s string) string { + if IsUpperSnakeCase(s) { + s = strings.ToLower(s) + } + + var output string + for _, w := range SplitSnakeCaseWord(s) { + output += strings.Title(w) + } + return output +} + +// ToLowerCamelCase converts s to LowerCamelCase. +func ToLowerCamelCase(s string) string { + if IsUpperSnakeCase(s) { + s = strings.ToLower(s) + } + + var output string + for i, w := range SplitSnakeCaseWord(s) { + if i == 0 { + output += w + } else { + output += strings.Title(w) + } + } + return output +} + // toSnake converts s to snake_case. func toSnake(s string) string { output := "" diff --git a/linter/strs/strs_test.go b/linter/strs/strs_test.go index 3581acf2..5b8d338d 100644 --- a/linter/strs/strs_test.go +++ b/linter/strs/strs_test.go @@ -280,6 +280,56 @@ func TestToUpperSnakeCaseFromCamelCase(t *testing.T) { } } +func TestToUpperCamelCase(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "input consists of one word", + input: "account", + want: "Account", + }, + { + name: "input consists of words with an initial capital", + input: "AccountStatus", + want: "AccountStatus", + }, + { + name: "input consists of words without an initial capital", + input: "accountStatus", + want: "AccountStatus", + }, + { + name: "input consists of words without capital letters", + input: "accountstatus", + want: "Accountstatus", + }, + { + name: "input lower_snake_case", + input: "account_status", + want: "AccountStatus", + }, + { + name: "input UPPER_SNAKE_CASE", + input: "ACCOUNT_STATUS", + want: "AccountStatus", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + got := strs.ToUpperCamelCase(test.input) + + if !reflect.DeepEqual(got, test.want) { + t.Errorf("got %v, but want %v", got, test.want) + } + }) + } +} + func TestSplitSnakeCaseWord(t *testing.T) { tests := []struct { name string diff --git a/protolint b/protolint new file mode 100755 index 00000000..b988f6e8 Binary files /dev/null and b/protolint differ