Merge pull request #210 from yoheimuta/field-fixer

Add field fixers

README.md

@@ -169,9 +169,9 @@ See Fixable columns below.
 | Yes | ✅ | SERVICE_NAMES_UPPER_CAMEL_CASE    | Verifies that all service names are CamelCase (with an initial capital). |
 | Yes | _  | MAX_LINE_LENGTH    | Enforces a maximum line length. The length of a line is defined as the number of Unicode characters in the line. The default is 80 characters. You can configure the detail with `.protolint.yaml`. |
 | Yes | ✅ | INDENT    | Enforces a consistent indentation style. The default style is 2 spaces. Inserting appropriate new lines is also forced by default. You can configure the detail with `.protolint.yaml`. |
-| Yes | _  | PROTO3_FIELDS_AVOID_REQUIRED      | Verifies that all fields should avoid required for proto3.            |
+| Yes | ✅ | PROTO3_FIELDS_AVOID_REQUIRED      | Verifies that all fields should avoid required for proto3.            |
 | Yes | _  | PROTO3_GROUPS_AVOID      | Verifies that all groups should be avoided for proto3.            |
-| Yes | _  | REPEATED_FIELD_NAMES_PLURALIZED   | Verifies that repeated field names are pluralized names.            |
+| Yes | ✅ | REPEATED_FIELD_NAMES_PLURALIZED   | Verifies that repeated field names are pluralized names.            |
 | Yes | ✅ | QUOTE_CONSISTENT   | Verifies that the use of quote for strings is consistent. The default is double quoted. You can configure the specific quote with `.protolint.yaml`.          |
 | No | _  | SERVICE_NAMES_END_WITH    | Enforces a consistent suffix for service names. You can configure the specific suffix with `.protolint.yaml`. |
 | No | _  | FIELD_NAMES_EXCLUDE_PREPOSITIONS | Verifies that all field names don't include prepositions (e.g. "for", "during", "at"). You can configure the specific prepositions and excluded keywords with `.protolint.yaml`. |

_testdata/rules/proto3FieldsAvoidRequired/avoid_required.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+message first_outer {
+    // inner is an inner message.
+    message first_inner {   // Level 2
+                            repeated inner innerMessage = 2;
+                            inner innerRequired = 5;
+    }
+}
+
+message second_outer {
+    int64 hoge = 4;
+    string fuga = 3;
+}
+

_testdata/rules/proto3FieldsAvoidRequired/invalid.proto

@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+message first_outer {
+    // inner is an inner message.
+    message first_inner {   // Level 2
+                            repeated inner innerMessage = 2;
+                            required inner innerRequired = 5;
+    }
+}
+
+message second_outer {
+    required int64 hoge = 4;
+    string fuga = 3;
+}
+

_testdata/rules/repeatedFieldNamesPluralized/invalid.proto

@@ -0,0 +1,18 @@
+syntax = "proto3";
+
+message first_outer {
+    // inner is an inner message.
+    message first_inner {   // Level 2
+                            repeated inner innerMessage = 2;
+    }
+    repeated string My_map = 4;
+    group Result = 8 {
+        repeated string url = 9;
+        int64 amount = 10;
+    }
+}
+
+message second_outer {
+    repeated google.protobuf.Empty OneofEmpty = 20;
+    string oneof_String = 21;
+}

_testdata/rules/repeatedFieldNamesPluralized/pluralized.proto

@@ -0,0 +1,18 @@
+syntax = "proto3";
+
+message first_outer {
+    // inner is an inner message.
+    message first_inner {   // Level 2
+                            repeated inner innerMessages = 2;
+    }
+    repeated string My_maps = 4;
+    group Result = 8 {
+        repeated string urls = 9;
+        int64 amount = 10;
+    }
+}
+
+message second_outer {
+    repeated google.protobuf.Empty OneofEmpties = 20;
+    string oneof_String = 21;
+}

internal/addon/rules/proto3FieldsAvoidRequiredRule.go

@@ -1,19 +1,26 @@
 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/visitor"
 )
 
 // Proto3FieldsAvoidRequiredRule verifies that all fields should avoid required for proto3.
 // See https://developers.google.com/protocol-buffers/docs/style#things-to-avoid
 type Proto3FieldsAvoidRequiredRule struct {
+	fixMode bool
 }
 
 // NewProto3FieldsAvoidRequiredRule creates a new Proto3FieldsAvoidRequiredRule.
-func NewProto3FieldsAvoidRequiredRule() Proto3FieldsAvoidRequiredRule {
-	return Proto3FieldsAvoidRequiredRule{}
+func NewProto3FieldsAvoidRequiredRule(
+	fixMode bool,
+) Proto3FieldsAvoidRequiredRule {
+	return Proto3FieldsAvoidRequiredRule{
+		fixMode: fixMode,
+	}
 }
 
 // ID returns the ID of this rule.
@@ -33,14 +40,19 @@ func (r Proto3FieldsAvoidRequiredRule) IsOfficial() bool {
 
 // Apply applies the rule to the proto.
 func (r Proto3FieldsAvoidRequiredRule) Apply(proto *parser.Proto) ([]report.Failure, error) {
+	base, err := visitor.NewBaseFixableVisitor(r.ID(), r.fixMode, proto)
+	if err != nil {
+		return nil, err
+	}
+
 	v := &proto3FieldsAvoidRequiredVisitor{
-		BaseAddVisitor: visitor.NewBaseAddVisitor(r.ID()),
+		BaseFixableVisitor: base,
 	}
 	return visitor.RunVisitor(v, proto, r.ID())
 }
 
 type proto3FieldsAvoidRequiredVisitor struct {
-	*visitor.BaseAddVisitor
+	*visitor.BaseFixableVisitor
 	isProto3 bool
 }
 
@@ -54,6 +66,18 @@ func (v *proto3FieldsAvoidRequiredVisitor) VisitSyntax(s *parser.Syntax) bool {
 func (v *proto3FieldsAvoidRequiredVisitor) VisitField(field *parser.Field) bool {
 	if v.isProto3 && field.IsRequired {
 		v.AddFailuref(field.Meta.Pos, `Field %q should avoid required for proto3`, field.FieldName)
+
+		err := v.Fixer.SearchAndReplace(field.Meta.Pos, func(lex *lexer.Lexer) fixer.TextEdit {
+			lex.NextKeyword()
+			return fixer.TextEdit{
+				Pos:     lex.Pos.Offset,
+				End:     lex.Pos.Offset + len(lex.Text),
+				NewText: []byte(""),
+			}
+		})
+		if err != nil {
+			panic(err)
+		}
 	}
 	return false
 }

internal/addon/rules/proto3FieldsAvoidRequiredRule_test.go

@@ -148,7 +148,7 @@ func TestProto3FieldsAvoidRequiredRule_Apply(t *testing.T) {
 	for _, test := range tests {
 		test := test
 		t.Run(test.name, func(t *testing.T) {
-			rule := rules.NewProto3FieldsAvoidRequiredRule()
+			rule := rules.NewProto3FieldsAvoidRequiredRule(false)
 
 			got, err := rule.Apply(test.inputProto)
 			if err != nil {
@@ -161,3 +161,30 @@ func TestProto3FieldsAvoidRequiredRule_Apply(t *testing.T) {
 		})
 	}
 }
+
+func TestProto3FieldsAvoidRequiredRule_Apply_fix(t *testing.T) {
+	tests := []struct {
+		name          string
+		inputFilename string
+		wantFilename  string
+	}{
+		{
+			name:          "no fix for a correct proto",
+			inputFilename: "avoid_required.proto",
+			wantFilename:  "avoid_required.proto",
+		},
+		{
+			name:          "fix for an incorrect proto",
+			inputFilename: "invalid.proto",
+			wantFilename:  "avoid_required.proto",
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			r := rules.NewProto3FieldsAvoidRequiredRule(true)
+			testApplyFix(t, r, test.inputFilename, test.wantFilename)
+		})
+	}
+}

internal/addon/rules/repeatedFieldNamesPluralizedRule.go

@@ -1,7 +1,10 @@
 package rules
 
 import (
+	"github.com/yoheimuta/go-protoparser/v4/lexer"
+	"github.com/yoheimuta/go-protoparser/v4/lexer/scanner"
 	"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"
 	"github.com/yoheimuta/protolint/linter/visitor"
@@ -14,6 +17,7 @@ type RepeatedFieldNamesPluralizedRule struct {
 	singularRules    map[string]string
 	uncountableRules []string
 	irregularRules   map[string]string
+	fixMode          bool
 }
 
 // NewRepeatedFieldNamesPluralizedRule creates a new RepeatedFieldNamesPluralizedRule.
@@ -22,12 +26,14 @@ func NewRepeatedFieldNamesPluralizedRule(
 	singularRules map[string]string,
 	uncountableRules []string,
 	irregularRules map[string]string,
+	fixMode bool,
 ) RepeatedFieldNamesPluralizedRule {
 	return RepeatedFieldNamesPluralizedRule{
 		pluralRules:      pluralRules,
 		singularRules:    singularRules,
 		uncountableRules: uncountableRules,
 		irregularRules:   irregularRules,
+		fixMode:          fixMode,
 	}
 }
 
@@ -62,34 +68,77 @@ func (r RepeatedFieldNamesPluralizedRule) Apply(proto *parser.Proto) ([]report.F
 		c.AddIrregularRule(k, v)
 	}
 
-	v := &repeatedFieldNamesPluralizedCaseVisitor{
-		BaseAddVisitor:  visitor.NewBaseAddVisitor(r.ID()),
-		pluralizeClient: c,
+	base, err := visitor.NewBaseFixableVisitor(r.ID(), r.fixMode, proto)
+	if err != nil {
+		return nil, err
+	}
+
+	v := &repeatedFieldNamesPluralizedVisitor{
+		BaseFixableVisitor: base,
+		pluralizeClient:    c,
 	}
 	return visitor.RunVisitor(v, proto, r.ID())
 }
 
-type repeatedFieldNamesPluralizedCaseVisitor struct {
-	*visitor.BaseAddVisitor
+type repeatedFieldNamesPluralizedVisitor struct {
+	*visitor.BaseFixableVisitor
 	pluralizeClient *strs.PluralizeClient
 }
 
 // VisitField checks the field.
-func (v *repeatedFieldNamesPluralizedCaseVisitor) VisitField(field *parser.Field) bool {
+func (v *repeatedFieldNamesPluralizedVisitor) VisitField(field *parser.Field) bool {
 	got := field.FieldName
 	want := v.pluralizeClient.ToPlural(got)
 	if field.IsRepeated && got != want {
 		v.AddFailuref(field.Meta.Pos, "Repeated field name %q must be pluralized name %q", got, want)
+
+		err := v.Fixer.SearchAndReplace(field.Meta.Pos, func(lex *lexer.Lexer) fixer.TextEdit {
+			lex.NextKeyword()
+			switch lex.Token {
+			case scanner.TREPEATED, scanner.TREQUIRED, scanner.TOPTIONAL:
+			default:
+				lex.UnNext()
+			}
+			parseType(lex)
+			lex.Next()
+			return fixer.TextEdit{
+				Pos:     lex.Pos.Offset,
+				End:     lex.Pos.Offset + len(lex.Text) - 1,
+				NewText: []byte(want),
+			}
+		})
+		if err != nil {
+			panic(err)
+		}
 	}
 	return false
 }
 
 // VisitGroupField checks the group field.
-func (v *repeatedFieldNamesPluralizedCaseVisitor) VisitGroupField(field *parser.GroupField) bool {
+func (v *repeatedFieldNamesPluralizedVisitor) VisitGroupField(field *parser.GroupField) bool {
 	got := field.GroupName
 	want := v.pluralizeClient.ToPlural(got)
 	if field.IsRepeated && got != want {
 		v.AddFailuref(field.Meta.Pos, "Repeated group name %q must be pluralized name %q", got, want)
+
+		err := v.Fixer.SearchAndReplace(field.Meta.Pos, func(lex *lexer.Lexer) fixer.TextEdit {
+			lex.NextKeyword()
+			switch lex.Token {
+			case scanner.TREPEATED, scanner.TREQUIRED, scanner.TOPTIONAL:
+			default:
+				lex.UnNext()
+			}
+			lex.NextKeyword()
+			lex.Next()
+			return fixer.TextEdit{
+				Pos:     lex.Pos.Offset,
+				End:     lex.Pos.Offset + len(lex.Text) - 1,
+				NewText: []byte(want),
+			}
+		})
+		if err != nil {
+			panic(err)
+		}
 	}
 	return true
 }

internal/addon/rules/repeatedFieldNamesPluralizedRule_test.go

@@ -186,6 +186,7 @@ func TestRepeatedFieldNamesPluralizedRule_Apply(t *testing.T) {
 				test.singularRules,
 				test.uncountableRules,
 				test.irregularRules,
+				false,
 			)
 
 			got, err := rule.Apply(test.inputProto)
@@ -199,3 +200,40 @@ func TestRepeatedFieldNamesPluralizedRule_Apply(t *testing.T) {
 		})
 	}
 }
+
+func TestRepeatedFieldNamesPluralizedRule_Apply_fix(t *testing.T) {
+	tests := []struct {
+		name             string
+		pluralRules      map[string]string
+		singularRules    map[string]string
+		uncountableRules []string
+		irregularRules   map[string]string
+		inputFilename    string
+		wantFilename     string
+	}{
+		{
+			name:          "no fix for a correct proto",
+			inputFilename: "pluralized.proto",
+			wantFilename:  "pluralized.proto",
+		},
+		{
+			name:          "fix for an incorrect proto",
+			inputFilename: "invalid.proto",
+			wantFilename:  "pluralized.proto",
+		},
+	}
+
+	for _, test := range tests {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			r := rules.NewRepeatedFieldNamesPluralizedRule(
+				test.pluralRules,
+				test.singularRules,
+				test.uncountableRules,
+				test.irregularRules,
+				true,
+			)
+			testApplyFix(t, r, test.inputFilename, test.wantFilename)
+		})
+	}
+}

internal/cmd/subcmds/rules.go

@@ -95,13 +95,14 @@ func newAllInternalRules(
 		rules.NewFieldsHaveCommentRule(
 			fieldsHaveComment.ShouldFollowGolangStyle,
 		),
-		rules.NewProto3FieldsAvoidRequiredRule(),
+		rules.NewProto3FieldsAvoidRequiredRule(fixMode),
 		rules.NewProto3GroupsAvoidRule(),
 		rules.NewRepeatedFieldNamesPluralizedRule(
 			repeatedFieldNamesPluralized.PluralRules,
 			repeatedFieldNamesPluralized.SingularRules,
 			repeatedFieldNamesPluralized.UncountableRules,
 			repeatedFieldNamesPluralized.IrregularRules,
+			fixMode,
 		),
 
 		rules.NewMessageNamesUpperCamelCaseRule(fixMode),