Enable staticcheck and fix issues (uber-go#302)

This adds staticcheck to our `make lint` and fixes issues reported by it.

Of the failures reported by staticcheck, the following were actual
issues:

- shallowCheckDependencies: addMissingNodes was unused. I think this was
  intended to go into visualiziation somehow, but we never did it.
- digSentinel fields on dig.In/Out can be hidden away
- A test validating that we don't allow the `Group` provide option for
  `dig.Out`-based structs was not actually using the result struct
  declared there so it wasn't testing what it claimed to test.

Besides that, there were a number of unused fields in test structs.
These are intentionally unused fields so I've appeased the linter by
referencing them with `_ = ..` assignments.

Author: abhinav

.github/workflows/go.yml

@@ -36,7 +36,7 @@ jobs:
           ${{ runner.os }}-go-
 
     - name: Download Dependencies
-      run: go mod download
+      run: make install
 
     - name: Lint
       if: matrix.latest

Makefile

@@ -1,36 +1,52 @@
 export GOBIN ?= $(shell pwd)/bin
 
-BENCH_FLAGS ?= -cpuprofile=cpu.pprof -memprofile=mem.pprof -benchmem
 GOLINT = $(GOBIN)/golint
+STATICCHECK = $(GOBIN)/staticcheck
+
+BENCH_FLAGS ?= -cpuprofile=cpu.pprof -memprofile=mem.pprof -benchmem
 
-GO_FILES := $(shell \
+GO_FILES = $(shell \
 	find . '(' -path '*/.*' -o -path './vendor' ')' -prune \
 	-o -name '*.go' -print | cut -b3-)
 
+MODULES = . ./tools
+
 .PHONY: all
 all: build lint test
 
 .PHONY: build
 build:
 	go build ./...
 
+.PHONY: install
+install:
+	$(foreach dir,$(MODULES),( \
+		cd $(dir) && \
+		go mod download) && \
+	) true
+
 .PHONY: lint
-lint: $(GOLINT)
+lint: $(GOLINT) $(STATICCHECK)
 	@rm -rf lint.log
 	@echo "Checking formatting..."
 	@gofmt -d -s $(GO_FILES) 2>&1 | tee lint.log
 	@echo "Checking vet..."
 	@go vet ./... 2>&1 | tee -a lint.log
 	@echo "Checking lint..."
 	@$(GOLINT) ./... 2>&1 | tee -a lint.log
+	@echo "Checking staticcheck..."
+	@$(STATICCHECK) ./... 2>&1 | tee -a lint.log
 	@echo "Checking for unresolved FIXMEs..."
 	@git grep -i fixme | grep -v -e Makefile | tee -a lint.log
 	@echo "Checking for license headers..."
 	@./check_license.sh | tee -a lint.log
 	@[ ! -s lint.log ]
 
-$(GOLINT):
-	go install golang.org/x/lint/golint
+$(GOLINT): tools/go.mod
+	cd tools && go install golang.org/x/lint/golint
+
+$(STATICCHECK): tools/go.mod
+	cd tools && go install honnef.co/go/tools/cmd/staticcheck
 
 .PHONY: test
 test:
@@ -45,3 +61,7 @@ cover:
 BENCH ?= .
 bench:
 	go list ./... | xargs -n1 go test -bench=$(BENCH) -run="^$$" $(BENCH_FLAGS)
+
+.PHONY: tidy
+tidy:
+	$(foreach dir,$(MODULES),(cd $(dir) && go mod tidy) &&) true

dig_test.go

@@ -1067,8 +1067,9 @@ func TestGroups(t *testing.T) {
 		}
 
 		func(i int) {
-			require.Error(t, c.Provide(func() {
+			require.Error(t, c.Provide(func() out {
 				t.Fatal("This should not be called")
+				return out{}
 			}, Group("val")), "This Provide should fail")
 		}(1)
 	})
@@ -1425,7 +1426,7 @@ func TestProvideConstructorErrors(t *testing.T) {
 
 	t.Run("name option cannot be provided for result structs", func(t *testing.T) {
 		c := New()
-		type A struct{ idx int }
+		type A struct{}
 
 		type out struct {
 			Out
@@ -2660,6 +2661,9 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 			A1 A // all is good
 			a2 A // oops, unexported type
 		}
+
+		_ = in{}.a2 // unused but needed for the test
+
 		require.NoError(t, c.Provide(func() A { return A{} }))
 
 		err := c.Invoke(func(i in) { assert.Fail(t, "should never get in here") })
@@ -2679,6 +2683,8 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 			foo string
 		}
 
+		_ = in{}.foo // unused but needed for the test
+
 		err := c.Provide(func(in) int { return 0 })
 		require.Error(t, err, "Provide must fail")
 		assertErrorMatches(t, err,
@@ -2702,6 +2708,9 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 		type in struct {
 			Embed
 		}
+
+		_ = in{}.a2 // unused but needed for the test
+
 		require.NoError(t, c.Provide(func() A { return A{} }))
 
 		err := c.Invoke(func(i in) { assert.Fail(t, "should never get in here") })
@@ -2721,6 +2730,9 @@ func testInvokeFailures(t *testing.T, dryRun bool) {
 
 			string // embed an unexported std type
 		}
+
+		_ = param{}.string // unused but needed for the test
+
 		err := c.Invoke(func(p param) { assert.Fail(t, "should never get here") })
 		require.Error(t, err)
 		assertErrorMatches(t, err,
@@ -3229,6 +3241,7 @@ func TestUnexportedFieldsFailures(t *testing.T) {
 		type type1 struct{}
 		type type2 struct{}
 		type type3 struct{}
+
 		constructor := func() (*type1, *type2) {
 			return &type1{}, &type2{}
 		}
@@ -3241,10 +3254,12 @@ func TestUnexportedFieldsFailures(t *testing.T) {
 			T2 *type2 `optional:"true"` // optional type present in the graph
 			t3 *type3
 		}
+
 		require.NoError(t, c.Provide(constructor))
 		err := c.Invoke(func(p param) {
 			require.NotNil(t, p.T1, "whole param struct should not be nil")
 			assert.NotNil(t, p.T2, "optional type in the graph should not return nil")
+			_ = p.t3 // unused
 		})
 		require.Error(t, err)
 		assert.Contains(t, err.Error(),
@@ -3267,10 +3282,12 @@ func TestUnexportedFieldsFailures(t *testing.T) {
 			T2 *type2 `optional:"true"` // optional type present in the graph
 			t3 *type3
 		}
+
 		require.NoError(t, c.Provide(constructor))
 		err := c.Invoke(func(p param) {
 			require.NotNil(t, p.T1, "whole param struct should not be nil")
 			assert.NotNil(t, p.T2, "optional type in the graph should not return nil")
+			_ = p.t3
 		})
 		require.Error(t, err)
 		assert.Contains(t, err.Error(),

go.mod

@@ -2,8 +2,4 @@ module go.uber.org/dig
 
 go 1.13
 
-require (
-	github.com/stretchr/testify v1.4.0
-	golang.org/x/lint v0.0.0-20190930215403-16217165b5de
-	golang.org/x/tools v0.0.0-20191030062658-86caa796c7ab // indirect
-)
+require github.com/stretchr/testify v1.4.0

go.sum

@@ -5,19 +5,6 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
-golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/lint v0.0.0-20190930215403-16217165b5de h1:5hukYrvBGR8/eNkX5mdUezrA6JiaEZDtJb9Ei+1LlBs=
-golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHlgUYUwCkIfeOF89ocIRzGO/8vkc=
-golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
-golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
-golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
-golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
-golang.org/x/tools v0.0.0-20190311212946-11955173bddd h1:/e+gpKk9r3dJobndpTytxS2gOy6m5uvpg+ISQoEcusQ=
-golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs=
-golang.org/x/tools v0.0.0-20191030062658-86caa796c7ab h1:tpc/nJ4vD66vAk/2KN0sw/DvQIz2sKmCpWvyKtPmfMQ=
-golang.org/x/tools v0.0.0-20191030062658-86caa796c7ab/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
-golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=

inout.go

@@ -35,12 +35,10 @@ var (
 	_outType    = reflect.TypeOf(Out{})
 )
 
-// Special interface embedded inside dig sentinel values (dig.In, dig.Out) to
-// make their special nature obvious in the godocs. Otherwise they will appear
-// as plain empty structs.
-type digSentinel interface {
-	digSentinel()
-}
+// Placeholder type placed in dig.In/dig.out to make their special nature
+// obvious in godocs.
+// Otherwise they will appear as plain empty structs.
+type digSentinel struct{}
 
 // In may be embedded into structs to request dig to treat them as special
 // parameter structs. When a constructor accepts such a struct, instead of the
@@ -58,7 +56,7 @@ type digSentinel interface {
 //   group       Name of the Value Group from which this field will be filled.
 //               The field must be a slice type. See Value Groups in the
 //               package documentation for more information.
-type In struct{ digSentinel }
+type In struct{ _ digSentinel }
 
 // Out is an embeddable type that signals to dig that the returned
 // struct should be treated differently. Instead of the struct itself
@@ -79,7 +77,7 @@ type In struct{ digSentinel }
 //   group       Name of the Value Group to which this field's value is being
 //               sent. See Value Groups in the package documentation for more
 //               information.
-type Out struct{ digSentinel }
+type Out struct{ _ digSentinel }
 
 func isError(t reflect.Type) bool {
 	return t.Implements(_errType)

invoke.go

@@ -25,7 +25,6 @@ import (
 	"reflect"
 
 	"go.uber.org/dig/internal/digreflect"
-	"go.uber.org/dig/internal/dot"
 	"go.uber.org/dig/internal/graph"
 )
 
@@ -95,12 +94,10 @@ func (c *Container) Invoke(function interface{}, opts ...InvokeOption) error {
 // the container. Returns an error if not.
 func shallowCheckDependencies(c containerStore, pl paramList) error {
 	var err errMissingTypes
-	var addMissingNodes []*dot.Param
 
 	missingDeps := findMissingDependencies(c, pl.Params...)
 	for _, dep := range missingDeps {
 		err = append(err, newErrMissingTypes(c, key{name: dep.Name, t: dep.Type})...)
-		addMissingNodes = append(addMissingNodes, dep.DotParam()...)
 	}
 
 	if len(err) > 0 {

param_test.go

@@ -112,6 +112,8 @@ func TestParamObjectWithUnexportedFieldsSuccess(t *testing.T) {
 		t2 type2
 	}
 
+	_ = in{}.t2 // unused
+
 	po, err := newParamObject(reflect.TypeOf(in{}), New())
 	require.NoError(t, err)
 
@@ -134,6 +136,8 @@ func TestParamObjectFailure(t *testing.T) {
 			a2 A
 		}
 
+		_ = in{}.a2 // unused but needed
+
 		_, err := newParamObject(reflect.TypeOf(in{}), New())
 		require.Error(t, err)
 		assert.Contains(t, err.Error(),
@@ -149,6 +153,8 @@ func TestParamObjectFailure(t *testing.T) {
 			a2 A
 		}
 
+		_ = in{}.a2 // unused but needed
+
 		_, err := newParamObject(reflect.TypeOf(in{}), New())
 		require.Error(t, err)
 		assert.Contains(t, err.Error(),
@@ -164,6 +170,8 @@ func TestParamObjectFailure(t *testing.T) {
 			a2 A
 		}
 
+		_ = in{}.a2 // unused but needed
+
 		_, err := newParamObject(reflect.TypeOf(in{}), New())
 		require.Error(t, err)
 		assert.Contains(t, err.Error(),

tools/doc.go

@@ -0,0 +1,24 @@
+// Copyright (c) 2021 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 tools exists to make this directory a valid Go package.
+// The tools.go has a build tag that excludes it from being considered by Go
+// tooling except for dependency constraints.
+package tools

tools/go.mod

@@ -0,0 +1,11 @@
+module github.com/uber-go/dig/tools
+
+go 1.16
+
+require (
+	github.com/BurntSushi/toml v0.4.1 // indirect
+	golang.org/x/lint v0.0.0-20210508222113-6edffad5e616
+	golang.org/x/sys v0.0.0-20211124211545-fe61309f8881 // indirect
+	golang.org/x/tools v0.1.8 // indirect
+	honnef.co/go/tools v0.2.2
+)