Add staticcheck to CI

.circleci/config.yml

@@ -26,7 +26,8 @@ jobs:
     steps:
       - checkout
       - run: go mod verify
-      - run: make fmtcheck generate
+      - run: go install honnef.co/go/tools/cmd/staticcheck
+      - run: make fmtcheck generate staticcheck
       - run:
           name: verify no code was generated
           command: |

Makefile

@@ -20,6 +20,9 @@ protobuf:
 fmtcheck:
 	@sh -c "'$(CURDIR)/scripts/gofmtcheck.sh'"
 
+staticcheck:
+	@sh -c "'$(CURDIR)/scripts/staticcheck.sh'"
+
 website:
 ifeq (,$(wildcard $(GOPATH)/src/$(WEBSITE_REPO)))
 	echo "$(WEBSITE_REPO) not found in your GOPATH (necessary for layouts and assets), get-ting..."
@@ -46,4 +49,4 @@ endif
 # under parallel conditions.
 .NOTPARALLEL:
 
-.PHONY: fmtcheck generate protobuf website website-test
+.PHONY: fmtcheck generate protobuf website website-test staticcheck

go.mod

@@ -14,6 +14,7 @@ require (
 	github.com/Azure/go-autorest/logger v0.2.1 // indirect
 	github.com/Azure/go-autorest/tracing v0.6.0 // indirect
 	github.com/Azure/go-ntlmssp v0.0.0-20200615164410-66371956d46c // indirect
+	github.com/BurntSushi/toml v0.3.1 // indirect
 	github.com/ChrisTrenkamp/goxpath v0.0.0-20190607011252-c5096ec8773d // indirect
 	github.com/Masterminds/goutils v1.1.0 // indirect
 	github.com/Masterminds/semver v1.5.0 // indirect
@@ -172,6 +173,7 @@ require (
 	gopkg.in/inf.v0 v0.9.0 // indirect
 	gopkg.in/ini.v1 v1.42.0 // indirect
 	gopkg.in/yaml.v2 v2.3.0 // indirect
+	honnef.co/go/tools v0.0.1-2020.1.4
 	k8s.io/api v0.0.0-20190620084959-7cf5895f2711
 	k8s.io/apimachinery v0.0.0-20190913080033-27d36303b655
 	k8s.io/client-go v10.0.0+incompatible

go.sum

@@ -74,6 +74,7 @@ github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBp
 github.com/Azure/go-ntlmssp v0.0.0-20180810175552-4a21cbd618b4/go.mod h1:chxPXzSsl7ZWRAuOIE23GDNzjWuZquvFlgA8xmpunjU=
 github.com/Azure/go-ntlmssp v0.0.0-20200615164410-66371956d46c h1:/IBSNwUN8+eKzUzbJPqhK839ygXJ82sde8x3ogr6R28=
 github.com/Azure/go-ntlmssp v0.0.0-20200615164410-66371956d46c/go.mod h1:chxPXzSsl7ZWRAuOIE23GDNzjWuZquvFlgA8xmpunjU=
+github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
 github.com/ChrisTrenkamp/goxpath v0.0.0-20170922090931-c385f95c6022/go.mod h1:nuWgzSkT5PnyOd+272uUmV0dnAnAn42Mk7PiQC5VzN4=
@@ -1061,6 +1062,7 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
+honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8=
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 k8s.io/api v0.0.0-20190620084959-7cf5895f2711 h1:BblVYz/wE5WtBsD/Gvu54KyBUTJMflolzc5I2DTvh50=
 k8s.io/api v0.0.0-20190620084959-7cf5895f2711/go.mod h1:TBhBqb1AWbBQbW3XRusr7n7E4v2+5ZY8r8sAMnyFC5A=

internal/addrs/resource.go

@@ -194,14 +194,10 @@ func (r AbsResource) absMoveableSigil() {
 	// AbsResource is moveable
 }
 
-type absResourceKey string
-
 func (r AbsResource) UniqueKey() UniqueKey {
 	return absResourceInstanceKey(r.String())
 }
 
-func (rk absResourceKey) uniqueKeySigil() {}
-
 // AbsResourceInstance is an absolute address for a resource instance under a
 // given module path.
 type AbsResourceInstance struct {

internal/backend/remote/backend.go

@@ -888,7 +888,7 @@ func (b *Remote) VerifyWorkspaceTerraformVersion(workspaceName string) tfdiags.D
 
 	// If the workspace has remote operations disabled, the remote Terraform
 	// version is effectively meaningless, so we'll skip version verification.
-	if workspace.Operations == false {
+	if !workspace.Operations {
 		return nil
 	}
 
@@ -963,22 +963,6 @@ func (b *Remote) IsLocalOperations() bool {
 	return b.forceLocal
 }
 
-// Colorize returns the Colorize structure that can be used for colorizing
-// output. This is guaranteed to always return a non-nil value and so useful
-// as a helper to wrap any potentially colored strings.
-//
-// TODO SvH: Rename this back to Colorize as soon as we can pass -no-color.
-func (b *Remote) cliColorize() *colorstring.Colorize {
-	if b.CLIColor != nil {
-		return b.CLIColor
-	}
-
-	return &colorstring.Colorize{
-		Colors:  colorstring.DefaultColors,
-		Disable: true,
-	}
-}
-
 func generalError(msg string, err error) error {
 	var diags tfdiags.Diagnostics
 

internal/backend/remote/backend_apply_test.go

@@ -1594,7 +1594,7 @@ func TestRemote_applyVersionCheck(t *testing.T) {
 			}
 
 			// RUN: prepare the apply operation and run it
-			op, configCleanup, done := testOperationApply(t, "./testdata/apply")
+			op, configCleanup, _ := testOperationApply(t, "./testdata/apply")
 			defer configCleanup()
 
 			streams, done := terminal.StreamsForTesting(t)

internal/command/apply.go

@@ -45,8 +45,7 @@ func (c *ApplyCommand) Run(rawArgs []string) int {
 
 	// Instantiate the view, even if there are flag errors, so that we render
 	// diagnostics according to the desired view
-	var view views.Apply
-	view = views.NewApply(args.ViewType, c.Destroy, c.View)
+	view := views.NewApply(args.ViewType, c.Destroy, c.View)
 
 	if diags.HasErrors() {
 		view.Diagnostics(diags)

internal/command/apply_test.go

@@ -1779,6 +1779,7 @@ func TestApply_terraformEnvNonDefault(t *testing.T) {
 			},
 		}
 		if code := newCmd.Run([]string{"test"}); code != 0 {
+			t.Fatal("error creating workspace")
 		}
 	}
 
@@ -1792,6 +1793,7 @@ func TestApply_terraformEnvNonDefault(t *testing.T) {
 			},
 		}
 		if code := selCmd.Run(args); code != 0 {
+			t.Fatal("error switching workspace")
 		}
 	}
 

internal/command/format/diff.go

@@ -70,7 +70,7 @@ func ResourceChange(
 		buf.WriteString(color.Color(fmt.Sprintf("[bold]  # %s[reset] will be [bold][red]destroyed", dispAddr)))
 		if change.DeposedKey != states.NotDeposed {
 			// Some extra context about this unusual situation.
-			buf.WriteString(color.Color(fmt.Sprint("\n  # (left over from a partially-failed replacement of this instance)")))
+			buf.WriteString(color.Color("\n  # (left over from a partially-failed replacement of this instance)"))
 		}
 	default:
 		// should never happen, since the above is exhaustive
@@ -755,8 +755,6 @@ func (p *blockBodyDiffPrinter) writeNestedAttrDiff(
 			p.buf.WriteString(" -> (known after apply)")
 		}
 	}
-
-	return
 }
 
 func (p *blockBodyDiffPrinter) writeNestedBlockDiffs(name string, blockS *configschema.NestedBlock, old, new cty.Value, blankBefore bool, indent int, path cty.Path) int {

internal/command/jsonplan/values.go

@@ -274,28 +274,3 @@ func marshalPlanModules(
 
 	return ret, nil
 }
-
-// marshalSensitiveValues returns a map of sensitive attributes, with the value
-// set to true. It returns nil if the value is nil or if there are no sensitive
-// vals.
-func marshalSensitiveValues(value cty.Value) map[string]bool {
-	if value.RawEquals(cty.NilVal) || value.IsNull() {
-		return nil
-	}
-
-	ret := make(map[string]bool)
-
-	it := value.ElementIterator()
-	for it.Next() {
-		k, v := it.Element()
-		s := jsonstate.SensitiveAsBool(v)
-		if !s.RawEquals(cty.False) {
-			ret[k.AsString()] = true
-		}
-	}
-
-	if len(ret) == 0 {
-		return nil
-	}
-	return ret
-}

internal/command/login.go

@@ -311,13 +311,9 @@ func (c *LoginCommand) outputDefaultTFELoginSuccess(dispHostname string) {
 }
 
 func (c *LoginCommand) outputDefaultTFCLoginSuccess() {
-	c.Ui.Output(
-		fmt.Sprintf(
-			c.Colorize().Color(strings.TrimSpace(`
+	c.Ui.Output(c.Colorize().Color(strings.TrimSpace(`
 [green][bold]Success![reset] [bold]Logged in to Terraform Cloud[reset]
-`)),
-		) + "\n",
-	)
+` + "\n")))
 }
 
 func (c *LoginCommand) logMOTDError(err error) {

internal/command/meta.go

@@ -15,8 +15,6 @@ import (
 	"time"
 
 	plugin "github.com/hashicorp/go-plugin"
-	"github.com/hashicorp/hcl/v2"
-	"github.com/hashicorp/hcl/v2/hclsyntax"
 	"github.com/hashicorp/terraform-svchost/disco"
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/backend"
@@ -554,43 +552,6 @@ func (m *Meta) extendedFlagSet(n string) *flag.FlagSet {
 	return f
 }
 
-// parseTargetFlags must be called for any commands supporting -target
-// arguments. This method attempts to parse each -target flag into an
-// addrs.Target, storing in the Meta.targets slice.
-//
-// If any flags cannot be parsed, we rewrap the first error diagnostic with a
-// custom title to clarify the source of the error. The normal approach of
-// directly returning the diags from HCL or the addrs package results in
-// confusing incorrect "source" results when presented.
-func (m *Meta) parseTargetFlags() tfdiags.Diagnostics {
-	var diags tfdiags.Diagnostics
-	m.targets = nil
-	for _, tf := range m.targetFlags {
-		traversal, syntaxDiags := hclsyntax.ParseTraversalAbs([]byte(tf), "", hcl.Pos{Line: 1, Column: 1})
-		if syntaxDiags.HasErrors() {
-			diags = diags.Append(tfdiags.Sourceless(
-				tfdiags.Error,
-				fmt.Sprintf("Invalid target %q", tf),
-				syntaxDiags[0].Detail,
-			))
-			continue
-		}
-
-		target, targetDiags := addrs.ParseTarget(traversal)
-		if targetDiags.HasErrors() {
-			diags = diags.Append(tfdiags.Sourceless(
-				tfdiags.Error,
-				fmt.Sprintf("Invalid target %q", tf),
-				targetDiags[0].Description().Detail,
-			))
-			continue
-		}
-
-		m.targets = append(m.targets, target.Subject)
-	}
-	return diags
-}
-
 // process will process any -no-color entries out of the arguments. This
 // will potentially modify the args in-place. It will return the resulting
 // slice, and update the Meta and Ui.

internal/command/meta_backend.go

@@ -12,7 +12,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/hcldec"
 	"github.com/hashicorp/terraform/internal/backend"
@@ -244,7 +243,7 @@ func (m *Meta) BackendForPlan(settings plans.Backend) (backend.Enhanced, tfdiags
 	schema := b.ConfigSchema()
 	configVal, err := settings.Config.Decode(schema.ImpliedType())
 	if err != nil {
-		diags = diags.Append(errwrap.Wrapf("saved backend configuration is invalid: {{err}}", err))
+		diags = diags.Append(fmt.Errorf("saved backend configuration is invalid: %w", err))
 		return nil, diags
 	}
 

internal/command/refresh.go

@@ -26,8 +26,7 @@ func (c *RefreshCommand) Run(rawArgs []string) int {
 
 	// Instantiate the view, even if there are flag errors, so that we render
 	// diagnostics according to the desired view
-	var view views.Refresh
-	view = views.NewRefresh(args.ViewType, c.View)
+	view := views.NewRefresh(args.ViewType, c.View)
 
 	if diags.HasErrors() {
 		view.Diagnostics(diags)

internal/command/state_meta.go

@@ -121,7 +121,7 @@ func (c *StateMeta) lookupResourceInstanceAddr(state *states.State, allowMissing
 			}
 		}
 
-		if found == false && !allowMissing {
+		if !found && !allowMissing {
 			diags = diags.Append(tfdiags.Sourceless(
 				tfdiags.Error,
 				"Unknown module",

internal/command/state_mv_test.go

@@ -9,17 +9,11 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/mitchellh/cli"
-	"github.com/mitchellh/colorstring"
 
 	"github.com/hashicorp/terraform/internal/addrs"
 	"github.com/hashicorp/terraform/internal/states"
 )
 
-var disabledColorize = &colorstring.Colorize{
-	Colors:  colorstring.DefaultColors,
-	Disable: true,
-}
-
 func TestStateMv(t *testing.T) {
 	state := states.BuildState(func(s *states.SyncState) {
 		s.SetResourceInstanceCurrent(

internal/command/views/add.go

@@ -256,7 +256,7 @@ func (v *addHuman) writeConfigNestedBlock(buf *strings.Builder, name string, sch
 }
 
 func (v *addHuman) writeConfigNestedTypeAttribute(buf *strings.Builder, name string, schema *configschema.Attribute, indent int) error {
-	if schema.Required == false && v.optional == false {
+	if !schema.Required && !v.optional {
 		return nil
 	}
 
@@ -521,7 +521,6 @@ func writeAttrTypeConstraint(buf *strings.Builder, schema *configschema.Attribut
 	} else {
 		buf.WriteString(fmt.Sprintf("%s\n", schema.Type.FriendlyName()))
 	}
-	return
 }
 
 func writeBlockTypeConstraint(buf *strings.Builder, schema *configschema.NestedBlock) {
@@ -530,7 +529,6 @@ func writeBlockTypeConstraint(buf *strings.Builder, schema *configschema.NestedB
 	} else {
 		buf.WriteString(" # OPTIONAL block\n")
 	}
-	return
 }
 
 // copied from command/format/diff

internal/command/views/json/output_test.go

@@ -74,6 +74,9 @@ func TestOutputsFromMap(t *testing.T) {
 func TestOutputsFromChanges(t *testing.T) {
 	root := addrs.RootModuleInstance
 	num, err := plans.NewDynamicValue(cty.NumberIntVal(1234), cty.Number)
+	if err != nil {
+		t.Fatalf("unexpected error creating dynamic value: %v", err)
+	}
 	str, err := plans.NewDynamicValue(cty.StringVal("1234"), cty.String)
 	if err != nil {
 		t.Fatalf("unexpected error creating dynamic value: %v", err)

internal/command/views/output.go

@@ -105,10 +105,6 @@ func (v *OutputHuman) Diagnostics(diags tfdiags.Diagnostics) {
 // type of an output value is not important.
 type OutputRaw struct {
 	view *View
-
-	// Unit tests may set rawPrint to capture the output from the Output
-	// method, which would normally go to stdout directly.
-	rawPrint func(string)
 }
 
 var _ Output = (*OutputRaw)(nil)

internal/communicator/ssh/communicator.go

@@ -19,7 +19,6 @@ import (
 	"time"
 
 	"github.com/apparentlymart/go-shquot/shquot"
-	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/internal/communicator/remote"
 	"github.com/hashicorp/terraform/internal/provisioners"
 	"github.com/zclconf/go-cty/cty"
@@ -192,7 +191,7 @@ func (c *Communicator) Connect(o provisioners.UIOutput) (err error) {
 	log.Printf("[DEBUG] Connection established. Handshaking for user %v", c.connInfo.User)
 	sshConn, sshChan, req, err := ssh.NewClientConn(c.conn, hostAndPort, c.config.config)
 	if err != nil {
-		err = errwrap.Wrapf(fmt.Sprintf("SSH authentication failed (%s@%s): {{err}}", c.connInfo.User, hostAndPort), err)
+		err = fmt.Errorf("SSH authentication failed (%s@%s): %w", c.connInfo.User, hostAndPort, err)
 
 		// While in theory this should be a fatal error, some hosts may start
 		// the ssh service before it is properly configured, or before user

internal/configs/provider_requirements.go

@@ -86,6 +86,7 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia
 			continue
 		}
 
+	LOOP:
 		for _, kv := range kvs {
 			key, keyDiags := kv.Key.Value(nil)
 			if keyDiags.HasErrors() {
@@ -213,7 +214,7 @@ func decodeRequiredProvidersBlock(block *hcl.Block) (*RequiredProviders, hcl.Dia
 					Detail:   `required_providers objects can only contain "version", "source" and "configuration_aliases" attributes. To configure a provider, use a "provider" block.`,
 					Subject:  kv.Key.Range().Ptr(),
 				})
-				break
+				break LOOP
 			}
 
 		}