Merge pull request #29492 from hashicorp/jbardin/staticcheck

Add staticcheck to CI
This commit is contained in:
James Bardin 2021-09-01 11:44:47 -04:00 committed by GitHub
commit 05f21cdff8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
31 changed files with 55 additions and 184 deletions

View File

@ -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: |

View File

@ -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

2
go.mod
View File

@ -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

2
go.sum
View File

@ -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=

View File

@ -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 {

View File

@ -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

View File

@ -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)

View File

@ -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)

View File

@ -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")
}
}

View File

@ -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 {

View File

@ -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
}

View File

@ -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) {

View File

@ -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.

View File

@ -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
}

View File

@ -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)

View File

@ -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",

View File

@ -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(

View File

@ -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

View File

@ -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)

View File

@ -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)

View File

@ -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

View File

@ -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
}
}

View File

@ -160,7 +160,9 @@ func marshalVertexID(v Vertex) string {
case reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr, reflect.Slice, reflect.UnsafePointer:
return strconv.Itoa(int(val.Pointer()))
case reflect.Interface:
return strconv.Itoa(int(val.InterfaceData()[1]))
// A vertex shouldn't contain another layer of interface, but handle
// this just in case.
return fmt.Sprintf("%#v", val.Interface())
}
if v, ok := v.(Hashable); ok {

View File

@ -306,6 +306,7 @@ func (c *Context) prePlanFindAndApplyMoves(config *configs.Config, prevRunState
break
}
}
//lint:ignore SA9003 TODO
if !matchesTarget {
// TODO: Return an error stating that a targeted plan is
// only valid if it includes this address that was moved.

View File

@ -6,20 +6,6 @@ import (
"github.com/hashicorp/terraform/internal/dag"
)
// testGraphContains is an assertion helper that tests that a node is
// contained in the graph.
func testGraphContains(t *testing.T, g *Graph, name string) {
for _, v := range g.Vertices() {
if dag.VertexName(v) == name {
return
}
}
t.Fatalf(
"Expected %q in:\n\n%s",
name, g.String())
}
// testGraphnotContains is an assertion helper that tests that a node is
// NOT contained in the graph.
func testGraphNotContains(t *testing.T, g *Graph, name string) {

View File

@ -1,16 +0,0 @@
package terraform
const errPluginInit = `
Plugins are external binaries that Terraform uses to access and manipulate
resources. The configuration provided requires plugins which can't be located,
don't satisfy the version constraints, or are otherwise incompatible.
Terraform automatically discovers provider requirements from your
configuration, including providers used in child modules. To see the
requirements and constraints, run "terraform providers".
%s
Plugin reinitialization required. Please address the above error(s) and run
"terraform init".
`

View File

@ -46,33 +46,6 @@ func mockProviderWithResourceTypeSchema(name string, schema *configschema.Block)
}
}
// mockProviderWithProviderSchema is a test helper to create a mock provider
// from an existing ProviderSchema.
func mockProviderWithProviderSchema(providerSchema ProviderSchema) *MockProvider {
p := &MockProvider{
GetProviderSchemaResponse: &providers.GetProviderSchemaResponse{
Provider: providers.Schema{
Block: providerSchema.Provider,
},
ResourceTypes: map[string]providers.Schema{},
DataSources: map[string]providers.Schema{},
},
}
for name, schema := range providerSchema.ResourceTypes {
p.GetProviderSchemaResponse.ResourceTypes[name] = providers.Schema{
Block: schema,
Version: int64(providerSchema.ResourceTypeSchemaVersions[name]),
}
}
for name, schema := range providerSchema.DataSources {
p.GetProviderSchemaResponse.DataSources[name] = providers.Schema{Block: schema}
}
return p
}
// getProviderSchemaResponseFromProviderSchema is a test helper to convert a
// ProviderSchema to a GetProviderSchemaResponse for use when building a mock provider.
func getProviderSchemaResponseFromProviderSchema(providerSchema *ProviderSchema) *providers.GetProviderSchemaResponse {

View File

@ -1,12 +1,12 @@
#!/usr/bin/env bash
# Check gofmt
echo "==> Checking that code complies with gofmt requirements..."
gofmt_files=$(gofmt -l `find . -name '*.go' | grep -v vendor`)
# Check go fmt
echo "==> Checking that code complies with go fmt requirements..."
gofmt_files=$(go fmt ./...)
if [[ -n ${gofmt_files} ]]; then
echo 'gofmt needs running on the following files:'
echo "${gofmt_files}"
echo "You can use the command: \`gofmt -w .\` to reformat code."
echo "You can use the command: \`go fmt\` to reformat code."
exit 1
fi

16
scripts/staticcheck.sh Executable file
View File

@ -0,0 +1,16 @@
#!/usr/bin/env bash
echo "==> Checking that code complies with static analysis requirements..."
# Skip legacy code which is frozen, and can be removed once we can refactor the
# remote backends to no longer require it.
skip="internal/legacy|backend/remote-state/"
# Skip generated code for protobufs.
skip=$skip"|internal/planproto|internal/tfplugin5|internal/tfplugin6"
packages=$(go list ./... | egrep -v ${skip})
# We are skipping style-related checks, since terraform intentionally breaks
# some of these. The goal here is to find issues that reduce code clarity, or
# may result in bugs.
staticcheck -checks 'all,-ST*' ${packages}

View File

@ -80,7 +80,7 @@ func main() {
if err != nil {
log.Fatal(err)
}
protocGenGoGrpcExec, err := buildProtocGenGoGrpc(workDir)
_, err = buildProtocGenGoGrpc(workDir)
if err != nil {
log.Fatal(err)
}
@ -93,7 +93,7 @@ func main() {
if err != nil {
log.Fatal(err)
}
protocGenGoGrpcExec, err = filepath.Abs(protocGenGoExec)
protocGenGoGrpcExec, err := filepath.Abs(protocGenGoExec)
if err != nil {
log.Fatal(err)
}

View File

@ -10,4 +10,5 @@ import (
_ "golang.org/x/tools/cmd/cover"
_ "golang.org/x/tools/cmd/stringer"
_ "google.golang.org/grpc/cmd/protoc-gen-go-grpc"
_ "honnef.co/go/tools/cmd/staticcheck"
)