Repository: uber/prototool
Pull request #271 (2019-01-04 21:52:21)

Add linters to outlaw public and weak imports
Re: https://github.com/uber/prototool/issues/265.

codecov-io: # [Codecov](https://codecov.io/gh/uber/prototool/pull/271?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`dev@49e2407`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

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

```diff
@@          Coverage Diff          @@
##             dev    #271   +/-   ##
=====================================
  Coverage       ?   71.1%           
=====================================
  Files          ?      98           
  Lines          ?    4797           
  Branches       ?       0           
=====================================
  Hits           ?    3411           
  Misses         ?    1028           
  Partials       ?     358
```


| [Impacted Files](https://codecov.io/gh/uber/prototool/pull/271?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [internal/lint/lint.go](https://codecov.io/gh/uber/prototool/pull/271/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9saW50Lmdv) | `78.49% <ø> (ø)` | |
| [internal/lint/check\_imports\_not\_public.go](https://codecov.io/gh/uber/prototool/pull/271/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19pbXBvcnRzX25vdF9wdWJsaWMuZ28=) | `100% <100%> (ø)` | |
| [internal/lint/check\_imports\_not\_weak.go](https://codecov.io/gh/uber/prototool/pull/271/diff?src=pr&el=tree#diff-aW50ZXJuYWwvbGludC9jaGVja19pbXBvcnRzX25vdF93ZWFrLmdv) | `100% <100%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/uber/prototool/pull/271?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/271?src=pr&el=footer). Last update [49e2407...b1580f1](https://codecov.io/gh/uber/prototool/pull/271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/.travis.yml b/.travis.yml
index daec7ff..849123e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -5,10 +5,6 @@ branches:
 language: go
 go:
   - "1.11.x"
-cache:
-  directories:
-    - $GOPATH/pkg/mod
 script:
-  - make checknodiffgenerated
-  - make -j 8 golint vet errcheck staticcheck checklicense cover
+  - make all
   - make codecov
diff --git a/Makefile b/Makefile
index 760116a..5024bb4 100644
--- a/Makefile
+++ b/Makefile
@@ -14,6 +14,7 @@ TMP_BIN = $(TMP)/bin
 TMP_ETC := $(TMP)/etc
 TMP_LIB := $(TMP)/lib
 
+unexport GOPATH
 export GO111MODULE := on
 export GOBIN := $(abspath $(TMP_BIN))
 export PATH := $(GOBIN):$(PATH)
diff --git a/internal/lint/check_imports_not_public.go b/internal/lint/check_imports_not_public.go
new file mode 100644
index 0000000..fc123c6
--- /dev/null
+++ b/internal/lint/check_imports_not_public.go
@@ -0,0 +1,48 @@
+// 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 (
+	importsNotPublicLinter = NewLinter(
+		"IMPORTS_NOT_PUBLIC",
+		`Verifies that there are no public imports.`,
+		checkImportsNotPublic,
+	)
+)
+
+func checkImportsNotPublic(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(&importsNotPublicVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type importsNotPublicVisitor struct {
+	baseAddVisitor
+}
+
+func (v importsNotPublicVisitor) VisitImport(element *proto.Import) {
+	if element.Kind == "public" {
+		v.AddFailuref(element.Position, `Public imports are not allowed.`)
+	}
+}
diff --git a/internal/lint/check_imports_not_weak.go b/internal/lint/check_imports_not_weak.go
new file mode 100644
index 0000000..4783496
--- /dev/null
+++ b/internal/lint/check_imports_not_weak.go
@@ -0,0 +1,48 @@
+// 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 (
+	importsNotWeakLinter = NewLinter(
+		"IMPORTS_NOT_WEAK",
+		`Verifies that there are no weak imports.`,
+		checkImportsNotWeak,
+	)
+)
+
+func checkImportsNotWeak(add func(*text.Failure), dirPath string, descriptors []*proto.Proto) error {
+	return runVisitor(&importsNotWeakVisitor{baseAddVisitor: newBaseAddVisitor(add)}, descriptors)
+}
+
+type importsNotWeakVisitor struct {
+	baseAddVisitor
+}
+
+func (v importsNotWeakVisitor) VisitImport(element *proto.Import) {
+	if element.Kind == "weak" {
+		v.AddFailuref(element.Position, `Weak imports are not allowed.`)
+	}
+}
diff --git a/internal/lint/lint.go b/internal/lint/lint.go
index 2149e8f..7668b96 100644
--- a/internal/lint/lint.go
+++ b/internal/lint/lint.go
@@ -63,6 +63,8 @@ var (
 		fileOptionsUnsetJavaMultipleFilesLinter,
 		fileOptionsUnsetJavaOuterClassnameLinter,
 		gogoNotImportedLinter,
+		importsNotPublicLinter,
+		importsNotWeakLinter,
 		messageFieldsNotFloatsLinter,
 		messageFieldsNoJSONNameLinter,
 		messageFieldNamesLowerSnakeCaseLinter,
diff --git a/internal/reflect/proto/prototool.yaml b/internal/reflect/proto/prototool.yaml
index 248cbd6..21032fc 100644
--- a/internal/reflect/proto/prototool.yaml
+++ b/internal/reflect/proto/prototool.yaml
@@ -6,8 +6,11 @@ lint:
       - ENUM_FIELD_PREFIXES_EXCEPT_MESSAGE
       - ENUM_ZERO_VALUES_INVALID_EXCEPT_MESSAGE
       - ENUMS_HAVE_COMMENTS
+      - FIELDS_NOT_RESERVED
       - FILE_OPTIONS_EQUAL_GO_PACKAGE_LAST_TWO_SUFFIX
       - GOGO_NOT_IMPORTED
+      - IMPORTS_NOT_PUBLIC
+      - IMPORTS_NOT_WEAK
       - MESSAGE_FIELDS_NO_JSON_NAME
       - MESSAGES_HAVE_COMMENTS_EXCEPT_REQUEST_RESPONSE_TYPES
       - PACKAGE_MAJOR_VERSIONED


Test patch:
diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go
index f1f3c2b..ce96574 100644
--- a/internal/cmd/cmd_test.go
+++ b/internal/cmd/cmd_test.go
@@ -406,6 +406,20 @@ func TestLint(t *testing.T) {
 		"testdata/lint/gogonotimported/gogonotimported.proto",
 	)
 
+	assertDoLintFile(
+		t,
+		false,
+		`10:1:IMPORTS_NOT_PUBLIC`,
+		"testdata/lint/importsnotpublic/foo.proto",
+	)
+
+	assertDoLintFile(
+		t,
+		false,
+		`10:1:IMPORTS_NOT_WEAK`,
+		"testdata/lint/importsnotweak/foo.proto",
+	)
+
 	assertDoLintFile(
 		t,
 		false,
diff --git a/internal/cmd/testdata/lint/importsnotpublic/bar/bar.proto b/internal/cmd/testdata/lint/importsnotpublic/bar/bar.proto
new file mode 100644
index 0000000..baba400
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotpublic/bar/bar.proto
@@ -0,0 +1,12 @@
+syntax = "proto3";
+
+package bar;
+
+option go_package = "barpb";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar";
+
+message Bar {
+  int64 hello = 1;
+}
diff --git a/internal/cmd/testdata/lint/importsnotpublic/foo.proto b/internal/cmd/testdata/lint/importsnotpublic/foo.proto
new file mode 100644
index 0000000..0cd3d05
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotpublic/foo.proto
@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+package foo;
+
+option go_package = "foopb";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo";
+
+import public "bar/bar.proto";
+
+message Foo {
+  int64 hello = 1;
+  bar.Bar bar = 2;
+}
diff --git a/internal/cmd/testdata/lint/importsnotpublic/prototool.yaml b/internal/cmd/testdata/lint/importsnotpublic/prototool.yaml
new file mode 100644
index 0000000..99f1ef5
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotpublic/prototool.yaml
@@ -0,0 +1,4 @@
+lint:
+  rules:
+    add:
+      - IMPORTS_NOT_PUBLIC
diff --git a/internal/cmd/testdata/lint/importsnotweak/bar/bar.proto b/internal/cmd/testdata/lint/importsnotweak/bar/bar.proto
new file mode 100644
index 0000000..baba400
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotweak/bar/bar.proto
@@ -0,0 +1,12 @@
+syntax = "proto3";
+
+package bar;
+
+option go_package = "barpb";
+option java_multiple_files = true;
+option java_outer_classname = "BarProto";
+option java_package = "com.bar";
+
+message Bar {
+  int64 hello = 1;
+}
diff --git a/internal/cmd/testdata/lint/importsnotweak/foo.proto b/internal/cmd/testdata/lint/importsnotweak/foo.proto
new file mode 100644
index 0000000..e86f87e
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotweak/foo.proto
@@ -0,0 +1,15 @@
+syntax = "proto3";
+
+package foo;
+
+option go_package = "foopb";
+option java_multiple_files = true;
+option java_outer_classname = "FooProto";
+option java_package = "com.foo";
+
+import weak "bar/bar.proto";
+
+message Foo {
+  int64 hello = 1;
+  bar.Bar bar = 2;
+}
diff --git a/internal/cmd/testdata/lint/importsnotweak/prototool.yaml b/internal/cmd/testdata/lint/importsnotweak/prototool.yaml
new file mode 100644
index 0000000..e5558d5
--- /dev/null
+++ b/internal/cmd/testdata/lint/importsnotweak/prototool.yaml
@@ -0,0 +1,4 @@
+lint:
+  rules:
+    add:
+      - IMPORTS_NOT_WEAK