Skip to content

Commit ad3fbe1

Browse files
yoheimutayoheimuta
authored
Merge pull request #493 from frolovo22/fix-linter-started-from-zero-numbers
fix FIELD_NUMBERS_ORDER_ASCENDING rule. enums could starts from 0
2 parents 9c10008 + 5df24fa commit ad3fbe1

File tree

2 files changed

+180
-15
lines changed

2 files changed

+180
-15
lines changed

‎internal/addon/rules/fieldNumbersOrderAscendingRule.go‎

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,29 @@ func (v *fieldNumbersOrderAscendingVisitor) VisitMessage(message *parser.Message
6363
continue
6464
}
6565

66-
number, isError := v.isAscending(field.Meta.Pos, field.FieldName, field.FieldNumber, lastName, lastNumber)
66+
number, err := strconv.Atoi(field.FieldNumber)
67+
if err != nil {
68+
v.AddFailuref(
69+
field.Meta.Pos,
70+
"field number '%s' is not a number",
71+
field.FieldNumber,
72+
)
73+
74+
hasError = true
75+
continue
76+
}
77+
78+
if number <= 0 {
79+
v.AddFailuref(
80+
field.Meta.Pos,
81+
"field number should be positive integer",
82+
)
83+
84+
hasError = true
85+
continue
86+
}
87+
88+
number, isError := v.isAscending(field.Meta.Pos, field.FieldName, number, lastName, lastNumber)
6789
if isError {
6890
hasError = true
6991
}
@@ -78,7 +100,7 @@ func (v *fieldNumbersOrderAscendingVisitor) VisitMessage(message *parser.Message
78100
// VisitEnum checks the enum
79101
func (v *fieldNumbersOrderAscendingVisitor) VisitEnum(enum *parser.Enum) bool {
80102
var (
81-
lastNumber int
103+
lastNumber int = -1
82104
lastIdent string
83105
hasError bool
84106
)
@@ -89,7 +111,29 @@ func (v *fieldNumbersOrderAscendingVisitor) VisitEnum(enum *parser.Enum) bool {
89111
continue
90112
}
91113

92-
number, isError := v.isAscending(field.Meta.Pos, field.Ident, field.Number, lastIdent, lastNumber)
114+
number, err := strconv.Atoi(field.Number)
115+
if err != nil {
116+
v.AddFailuref(
117+
field.Meta.Pos,
118+
"field number '%s' is not a number",
119+
field.Number,
120+
)
121+
122+
hasError = true
123+
continue
124+
}
125+
126+
if number < 0 {
127+
v.AddFailuref(
128+
field.Meta.Pos,
129+
"field number should be positive integer",
130+
)
131+
132+
hasError = true
133+
continue
134+
}
135+
136+
number, isError := v.isAscending(field.Meta.Pos, field.Ident, number, lastIdent, lastNumber)
93137
if isError {
94138
hasError = true
95139
}
@@ -102,19 +146,8 @@ func (v *fieldNumbersOrderAscendingVisitor) VisitEnum(enum *parser.Enum) bool {
102146
}
103147

104148
func (v *fieldNumbersOrderAscendingVisitor) isAscending(
105-
pos meta.Position, fieldName, fieldNumber, lastName string, lastNumber int,
149+
pos meta.Position, fieldName string, number int, lastName string, lastNumber int,
106150
) (curNumber int, hasError bool) {
107-
number, err := strconv.Atoi(fieldNumber)
108-
if err != nil {
109-
v.AddFailuref(
110-
pos,
111-
"field number '%s' is not a number",
112-
fieldNumber,
113-
)
114-
115-
return 0, true
116-
}
117-
118151
if number == lastNumber {
119152
v.AddFailuref(
120153
pos,

‎internal/addon/rules/fieldNumbersOrderAscendingRule_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,27 @@ func TestFieldNumbersOrderAscendingRule_Apply(t *testing.T) {
3838
},
3939
wantFailures: nil,
4040
},
41+
{
42+
name: "no failures for proto enum started from 0",
43+
inputProto: &parser.Proto{
44+
ProtoBody: []parser.Visitee{
45+
&parser.Service{},
46+
&parser.Enum{
47+
EnumBody: []parser.Visitee{
48+
&parser.EnumField{
49+
Ident: "VALUE_UNSPECIFIED",
50+
Number: "0",
51+
},
52+
&parser.EnumField{
53+
Ident: "FIRST_VALUE",
54+
Number: "1",
55+
},
56+
},
57+
},
58+
},
59+
},
60+
wantFailures: nil,
61+
},
4162
{
4263
name: "no failures for proto enum with ascending order numbers with gap",
4364
inputProto: &parser.Proto{
@@ -59,6 +80,34 @@ func TestFieldNumbersOrderAscendingRule_Apply(t *testing.T) {
5980
},
6081
wantFailures: nil,
6182
},
83+
{
84+
name: "failures for proto enum with negative number",
85+
inputProto: &parser.Proto{
86+
ProtoBody: []parser.Visitee{
87+
&parser.Service{},
88+
&parser.Enum{
89+
EnumBody: []parser.Visitee{
90+
&parser.EnumField{
91+
Ident: "FIRST_VALUE",
92+
Number: "-1",
93+
},
94+
&parser.EnumField{
95+
Ident: "SECOND_VALUE",
96+
Number: "2",
97+
},
98+
},
99+
},
100+
},
101+
},
102+
wantFailures: []report.Failure{
103+
report.Failuref(
104+
meta.Position{},
105+
"FIELD_NUMBERS_ORDER_ASCENDING",
106+
string(rule.SeverityError),
107+
"field number should be positive integer",
108+
),
109+
},
110+
},
62111
{
63112
name: "failures for proto enum with duplicated numbers",
64113
inputProto: &parser.Proto{
@@ -279,6 +328,89 @@ func TestFieldNumbersOrderAscendingRule_Apply(t *testing.T) {
279328
),
280329
},
281330
},
331+
{
332+
name: "no failures for proto message with enum inside",
333+
inputProto: &parser.Proto{
334+
ProtoBody: []parser.Visitee{
335+
&parser.Service{},
336+
&parser.Message{
337+
MessageBody: []parser.Visitee{
338+
&parser.Enum{
339+
EnumName: "MY_ENUM",
340+
EnumBody: []parser.Visitee{
341+
&parser.Service{},
342+
&parser.EnumField{
343+
Ident: "FIRST_ENUM_VALUE",
344+
Number: "0",
345+
},
346+
},
347+
Comments: nil,
348+
InlineComment: nil,
349+
InlineCommentBehindLeftCurly: nil,
350+
Meta: meta.Meta{},
351+
},
352+
&parser.Field{
353+
FieldName: "FIRST_VALUE",
354+
FieldNumber: "1",
355+
},
356+
&parser.Field{
357+
FieldName: "SECOND_VALUE",
358+
FieldNumber: "2",
359+
},
360+
},
361+
},
362+
},
363+
},
364+
wantFailures: nil,
365+
},
366+
{
367+
name: "failures for proto message with number 0",
368+
inputProto: &parser.Proto{
369+
ProtoBody: []parser.Visitee{
370+
&parser.Service{},
371+
&parser.Message{
372+
MessageBody: []parser.Visitee{
373+
&parser.Field{
374+
FieldName: "FIRST_VALUE",
375+
FieldNumber: "0",
376+
},
377+
},
378+
},
379+
},
380+
},
381+
wantFailures: []report.Failure{
382+
report.Failuref(
383+
meta.Position{},
384+
"FIELD_NUMBERS_ORDER_ASCENDING",
385+
string(rule.SeverityError),
386+
"field number should be positive integer",
387+
),
388+
},
389+
},
390+
{
391+
name: "failures for proto message with negative number",
392+
inputProto: &parser.Proto{
393+
ProtoBody: []parser.Visitee{
394+
&parser.Service{},
395+
&parser.Message{
396+
MessageBody: []parser.Visitee{
397+
&parser.Field{
398+
FieldName: "FIRST_VALUE",
399+
FieldNumber: "-1",
400+
},
401+
},
402+
},
403+
},
404+
},
405+
wantFailures: []report.Failure{
406+
report.Failuref(
407+
meta.Position{},
408+
"FIELD_NUMBERS_ORDER_ASCENDING",
409+
string(rule.SeverityError),
410+
"field number should be positive integer",
411+
),
412+
},
413+
},
282414
}
283415

284416
for _, test := range tests {

0 commit comments

Comments
 (0)