From 182b72d6bc573329198af2b66d0ef7679cdbc61f Mon Sep 17 00:00:00 2001 From: Tricia Decker <1440268+pdecks@users.noreply.github.com> Date: Thu, 15 Dec 2022 16:24:06 -0800 Subject: [PATCH] Bump Go to 1.17, use golangci-lint instead of make lint (#112) --- .github/workflows/pre-commit.yml | 17 ++++++++++++ .github/workflows/test.yml | 7 ++--- .golangci.yml | 26 +++++++++++++++++++ .pre-commit-config.yaml | 7 +++++ Makefile | 14 ++-------- artifact_test.go | 14 +++++----- field_test.go | 3 +-- field_type_test.go | 5 ++-- lang/go/name.go | 8 +++--- lang/go/package_test.go | 8 ------ lang/go/testdata/outputs/import_prefix/params | 1 - .../outputs/import_prefix/prefix.proto | 4 --- .../outputs/import_prefix_srcrel/params | 2 -- .../outputs/import_prefix_srcrel/prefix.proto | 4 --- .../import_prefix/import_prefix.proto | 13 ---------- .../go/testdata/packages/import_prefix/params | 1 - lang/go/type_name_p2_presence_test.go | 1 + method.go | 2 +- 18 files changed, 73 insertions(+), 64 deletions(-) create mode 100644 .github/workflows/pre-commit.yml create mode 100644 .golangci.yml create mode 100644 .pre-commit-config.yaml delete mode 100644 lang/go/testdata/outputs/import_prefix/params delete mode 100644 lang/go/testdata/outputs/import_prefix/prefix.proto delete mode 100644 lang/go/testdata/outputs/import_prefix_srcrel/params delete mode 100644 lang/go/testdata/outputs/import_prefix_srcrel/prefix.proto delete mode 100644 lang/go/testdata/packages/import_prefix/import_prefix.proto delete mode 100644 lang/go/testdata/packages/import_prefix/params diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml new file mode 100644 index 0000000..6dec0d1 --- /dev/null +++ b/.github/workflows/pre-commit.yml @@ -0,0 +1,17 @@ +name: pre-commit +on: + push: + branches: + - master + pull_request: + branches: + - master +jobs: + pre-commit: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 + with: + go-version: '1.17' + - uses: pre-commit/action@v3.0.0 diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b16df07..94a0a72 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -26,7 +26,7 @@ jobs: - name: Set Up Go uses: actions/setup-go@v2 with: - go-version: '1.14.6' + go-version: '1.17' - run: mkdir -p $GOPATH/bin - run: wget "https://github.com/protocolbuffers/protobuf/releases/download/v${{ matrix.protoc-version }}/protoc-${{ matrix.protoc-version }}-linux-x86_64.zip" -O /tmp/protoc.zip - run: unzip /tmp/protoc.zip -d /tmp @@ -34,5 +34,6 @@ jobs: - run: sudo mv /tmp/include/google /usr/local/include/google - name: Generate Testdata run: make testdata - - name: Lint - run: make lint tests + - name: Run Tests + run: make tests + diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..dddcca4 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,26 @@ +linters: + disable-all: true + enable: + - deadcode + - goconst + - gocyclo + - gofmt + - goimports + - gosimple + - govet + - ineffassign + - misspell + - revive + - structcheck + - typecheck + - unconvert + - unparam + - unused + - varcheck +issues: + max-per-linter: 0 + max-same-issues: 0 +run: + build-tags: + - integration + deadline: 5m diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..77c83de --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,7 @@ +default_language_version: + python: python3.8 +repos: + - repo: https://github.com/golangci/golangci-lint + rev: v1.42.1 + hooks: + - id: golangci-lint diff --git a/Makefile b/Makefile index c679944..54654c4 100644 --- a/Makefile +++ b/Makefile @@ -5,16 +5,6 @@ PROTOC_VER := $(shell protoc --version | cut -d' ' -f2) .PHONY: bootstrap bootstrap: testdata # set up the project for development -.PHONY: lint -lint: # lints the package for common code smells - set -e; for f in `find . -name "*.go" -not -name "*.pb.go"`; do \ - out=`gofmt -s -d $$f`; \ - test -z "$$out" || (echo $$out && exit 1); \ - done - which golint || go get -u golang.org/x/lint/golint - golint -set_exit_status ./... - go vet -all - .PHONY: quick quick: testdata # runs all tests without the race detector or coverage ifeq ($(PROTOC_VER), 3.17.0) @@ -59,7 +49,7 @@ testdata-graph: bin/protoc-gen-debug # parses the proto file sets in testdata/gr done testdata/generated: protoc-gen-go bin/protoc-gen-example - which protoc-gen-go || (go install github.com/golang/protobuf/protoc-gen-go) + go install google.golang.org/protobuf/cmd/protoc-gen-go rm -rf ./testdata/generated && mkdir -p ./testdata/generated # generate the official go code, must be one directory at a time set -e; for subdir in `find ./testdata/protos -mindepth 1 -type d`; do \ @@ -100,7 +90,7 @@ vendor: # install project dependencies .PHONY: protoc-gen-go protoc-gen-go: - which protoc-gen-go || (go install github.com/golang/protobuf/protoc-gen-go) + go install google.golang.org/protobuf/cmd/protoc-gen-go bin/protoc-gen-example: # creates the demo protoc plugin for demonstrating uses of PG* go build -o ./bin/protoc-gen-example ./testdata/protoc-gen-example diff --git a/artifact_test.go b/artifact_test.go index 5e87638..ce5338c 100644 --- a/artifact_test.go +++ b/artifact_test.go @@ -13,6 +13,8 @@ var ( artifactTpl = template.Must(template.New("foo").Parse("{{ . }}")) ) +const fName = "foo" + func TestGeneratorFile_ProtoFile(t *testing.T) { t.Parallel() @@ -25,7 +27,7 @@ func TestGeneratorFile_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.Name = "foo" + f.Name = fName pb, err = f.ProtoFile() assert.NoError(t, err) assert.Equal(t, f.Name, pb.GetName()) @@ -47,7 +49,7 @@ func TestGeneratorTemplateFile_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.Name = "foo" + f.Name = fName pb, err = f.ProtoFile() assert.Error(t, err) assert.Nil(t, pb) @@ -71,7 +73,7 @@ func TestGeneratorAppend_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.FileName = "foo" + f.FileName = fName pb, err = f.ProtoFile() assert.NoError(t, err) assert.Empty(t, pb.GetName()) @@ -93,7 +95,7 @@ func TestGeneratorTemplateAppend_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.FileName = "foo" + f.FileName = fName pb, err = f.ProtoFile() assert.Error(t, err) assert.Nil(t, pb) @@ -118,7 +120,7 @@ func TestGeneratorInjection_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.FileName = "foo" + f.FileName = fName pb, err = f.ProtoFile() assert.NoError(t, err) assert.Equal(t, f.FileName, pb.GetName()) @@ -142,7 +144,7 @@ func TestGeneratorTemplateInjection_ProtoFile(t *testing.T) { assert.Error(t, err) assert.Nil(t, pb) - f.FileName = "foo" + f.FileName = fName pb, err = f.ProtoFile() assert.Error(t, err) assert.Nil(t, pb) diff --git a/field_test.go b/field_test.go index a243cbc..aae8612 100644 --- a/field_test.go +++ b/field_test.go @@ -251,8 +251,7 @@ func dummyOneOfField(synthetic bool) *field { m := dummyMsg() o := dummyOneof() str := descriptor.FieldDescriptorProto_TYPE_STRING - var oIndex int32 - oIndex = 1 + var oIndex int32 = 1 f := &field{desc: &descriptor.FieldDescriptorProto{ Name: proto.String("field"), Type: &str, diff --git a/field_type_test.go b/field_type_test.go index 66a043d..dcb78a8 100644 --- a/field_type_test.go +++ b/field_type_test.go @@ -304,9 +304,8 @@ func TestMapT_Key(t *testing.T) { type mockT struct { FieldType - i []File - f Field - err error + i []File + f Field } func (t *mockT) Imports() []File { return t.i } diff --git a/lang/go/name.go b/lang/go/name.go index d31a965..8daef74 100644 --- a/lang/go/name.go +++ b/lang/go/name.go @@ -22,7 +22,7 @@ func (c context) Name(node pgs.Node) pgs.Name { return c.PackageName(en) case ChildEntity: // Message or Enum types, which may be nested if p, ok := en.Parent().(pgs.Message); ok { - return pgs.Name(joinChild(c.Name(p), en.Name())) + return joinChild(c.Name(p), en.Name()) } return PGGUpperCamelCase(en.Name()) case pgs.Field: // field names cannot conflict with other generated methods @@ -31,9 +31,9 @@ func (c context) Name(node pgs.Node) pgs.Name { return replaceProtected(PGGUpperCamelCase(en.Name())) case pgs.EnumValue: // EnumValue are prefixed with the enum name if _, ok := en.Enum().Parent().(pgs.File); ok { - return pgs.Name(joinNames(c.Name(en.Enum()), en.Name())) + return joinNames(c.Name(en.Enum()), en.Name()) } - return pgs.Name(joinNames(c.Name(en.Enum().Parent()), en.Name())) + return joinNames(c.Name(en.Enum().Parent()), en.Name()) case pgs.Service: // always return the server name return c.ServerName(en) case pgs.Entity: // any other entity should be just upper-camel-cased @@ -44,7 +44,7 @@ func (c context) Name(node pgs.Node) pgs.Name { } func (c context) OneofOption(field pgs.Field) pgs.Name { - n := pgs.Name(joinNames(c.Name(field.Message()), c.Name(field))) + n := joinNames(c.Name(field.Message()), c.Name(field)) for _, msg := range field.Message().Messages() { if c.Name(msg) == n { diff --git a/lang/go/package_test.go b/lang/go/package_test.go index c86ef08..ecb47bd 100644 --- a/lang/go/package_test.go +++ b/lang/go/package_test.go @@ -62,12 +62,6 @@ func TestImportPath(t *testing.T) { "example.com/fizz/buzz", "example.com/quux", }, - { // import_prefix param prefixes everything...pretty much doesn't work since it also prefixes the proto package - "import_prefix", - "foo.bar/example.com/packages/targets/fully_qualified", - "foo.bar/targets/unqualified", - "foo.bar/fizz/buzz", - }, } for _, test := range tests { @@ -109,8 +103,6 @@ func TestOutputPath(t *testing.T) { {"unqualified_srcrel", "unqualified.proto", "unqualified.pb.go"}, {"qualified", "qualified.proto", "example.com/qualified/qualified.pb.go"}, {"qualified_srcrel", "qualified.proto", "qualified.pb.go"}, - {"import_prefix", "prefix.proto", "example.com/import_prefix/prefix.pb.go"}, - {"import_prefix_srcrel", "prefix.proto", "prefix.pb.go"}, {"mapped", "mapped.proto", "mapped.pb.go"}, {"mapped_srcrel", "mapped.proto", "mapped.pb.go"}, } diff --git a/lang/go/testdata/outputs/import_prefix/params b/lang/go/testdata/outputs/import_prefix/params deleted file mode 100644 index 187385e..0000000 --- a/lang/go/testdata/outputs/import_prefix/params +++ /dev/null @@ -1 +0,0 @@ -import_prefix=foo/ diff --git a/lang/go/testdata/outputs/import_prefix/prefix.proto b/lang/go/testdata/outputs/import_prefix/prefix.proto deleted file mode 100644 index eeca6bd..0000000 --- a/lang/go/testdata/outputs/import_prefix/prefix.proto +++ /dev/null @@ -1,4 +0,0 @@ -syntax="proto3"; -package outputs.import_prefix; -option go_package = "example.com/import_prefix"; -message ImportPrefix {} diff --git a/lang/go/testdata/outputs/import_prefix_srcrel/params b/lang/go/testdata/outputs/import_prefix_srcrel/params deleted file mode 100644 index b2b1469..0000000 --- a/lang/go/testdata/outputs/import_prefix_srcrel/params +++ /dev/null @@ -1,2 +0,0 @@ -paths=source_relative,import_prefix=foo/ - diff --git a/lang/go/testdata/outputs/import_prefix_srcrel/prefix.proto b/lang/go/testdata/outputs/import_prefix_srcrel/prefix.proto deleted file mode 100644 index eeca6bd..0000000 --- a/lang/go/testdata/outputs/import_prefix_srcrel/prefix.proto +++ /dev/null @@ -1,4 +0,0 @@ -syntax="proto3"; -package outputs.import_prefix; -option go_package = "example.com/import_prefix"; -message ImportPrefix {} diff --git a/lang/go/testdata/packages/import_prefix/import_prefix.proto b/lang/go/testdata/packages/import_prefix/import_prefix.proto deleted file mode 100644 index b6fc199..0000000 --- a/lang/go/testdata/packages/import_prefix/import_prefix.proto +++ /dev/null @@ -1,13 +0,0 @@ -syntax="proto3"; -package packages.import_prefix; -option go_package="example.com/packages/import_prefix"; - -import "targets/fully_qualified/fully_qualified.proto"; -import "targets/unqualified/unqualified.proto"; -import "targets/none/none.proto"; - -message ImportPrefixed { - targets.fully_qualified.FullyQualified fully = 1; - targets.unqualified.Unqualified unqualified = 2; - targets.none.None none = 3; -} diff --git a/lang/go/testdata/packages/import_prefix/params b/lang/go/testdata/packages/import_prefix/params deleted file mode 100644 index e25d003..0000000 --- a/lang/go/testdata/packages/import_prefix/params +++ /dev/null @@ -1 +0,0 @@ -Mtargets/none/none.proto=fizz/buzz,import_prefix=foo.bar/ diff --git a/lang/go/type_name_p2_presence_test.go b/lang/go/type_name_p2_presence_test.go index c09cd7d..03b98e6 100644 --- a/lang/go/type_name_p2_presence_test.go +++ b/lang/go/type_name_p2_presence_test.go @@ -1,3 +1,4 @@ +//go:build !proto3_presence // +build !proto3_presence package pgsgo diff --git a/method.go b/method.go index f9aeffa..ff45731 100644 --- a/method.go +++ b/method.go @@ -93,4 +93,4 @@ func (m *method) childAtPath(path []int32) Entity { func (m *method) addSourceCodeInfo(info SourceCodeInfo) { m.info = info } -var m Method = (*method)(nil) +var _ Method = (*method)(nil)