Repository: uber/prototool
Pull request #326 (2019-02-06 16:13:29)

Add linter to check that package names are only a-z0-9
I thought we already had this, but we didn't.

codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=h1) Report
> Merging [#326](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=desc) into [dev](https://codecov.io/gh/uber/prototool/commit/f5ec593f6ed78a487941665cd36a478d8c8ff0db?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `85.71%`.

[![Impacted file tree graph](https://codecov.io/gh/uber/prototool/pull/326/graphs/tree.svg?width=650&token=A9V0DApqZg&height=150&src=pr)](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev     #326      +/-   ##
==========================================
+ Coverage   73.71%   73.75%   +0.03%     
==========================================
  Files         144      145       +1     
  Lines        6342     6363      +21     
==========================================
+ Hits         4675     4693      +18     
- Misses       1243     1245       +2     
- Partials      424      425       +1
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/lint/lint.go](https://codecov.io/gh/uber/prototool/pull/326/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9saW50Lmdv) | `71.01% <ø> (ø)` | :arrow_up: |
| [internal/lint/check\_package\_lower\_case.go](https://codecov.io/gh/uber/prototool/pull/326/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19wYWNrYWdlX2xvd2VyX2Nhc2UuZ28=) | `85.71% <85.71%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=footer). Last update [f5ec593...f9c79ad](https://codecov.io/gh/uber/prototool/pull/326?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/internal/lint/BUILD.bazel b/internal/lint/BUILD.bazel
index fc166fe..c212991 100644
--- a/internal/lint/BUILD.bazel
+++ b/internal/lint/BUILD.bazel
@@ -46,6 +46,7 @@ go_library(
         "check_names.go",
         "check_oneof_names_lower_snake_case.go",
         "check_package_is_declared.go",
+        "check_package_lower_case.go",
         "check_package_lower_snake_case.go",
         "check_package_major_beta_versioned.go",
         "check_package_no_keywords.go",
diff --git a/internal/lint/check_package_lower_case.go b/internal/lint/check_package_lower_case.go
new file mode 100644
index 0000000..5a6e2e7
--- /dev/null
+++ b/internal/lint/check_package_lower_case.go
@@ -0,0 +1,75 @@
+// Copyright (c) 2019 Uber Technologies, Inc.
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in
+// all copies or substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+// THE SOFTWARE.
+
+package lint
+
+import (
+	"github.com/emicklei/proto"
+	"github.com/uber/prototool/internal/text"
+)
+
+var packageLowerCaseLinter = NewLinter(
+	"PACKAGE_LOWER_CASE",
+	"Verifies that the package name only contains characters in the range a-z0-9 and periods.",
+	checkPackageLowerCase,
+)
+
+func checkPackageLowerCase(add func(*text.Failure), dirPath string, descriptors []*FileDescriptor) error {
+	return runVisitor(&packageLowerCaseVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type packageLowerCaseVisitor struct {
+	baseAddVisitor
+
+	filename string
+	pkg      *proto.Package
+}
+
+func (v *packageLowerCaseVisitor) OnStart(descriptor *FileDescriptor) error {
+	v.filename = descriptor.Filename
+	v.pkg = nil
+	return nil
+}
+
+func (v *packageLowerCaseVisitor) VisitPackage(pkg *proto.Package) {
+	if v.pkg != nil {
+		v.AddFailuref(pkg.Position, "multiple package declarations, first was %v", v.pkg)
+		return
+	}
+	v.pkg = pkg
+}
+
+func (v *packageLowerCaseVisitor) Finally() error {
+	if v.pkg != nil {
+		if !isPackageLowerCase(v.pkg.Name) {
+			v.AddFailuref(v.pkg.Position, "Packages should only contain characters in the range a-z0-9 and periods but was %q.", v.pkg.Name)
+		}
+	}
+	return nil
+}
+
+func isPackageLowerCase(packageName string) bool {
+	for _, r := range packageName {
+		if !(('a' <= r && r <= 'z') || ('0' <= r && r <= '9') || r == '.') {
+			return false
+		}
+	}
+	return true
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index 3d66b5f..d400efd 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -100,6 +100,7 @@ var (
 		namesNoUUIDLinter,
 		oneofNamesLowerSnakeCaseLinter,
 		packageIsDeclaredLinter,
+		packageLowerCaseLinter,
 		packageLowerSnakeCaseLinter,
 		packageMajorBetaVersionedLinter,
 		packageNoKeywordsLinter,
@@ -238,7 +239,7 @@ var (
 		namesNoUUIDLinter,
 		oneofNamesLowerSnakeCaseLinter,
 		packageIsDeclaredLinter,
-		packageLowerSnakeCaseLinter,
+		packageLowerCaseLinter,
 		packageMajorBetaVersionedLinter,
 		packageNoKeywordsLinter,
 		packagesSameInDirLinter,


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index f2aa0a6..9968146 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -626,6 +626,13 @@ func TestLint(t *testing.T) {
 		"testdata/lint/nokeywords/foo/public/public_suppressed.proto",
 	)
 
+	assertDoLintFile(
+		t,
+		false,
+		`3:1:PACKAGE_LOWER_CASE`,
+		"testdata/lint/packagelowercase/foo_bar/v1/foo.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
diff --git a/internal/cmd/testdata/lint/packagelowercase/foo_bar/v1/foo.proto b/internal/cmd/testdata/lint/packagelowercase/foo_bar/v1/foo.proto
new file mode 100644
index 0000000..7ce604f
--- /dev/null
+++ b/internal/cmd/testdata/lint/packagelowercase/foo_bar/v1/foo.proto
@@ -0,0 +1,11 @@
+syntax = "proto3";
+
+package foo_bar.v1;
+
+option csharp_namespace = "Foo_bar.V1";
+option go_package = "foo_barv1";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo_bar.v1";
+option objc_class_prefix = "FXX";
+option php_namespace = "Foo_bar\\V1";
diff --git a/internal/cmd/testdata/lint/packagelowercase/prototool.yaml b/internal/cmd/testdata/lint/packagelowercase/prototool.yaml
new file mode 100644
index 0000000..7abb392
--- /dev/null
+++ b/internal/cmd/testdata/lint/packagelowercase/prototool.yaml
@@ -0,0 +1,9 @@
+lint:
+  group: uber2
+  rules:
+    remove:
+      - ENUMS_HAVE_SENTENCE_COMMENTS
+      - MESSAGES_HAVE_SENTENCE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
+      - RPCS_HAVE_SENTENCE_COMMENTS
+      - SERVICES_HAVE_SENTENCE_COMMENTS
+      - SERVICE_NAMES_MATCH_FILE_NAME