Skip to content
This repository was archived by the owner on Mar 4, 2022. It is now read-only.

Commit dd0e69c

Browse files
authored
Add linter to check for keywords in the package name (#281)
1 parent 317aad3 commit dd0e69c

File tree

7 files changed

+125
-4
lines changed

7 files changed

+125
-4
lines changed

README.md

+8
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,16 @@ to the element comment. The following lint rules can be suppressed:
203203
| Lint Rule | Containing Lint Groups | Suppressing Annotation |
204204
|-----------------------------|--------------------------|--------------------------|
205205
| `MESSAGE_FIELDS_NOT_FLOATS` | `uber2` | `floats` |
206+
| `PACKAGE_NO_KEYWORDS` | `uber2` | `keywords` |
206207

207208
As an example:
208209

209210
```
211+
// This contains the "public". keyword.
212+
//
213+
// @suppresswarnings keywords
214+
package foo.public.bar;
215+
210216
// Hello is a field where we understand the concerns with using doubles but require them.
211217
//
212218
// @suppresswarnings floats
@@ -470,6 +476,8 @@ major version, with some exceptions:
470476
reflect things such as max line lengths.
471477
- The breaking change detector may have additional checks added between minor versions, and therefore a change that might not have been
472478
breaking previously might become a breaking change.
479+
- The `PACKAGE_NO_KEYWORDS` linter on the `uber2` lint group may have additional keywords added. This can be suppressed by adding
480+
`@suppresswarnings keywords` to the package comment.
473481

474482
## Development
475483

internal/cmd/cmd_test.go

+17-4
Original file line numberDiff line numberDiff line change
@@ -392,10 +392,10 @@ func TestLint(t *testing.T) {
392392
t,
393393
false,
394394
`11:3:FIELDS_NOT_RESERVED
395-
12:3:FIELDS_NOT_RESERVED
396-
14:5:FIELDS_NOT_RESERVED
397-
15:5:FIELDS_NOT_RESERVED
398-
20:5:FIELDS_NOT_RESERVED`,
395+
12:3:FIELDS_NOT_RESERVED
396+
14:5:FIELDS_NOT_RESERVED
397+
15:5:FIELDS_NOT_RESERVED
398+
20:5:FIELDS_NOT_RESERVED`,
399399
"testdata/lint/noreserved/foo.proto",
400400
)
401401

@@ -435,6 +435,19 @@ func TestLint(t *testing.T) {
435435
"testdata/lint/floats/foo/v1/foo.proto",
436436
)
437437

438+
assertDoLintFile(
439+
t,
440+
false,
441+
`3:1:PACKAGE_NO_KEYWORDS`,
442+
"testdata/lint/nokeywords/foo/public/public.proto",
443+
)
444+
assertDoLintFile(
445+
t,
446+
false,
447+
``,
448+
"testdata/lint/nokeywords/foo/public/public_suppressed.proto",
449+
)
450+
438451
assertDoLintFile(
439452
t,
440453
false,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
syntax = "proto3";
2+
3+
package foo.public;
4+
5+
option go_package = "publicpb";
6+
option java_multiple_files = true;
7+
option java_outer_classname = "PublicProto";
8+
option java_package = "com.foo.public";
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
syntax = "proto3";
2+
3+
// @suppresswarnings keywords
4+
package foo.public;
5+
6+
option go_package = "publicpb";
7+
option java_multiple_files = true;
8+
option java_outer_classname = "PublicSuppressedProto";
9+
option java_package = "com.foo.public";
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lint:
2+
rules:
3+
add:
4+
- PACKAGE_NO_KEYWORDS
+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
// Copyright (c) 2019 Uber Technologies, Inc.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a copy
4+
// of this software and associated documentation files (the "Software"), to deal
5+
// in the Software without restriction, including without limitation the rights
6+
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
7+
// copies of the Software, and to permit persons to whom the Software is
8+
// furnished to do so, subject to the following conditions:
9+
//
10+
// The above copyright notice and this permission notice shall be included in
11+
// all copies or substantial portions of the Software.
12+
//
13+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
14+
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
15+
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
16+
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
17+
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
18+
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
19+
// THE SOFTWARE.
20+
21+
package lint
22+
23+
import (
24+
"fmt"
25+
"strings"
26+
27+
"github.com/emicklei/proto"
28+
"github.com/uber/prototool/internal/text"
29+
)
30+
31+
const packageNoKeywordsSuppressableAnnotation = "keywords"
32+
33+
var (
34+
packageNoKeywordsKeywords = []string{
35+
"internal", // Golang
36+
"public", // Java, C++, others
37+
"private", // Java, C++, others
38+
"protected", // Java, C++, others
39+
"std", // C++ (causes a problem with the std package)
40+
}
41+
42+
packageNoKeywordsLinter = NewLinter(
43+
"PACKAGE_NO_KEYWORDS",
44+
fmt.Sprintf(`Suppressable with "@suppresswarnings %s". Verifies that no packages contain one of the keywords "%s" as part of the name when split on '.'.`, packageNoKeywordsSuppressableAnnotation, strings.Join(packageNoKeywordsKeywords, ",")),
45+
checkPackageNoKeywords,
46+
)
47+
48+
packageNoKeywordsKeywordsMap = make(map[string]struct{}, len(packageNoKeywordsKeywords))
49+
)
50+
51+
func init() {
52+
for _, keyword := range packageNoKeywordsKeywords {
53+
packageNoKeywordsKeywordsMap[keyword] = struct{}{}
54+
}
55+
56+
}
57+
58+
func checkPackageNoKeywords(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
59+
return runVisitor(packageNoKeywordsVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
60+
}
61+
62+
type packageNoKeywordsVisitor struct {
63+
baseAddVisitor
64+
}
65+
66+
func (v packageNoKeywordsVisitor) VisitPackage(pkg *proto.Package) {
67+
for _, subPackage := range strings.Split(pkg.Name, ".") {
68+
potentialKeyword := strings.ToLower(subPackage)
69+
if _, ok := packageNoKeywordsKeywordsMap[potentialKeyword]; ok {
70+
if isSuppressed(pkg.Comment, packageNoKeywordsSuppressableAnnotation) {
71+
// can just return as this will be true for other keywords as well
72+
return
73+
}
74+
v.AddFailuref(pkg.Position, `Package %q contains the keyword %q, this could cause problems in generated code. This can be suppressed by adding "@suppresswarnings %s" to the package comment.`, pkg.Name, potentialKeyword, packageNoKeywordsSuppressableAnnotation)
75+
}
76+
}
77+
}

internal/lint/lint.go

+2
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ var (
7878
packageIsDeclaredLinter,
7979
packageLowerSnakeCaseLinter,
8080
packageMajorBetaVersionedLinter,
81+
packageNoKeywordsLinter,
8182
packagesSameInDirLinter,
8283
rpcsHaveCommentsLinter,
8384
rpcNamesCamelCaseLinter,
@@ -181,6 +182,7 @@ var (
181182
packageIsDeclaredLinter,
182183
packageLowerSnakeCaseLinter,
183184
packageMajorBetaVersionedLinter,
185+
packageNoKeywordsLinter,
184186
packagesSameInDirLinter,
185187
rpcNamesCamelCaseLinter,
186188
rpcNamesCapitalizedLinter,

0 commit comments

Comments
 (0)