Skip to content

Commit 5fab299

Browse files
quirogasnoahdietz
quirogas
and
noahdietz
authored
feat: add relative time segments comment rule
* chore(aip0142-test): Add tests for durration offset comment * chore(aip0142): Add rule for durration offset comment * docs(aip0142): Add documentation for durration offset comment * chore(aip0142): improve triggering logic * Update rules/aip0142/duration_offset_comment_test.go Co-authored-by: Noah Dietz <[email protected]> * Update rules/aip0142/duration_offset_comment.go Co-authored-by: Noah Dietz <[email protected]> --------- Co-authored-by: Noah Dietz <[email protected]>
1 parent bb82f00 commit 5fab299

File tree

4 files changed

+206
-0
lines changed

4 files changed

+206
-0
lines changed

‎docs/rules/0142/duration-offset-comment.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
---
2+
rule:
3+
aip: 142
4+
name: [core, '0142', duration-offset-comment]
5+
summary: Duration fields ending in `_offset` must have a clarifying comment.
6+
permalink: /142/duration-offset-comment
7+
redirect_from:
8+
- /0142/duration-offset-comment
9+
---
10+
11+
# Duration offset comment
12+
13+
This rule enforces that `google.protobuf.Duration` fields ending in `_offset`
14+
have a comment explaining what the offset is relative to, as mandated in
15+
[AIP-142][].
16+
17+
## Details
18+
19+
This rule looks at each `google.protobuf.Duration` field that have a name
20+
with an `_offset` suffix, and ensures that is has a leading
21+
comment.
22+
23+
## Examples
24+
25+
**Incorrect** code for this rule:
26+
27+
```proto
28+
// Incorrect: `start_offset` is a Duration with the `_offset` suffix
29+
// but is missing the required comment.
30+
message AudioSegment {
31+
google.protobuf.Duration start_offset = 1;
32+
33+
// The total length of the segment.
34+
google.protobuf.Duration segment_duration = 2;
35+
}
36+
```
37+
38+
**Correct** code for this rule:
39+
40+
```proto
41+
// Correct: `start_offset` is a Duration with the `_offset` suffix
42+
// and has a clarifying comment.
43+
message AudioSegment {
44+
// The duration relative to the start of the stream representing the
45+
// beginning of the segment.
46+
google.protobuf.Duration start_offset = 1;
47+
48+
// The total length of the segment.
49+
google.protobuf.Duration segment_duration = 2;
50+
}
51+
```
52+
53+
## Disabling
54+
55+
If you need to violate this rule, use a leading comment above the field.
56+
Remember to also include an [aip.dev/not-precedent][] comment explaining why.
57+
58+
```proto
59+
// (-- api-linter: core::0142::duration-offset-comment=disabled
60+
// aip.dev/not-precedent: We need to do this because reasons. --)
61+
message AudioSegment {
62+
google.protobuf.Duration start_offset = 1;
63+
}
64+
```
65+
66+
If you need to violate this rule for an entire file, place the comment at the
67+
top of the file.
68+
69+
[aip-142]: https://aip.dev/142
70+
[aip.dev/not-precedent]: https://aip.dev/not-precedent

‎rules/aip0142/aip0142.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ func AddRules(r lint.RuleRegistry) error {
2727
142,
2828
fieldNames,
2929
fieldType,
30+
durationOffsetComment,
3031
)
3132
}
3233

‎rules/aip0142/duration_offset_comment.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package aip0142
16+
17+
import (
18+
"strings"
19+
20+
"github.com/googleapis/api-linter/lint"
21+
"github.com/googleapis/api-linter/rules/internal/utils"
22+
"github.com/jhump/protoreflect/desc"
23+
)
24+
25+
var durationOffsetComment = &lint.FieldRule{
26+
Name: lint.NewRuleName(142, "duration-offset-comment"),
27+
OnlyIf: func(f *desc.FieldDescriptor) bool {
28+
return utils.GetTypeName(f) == "google.protobuf.Duration" && strings.HasSuffix(f.GetName(), "_offset")
29+
},
30+
LintField: func(f *desc.FieldDescriptor) []lint.Problem {
31+
comment := strings.ToLower(f.GetSourceInfo().GetLeadingComments())
32+
if comment == "" || (!strings.Contains(comment, "relative") && !strings.Contains(comment, "in respect")) {
33+
return []lint.Problem{{
34+
Message: "Duration fields ending in `_offset` must include a clear comment explaining the relative start point.",
35+
Descriptor: f,
36+
}}
37+
}
38+
return nil
39+
},
40+
}

‎rules/aip0142/duration_offset_comment_test.go

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package aip0142
16+
17+
import (
18+
"testing"
19+
20+
"github.com/googleapis/api-linter/rules/internal/testutils"
21+
)
22+
23+
func TestDurationOffsetComment(t *testing.T) {
24+
for _, test := range []struct {
25+
name string
26+
Comment string
27+
FieldName string
28+
FieldType string
29+
problems testutils.Problems
30+
}{
31+
{
32+
name: "ValidWithComment",
33+
Comment: "// The duration relative to the start of the stream.",
34+
FieldName: "start_offset",
35+
FieldType: "google.protobuf.Duration",
36+
problems: nil,
37+
},
38+
{
39+
name: "InvalidNoComment",
40+
Comment: "",
41+
FieldName: "start_offset",
42+
FieldType: "google.protobuf.Duration",
43+
problems: testutils.Problems{{Message: "must include a clear comment explaining the relative start point."}},
44+
},
45+
{
46+
name: "ValidWithRespectComment",
47+
Comment: "// The duration in respect to the start.",
48+
FieldName: "end_offset",
49+
FieldType: "google.protobuf.Duration",
50+
problems: nil,
51+
},
52+
{
53+
name: "InvallidOfTheComment",
54+
Comment: "// The duration of the event offset from start.",
55+
FieldName: "event_offset",
56+
FieldType: "google.protobuf.Duration",
57+
problems: testutils.Problems{{Message: "must include a clear comment explaining the relative start point."}},
58+
},
59+
{
60+
name: "InvalidInadequateComment",
61+
Comment: "// This is just a comment.",
62+
FieldName: "another_offset",
63+
FieldType: "google.protobuf.Duration",
64+
problems: testutils.Problems{{Message: "must include a clear comment explaining the relative start point."}},
65+
},
66+
{
67+
name: "SkipNoOffsetSuffix",
68+
Comment: "",
69+
FieldName: "start",
70+
FieldType: "google.protobuf.Duration",
71+
problems: nil,
72+
},
73+
{
74+
name: "SkipNotDuration",
75+
Comment: "",
76+
FieldName: "map_offset",
77+
FieldType: "string",
78+
problems: nil,
79+
},
80+
} {
81+
t.Run(test.name, func(t *testing.T) {
82+
f := testutils.ParseProto3Tmpl(t, `
83+
import "google/protobuf/duration.proto";
84+
message AudioSegment {
85+
{{.Comment}}
86+
{{.FieldType}} {{.FieldName}} = 1;
87+
}
88+
`, test)
89+
field := f.GetMessageTypes()[0].GetFields()[0]
90+
if diff := test.problems.SetDescriptor(field).Diff(durationOffsetComment.Lint(f)); diff != "" {
91+
t.Error(diff)
92+
}
93+
})
94+
}
95+
}

0 commit comments

Comments
 (0)