Skip to content

Commit 2d871c3

Browse files
committed
Add rule to ensure inline code is surrounded by backticks
1 parent ebf2763 commit 2d871c3

File tree

4 files changed

+345
-0
lines changed

4 files changed

+345
-0
lines changed

docs/rules/0192/closed-backticks.md

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
---
2+
rule:
3+
aip: 192
4+
name: [core, '0192', closed-backticks]
5+
summary: Inline code should be surrounded by backticks.
6+
permalink: /192/closed-backticks
7+
redirect_from:
8+
- /0192/closed-backticks
9+
---
10+
11+
# Closed backticks
12+
13+
This rule enforces that any inline code in public comments for proto descriptors
14+
is surrounded by backticks, as mandated in [AIP-192][].
15+
16+
## Details
17+
18+
This rule looks at each descriptor in each proto file (exempting oneofs and the
19+
file itself) and complains if _public_ comments include text that only has
20+
either an opening backtick or a closing backtick.
21+
22+
## Examples
23+
24+
**Incorrect** code for this rule:
25+
26+
```proto
27+
// Incorrect.
28+
// Fields on the book include:
29+
//
30+
// - name`: `string
31+
message Book {
32+
// The resource name of the book.
33+
string name = 1;
34+
}
35+
```
36+
37+
**Correct** code for this rule:
38+
39+
```proto
40+
// Correct.
41+
// Fields on the book include:
42+
//
43+
// - `name`: `string`
44+
message Book {
45+
// The resource name of the book.
46+
string name = 1;
47+
}
48+
```
49+
50+
## Disabling
51+
52+
If you need to violate this rule, use a leading comment above the descriptor.
53+
Remember to also include an [aip.dev/not-precedent][] comment explaining why.
54+
55+
```proto
56+
// Fields on the book include:
57+
//
58+
// - name`: `string
59+
// (-- api-linter: core::0192::closed-backticks=disabled
60+
// aip.dev/not-precedent: We need to do this because reasons. --)
61+
message Book {
62+
// The resource name of the book.
63+
string name = 1;
64+
}
65+
```
66+
67+
[aip-192]: https://aip.dev/192
68+
[aip.dev/not-precedent]: https://aip.dev/not-precedent

rules/aip0192/aip0192.go

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ func AddRules(r lint.RuleRegistry) error {
2525
return r.Register(
2626
192,
2727
absoluteLinks,
28+
closedBackticks,
2829
deprecatedComment,
2930
hasComments,
3031
noHTML,

rules/aip0192/closed_backticks.go

+186
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
// Copyright 2023 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 aip0192
16+
17+
import (
18+
"regexp"
19+
"strings"
20+
21+
"bitbucket.org/creachadair/stringset"
22+
"github.com/googleapis/api-linter/lint"
23+
"github.com/googleapis/api-linter/rules/internal/utils"
24+
"github.com/jhump/protoreflect/desc"
25+
)
26+
27+
type backtickPair int
28+
29+
const (
30+
unspecified backtickPair = iota
31+
missingOpening
32+
missingClosing
33+
paired
34+
)
35+
36+
// Represents a backtick character in a line.
37+
//
38+
// To narrow which backticks to lint and the likely intended use of a stray
39+
// backtick, we compute whether the backtick can be used as an opening and/or
40+
// closing backtick.
41+
//
42+
// If the character before a backtick is one of the `separators`, the backtick
43+
// is considered opening. Likewise, if the character after a backtick is one of
44+
// the `separators`, the backtick is considered closing.
45+
type backtick struct {
46+
index int
47+
opening bool
48+
closing bool
49+
pair backtickPair
50+
}
51+
52+
var separators = stringset.New(" ", ":", ",", ".", ";", "-")
53+
var reBacktick = regexp.MustCompile("`")
54+
55+
var closedBackticks = &lint.DescriptorRule{
56+
Name: lint.NewRuleName(192, "closed-backticks"),
57+
LintDescriptor: func(d desc.Descriptor) []lint.Problem {
58+
problems := []lint.Problem{}
59+
60+
for _, comment := range utils.SeparateInternalComments(d.GetSourceInfo().GetLeadingComments()).External {
61+
for _, line := range strings.Split(comment, "\n") {
62+
// Pad the line with whitespace, so it's easier to lint
63+
// backticks at the start and end of the line.
64+
line = " " + line + " "
65+
66+
// Find all the backticks in the line and compute whether each
67+
// one can be used as an opening and/or closing backtick.
68+
backticks := []backtick{}
69+
for _, loc := range reBacktick.FindAllStringIndex(line, -1) {
70+
backticks = append(backticks, backtickAtIndex(line, loc[0]))
71+
}
72+
73+
// Compute whether the backticks are paired.
74+
backticks = computeBacktickPairs(filterUnusableBackticks(backticks))
75+
76+
// Add a problem for each backtick that is missing a pair.
77+
for _, backtick := range backticks {
78+
if backtick.pair != paired {
79+
problems = append(problems, lint.Problem{
80+
Message: "Inline code should be surrounded by backticks.",
81+
Suggestion: "`" + suggestionWord(line, backtick) + "`",
82+
Descriptor: d,
83+
})
84+
}
85+
}
86+
}
87+
}
88+
return problems
89+
},
90+
}
91+
92+
func backtickAtIndex(line string, index int) backtick {
93+
return backtick{
94+
index: index,
95+
opening: separators.Contains(string(line[index-1])),
96+
closing: separators.Contains(string(line[index+1])),
97+
}
98+
}
99+
100+
// Filter out backticks that cannot be used as opening or closing backticks.
101+
func filterUnusableBackticks(backticks []backtick) []backtick {
102+
list := []backtick{}
103+
for _, backtick := range backticks {
104+
if backtick.opening || backtick.closing {
105+
list = append(list, backtick)
106+
}
107+
}
108+
return list
109+
}
110+
111+
// Compute whether each backtick is paired with an adjacent backtick.
112+
//
113+
// A backtick pair consists of a set of adjacent backticks, where the first is
114+
// opening and the second is closing. Each backtick can be in at most one pair.
115+
//
116+
// This fills in the `pair` field for each backtick. It assumes every backtick
117+
// is either opening, closing, or both.
118+
func computeBacktickPairs(backticks []backtick) []backtick {
119+
computed := []backtick{}
120+
121+
prevBacktickOpen := false
122+
for i, backtick := range backticks {
123+
backtick.pair = computePairState(backtick, prevBacktickOpen)
124+
125+
// If this backtick got paired, mark the previous one as paired as well.
126+
if backtick.pair == paired {
127+
computed[i-1].pair = paired
128+
}
129+
130+
// If paired, this backtick cannot be used as an `opening` for another
131+
// backtick.
132+
prevBacktickOpen = false
133+
if backtick.opening && backtick.pair != paired {
134+
prevBacktickOpen = true
135+
}
136+
137+
computed = append(computed, backtick)
138+
}
139+
return computed
140+
}
141+
142+
func computePairState(backtick backtick, prevBacktickOpen bool) backtickPair {
143+
// If the backtick is only opening, it needs a closing backtick.
144+
if backtick.opening && !backtick.closing {
145+
return missingClosing
146+
}
147+
// Otherwise, the backtick is closing. It is paired if the last backtick was
148+
// opening.
149+
if prevBacktickOpen {
150+
return paired
151+
}
152+
return missingOpening
153+
}
154+
155+
// Extract the word around an unpaired backtick.
156+
//
157+
// Even though inline code can include separators (e.g. colons, spaces), just
158+
// suggest the string of characters up to the first separator character.
159+
func suggestionWord(line string, backtick backtick) string {
160+
switch backtick.pair {
161+
case missingOpening:
162+
return reverseString(firstWordBeforeSeparator(reverseString(line[:backtick.index])))
163+
case missingClosing:
164+
return firstWordBeforeSeparator(line[backtick.index+1:])
165+
default:
166+
return ""
167+
}
168+
}
169+
170+
func firstWordBeforeSeparator(s string) string {
171+
index := strings.IndexFunc(s, func(r rune) bool {
172+
return separators.Contains(string(r))
173+
})
174+
if index != -1 {
175+
return s[:index]
176+
}
177+
return s
178+
}
179+
180+
func reverseString(s string) string {
181+
runes := []rune(s)
182+
for i, j := 0, len(runes)-1; i < j; i, j = i+1, j-1 {
183+
runes[i], runes[j] = runes[j], runes[i]
184+
}
185+
return string(runes)
186+
}
+90
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// Copyright 2023 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 aip0192
16+
17+
import (
18+
"testing"
19+
20+
"github.com/googleapis/api-linter/rules/internal/testutils"
21+
)
22+
23+
func TestClosedBackticks(t *testing.T) {
24+
for _, test := range []struct {
25+
name string
26+
Comment string
27+
problems testutils.Problems
28+
}{
29+
{"ValidWordChars", "`foo_bar_baz_1`", nil},
30+
{"MissingFrontBacktickWordChars", "foo_bar_baz_1`", testutils.Problems{{Suggestion: "`foo_bar_baz_1`"}}},
31+
{"MissingBackBacktickWordChars", "`foo_bar_baz_1", testutils.Problems{{Suggestion: "`foo_bar_baz_1`"}}},
32+
33+
{"ValidBetweenWords", "foo `bar` baz", nil},
34+
{"MissingFrontBacktickBetweenWords", "foo bar` baz", testutils.Problems{{Suggestion: "`bar`"}}},
35+
{"MissingBackBacktickBetweenWords", "foo `bar baz", testutils.Problems{{Suggestion: "`bar`"}}},
36+
37+
{"ValidNonWordChars", "`foo:bar/baz->qux.quux[0]()`", nil},
38+
{"MissingFrontBacktickNonWordChars", "foo:bar/baz->qux.quux[0]()`", testutils.Problems{{Suggestion: "`quux[0]()`"}}},
39+
{"MissingBackBacktickNonWordChars", "`foo:bar/baz->qux.quux[0]()", testutils.Problems{{Suggestion: "`foo`"}}},
40+
41+
{"ValidContainsSpace", "`foo + bar`", nil},
42+
{"MissingFrontBacktickContainsSpace", "foo + bar`", testutils.Problems{{Suggestion: "`bar`"}}},
43+
{"MissingBackBacktickContainsSpace", "`foo + bar", testutils.Problems{{Suggestion: "`foo`"}}},
44+
45+
{"ValidNonAscii", "`汉语`", nil},
46+
{"MissingFrontBacktickNonAscii", "汉语`", testutils.Problems{{Suggestion: "`汉语`"}}},
47+
{"MissingBackBacktickNonAscii", "`汉语", testutils.Problems{{Suggestion: "`汉语`"}}},
48+
49+
{"ValidQuotes", "`\"foo\"`", nil},
50+
{"MissingFrontBacktickQuotes", "\"foo\"`", testutils.Problems{{Suggestion: "`\"foo\"`"}}},
51+
{"MissingBackBacktickQuotes", "`\"foo\"", testutils.Problems{{Suggestion: "`\"foo\"`"}}},
52+
53+
{"ValidSeparatedByColons", "foo:`bar:baz`:qux", nil},
54+
{"MissingFrontBacktickSeparatedByColons", "foo:`bar:baz:qux", testutils.Problems{{Suggestion: "`bar`"}}},
55+
{"MissingBackBacktickSeparatedByColons", "foo:bar:baz`:qux", testutils.Problems{{Suggestion: "`baz`"}}},
56+
57+
{"ValidComma", "`name`, a string", nil},
58+
{"MissingFrontBacktickComma", "name`, a string", testutils.Problems{{Suggestion: "`name`"}}},
59+
{"MissingBackBacktickComma", "`name, a string", testutils.Problems{{Suggestion: "`name`"}}},
60+
61+
{"ValidMultipleCodeText", "`name`: `string`", nil},
62+
{"MissingFrontBacktickMultipleCodeText", "name`: string`", testutils.Problems{{Suggestion: "`name`"}, {Suggestion: "`string`"}}},
63+
{"MissingBackBacktickMultipleCodeText", "`name: `string", testutils.Problems{{Suggestion: "`name`"}, {Suggestion: "`string`"}}},
64+
{"MissingFrontAndBackBacktickMultipleCodeText", "name`: `string", testutils.Problems{{Suggestion: "`name`"}, {Suggestion: "`string`"}}},
65+
66+
{"ValidContainsColon", "`name: string`", nil},
67+
68+
{"ValidContainsSeparator", "`.`", nil},
69+
{"MissingFrontBacktickContainsSeparator", ".`", testutils.Problems{{Suggestion: "``"}}},
70+
{"MissingBackBacktickContainsSeparator", "`.", testutils.Problems{{Suggestion: "``"}}},
71+
72+
{"ValidEmptySpace", "``", nil},
73+
{"ValidEmptySpaces", "`` ``", nil},
74+
{"UnpairedEmptySpace", "`` `", testutils.Problems{{Suggestion: "``"}}},
75+
76+
{"IgnoreTripleBacktick", "```", nil},
77+
{"IgnoreBackticksWithinWords", "a`b`c`d", nil},
78+
} {
79+
t.Run(test.name, func(t *testing.T) {
80+
f := testutils.ParseProto3Tmpl(t, `
81+
// {{.Comment}}
82+
message Foo {}
83+
`, test)
84+
m := f.GetMessageTypes()[0]
85+
if diff := test.problems.SetDescriptor(m).Diff(closedBackticks.Lint(f)); diff != "" {
86+
t.Errorf(diff)
87+
}
88+
})
89+
}
90+
}

0 commit comments

Comments
 (0)