Skip to content

Enable gosimple linter and fix issues #880

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 4, 2022
Merged

Enable gosimple linter and fix issues #880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ linters:
- errcheck
- exportloopref
- gofmt
# - gosimple # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
- gosimple
# - ineffassign # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
# - makezero # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
- nilerr
Expand Down
4 changes: 2 additions & 2 deletions helper/resource/testing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ func runSweepers(regions []string, sweepers map[string]*Sweeper, allowFailures b
return sweeperRunList, fmt.Errorf("sweeper (%s) for region (%s) failed: %s", sweeper.Name, region, err)
}
}
elapsed := time.Now().Sub(start)
elapsed := time.Since(start)
log.Printf("Completed Sweepers for region (%s) in %s", region, elapsed)

log.Printf("Sweeper Tests for region (%s) ran successfully:\n", region)
Expand Down Expand Up @@ -239,7 +239,7 @@ func runSweeperWithRegion(region string, s *Sweeper, sweepers map[string]*Sweepe

start := time.Now()
runE := s.F(region)
elapsed := time.Now().Sub(start)
elapsed := time.Since(start)

log.Printf("[DEBUG] Completed Sweeper (%s) in region (%s) in %s", s.Name, region, elapsed)

Expand Down
5 changes: 1 addition & 4 deletions helper/schema/field_reader_map.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,8 @@ func (r *MapFieldReader) readSet(
// "ports.1", but the "state" map might have those plus "ports.2".
// We don't want "ports.2"
countActual[idx] = struct{}{}
if len(countActual) >= countExpected {
return false
}

return true
Comment on lines -162 to -166
Copy link
Contributor

@detro detro Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I this is funny :)

return len(countActual) < countExpected
})
if !completed && err != nil {
return FieldReadResult{}, err
Expand Down
2 changes: 1 addition & 1 deletion helper/schema/resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ func validateResourceID(s *Schema) error {

// ID should at least be computed. If unspecified it will be set to Computed and Optional,
// but Optional is unnecessary if undesired.
if s.Computed != true {
if !s.Computed {
return fmt.Errorf(`the "id" attribute must be marked Computed`)
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions helper/schema/set.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package schema
import (
"bytes"
"fmt"
"github.com/google/go-cmp/cmp"
"reflect"
"sort"
"strconv"
"sync"

"github.com/google/go-cmp/cmp"
"github.com/hashicorp/terraform-plugin-sdk/v2/internal/helper/hashcode"
)

Expand Down Expand Up @@ -238,6 +238,6 @@ func (s *Set) listCode() []string {
for k := range s.m {
keys = append(keys, k)
}
sort.Sort(sort.StringSlice(keys))
sort.Strings(keys)
return keys
}
8 changes: 4 additions & 4 deletions helper/validation/float_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@ func TestValidateFloatAtLeast(t *testing.T) {
{
val: 1.5,
f: FloatAtLeast(2.5),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at least \\(2\\.5\\d*\\), got 1\\.5\\d*"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(2\.5\d*\), got 1\.5\d*`),
},
{
val: "2.5",
f: FloatAtLeast(1.5),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be float"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be float`),
},
})
}
Expand All @@ -86,12 +86,12 @@ func TestValidateFloatAtMost(t *testing.T) {
{
val: 2.5,
f: FloatAtMost(1.5),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at most \\(1\\.5\\d*\\), got 2\\.5\\d*"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at most \(1\.5\d*\), got 2\.5\d*`),
},
{
val: "2.5",
f: FloatAtMost(3.5),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be float"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be float`),
},
})
}
16 changes: 8 additions & 8 deletions helper/validation/int_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ func TestValidationIntBetween(t *testing.T) {
{
val: 1,
f: IntBetween(2, 3),
expectedErr: regexp.MustCompile("expected [\\w]+ to be in the range \\(2 - 3\\), got 1"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be in the range \(2 - 3\), got 1`),
},
{
val: "1",
f: IntBetween(2, 3),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be integer"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be integer`),
},
})
}
Expand All @@ -41,12 +41,12 @@ func TestValidationIntAtLeast(t *testing.T) {
{
val: 1,
f: IntAtLeast(2),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at least \\(2\\), got 1"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(2\), got 1`),
},
{
val: "1",
f: IntAtLeast(2),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be integer"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be integer`),
},
})
}
Expand All @@ -64,12 +64,12 @@ func TestValidationIntAtMost(t *testing.T) {
{
val: 1,
f: IntAtMost(0),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at most \\(0\\), got 1"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at most \(0\), got 1`),
},
{
val: "1",
f: IntAtMost(0),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be integer"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be integer`),
},
})
}
Expand Down Expand Up @@ -119,12 +119,12 @@ func TestValidationIntInSlice(t *testing.T) {
{
val: 42,
f: IntInSlice([]int{10, 20}),
expectedErr: regexp.MustCompile("expected [\\w]+ to be one of \\[10 20\\], got 42"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be one of \[10 20\], got 42`),
},
{
val: "InvalidValue",
f: IntInSlice([]int{10, 20}),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be integer"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be integer`),
},
})
}
Expand Down
10 changes: 5 additions & 5 deletions helper/validation/meta_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestValidationAll(t *testing.T) {
StringLenBetween(5, 42),
StringMatch(regexp.MustCompile(`[a-zA-Z0-9]+`), "value must be alphanumeric"),
),
expectedErr: regexp.MustCompile("expected length of [\\w]+ to be in the range \\(5 - 42\\), got foo"),
expectedErr: regexp.MustCompile(`expected length of [\w]+ to be in the range \(5 - 42\), got foo`),
},
{
val: "!!!!!",
Expand Down Expand Up @@ -87,15 +87,15 @@ func TestValidationAny(t *testing.T) {
IntAtLeast(42),
IntAtMost(5),
),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at least \\(42\\), got 7"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(42\), got 7`),
},
{
val: 7,
f: Any(
IntAtLeast(42),
IntAtMost(5),
),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at most \\(5\\), got 7"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at most \(5\), got 7`),
},
})
}
Expand All @@ -122,15 +122,15 @@ func TestToDiagFunc(t *testing.T) {
IntAtLeast(42),
IntAtMost(5),
)),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at least \\(42\\), got 7"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(42\), got 7`),
},
{
val: 7,
f: ToDiagFunc(Any(
IntAtLeast(42),
IntAtMost(5),
)),
expectedErr: regexp.MustCompile("expected [\\w]+ to be at most \\(5\\), got 7"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be at most \(5\), got 7`),
},
})
}
20 changes: 10 additions & 10 deletions helper/validation/strings_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,17 +290,17 @@ func TestValidationStringInSlice(t *testing.T) {
{
val: "VALIDVALUE",
f: StringInSlice([]string{"ValidValue", "AnotherValidValue"}, false),
expectedErr: regexp.MustCompile("expected [\\w]+ to be one of \\[ValidValue AnotherValidValue\\], got VALIDVALUE"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be one of \[ValidValue AnotherValidValue\], got VALIDVALUE`),
},
{
val: "InvalidValue",
f: StringInSlice([]string{"ValidValue", "AnotherValidValue"}, false),
expectedErr: regexp.MustCompile("expected [\\w]+ to be one of \\[ValidValue AnotherValidValue\\], got InvalidValue"),
expectedErr: regexp.MustCompile(`expected [\w]+ to be one of \[ValidValue AnotherValidValue\], got InvalidValue`),
},
{
val: 1,
f: StringInSlice([]string{"ValidValue", "AnotherValidValue"}, false),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be string"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be string`),
},
})
}
Expand All @@ -319,18 +319,18 @@ func TestValidationStringNotInSlice(t *testing.T) {
{
val: "AnotherInvalidValue",
f: StringNotInSlice([]string{"InvalidValue", "AnotherInvalidValue"}, false),
expectedErr: regexp.MustCompile("expected [\\w]+ to not be any of \\[InvalidValue AnotherInvalidValue\\], got AnotherInvalidValue"),
expectedErr: regexp.MustCompile(`expected [\w]+ to not be any of \[InvalidValue AnotherInvalidValue\], got AnotherInvalidValue`),
},
// ignore case
{
val: "INVALIDVALUE",
f: StringNotInSlice([]string{"InvalidValue", "AnotherInvalidValue"}, true),
expectedErr: regexp.MustCompile("expected [\\w]+ to not be any of \\[InvalidValue AnotherInvalidValue\\], got INVALIDVALUE"),
expectedErr: regexp.MustCompile(`expected [\w]+ to not be any of \[InvalidValue AnotherInvalidValue\], got INVALIDVALUE`),
},
{
val: 1,
f: StringNotInSlice([]string{"InvalidValue", "AnotherInvalidValue"}, false),
expectedErr: regexp.MustCompile("expected type of [\\w]+ to be string"),
expectedErr: regexp.MustCompile(`expected type of [\w]+ to be string`),
},
})
}
Expand All @@ -344,12 +344,12 @@ func TestValidationStringMatch(t *testing.T) {
{
val: "bar",
f: StringMatch(regexp.MustCompile(".*foo.*"), ""),
expectedErr: regexp.MustCompile("expected value of [\\w]+ to match regular expression " + regexp.QuoteMeta(`".*foo.*"`)),
expectedErr: regexp.MustCompile(`expected value of [\w]+ to match regular expression ` + regexp.QuoteMeta(`".*foo.*"`)),
},
{
val: "bar",
f: StringMatch(regexp.MustCompile(".*foo.*"), "value must contain foo"),
expectedErr: regexp.MustCompile("invalid value for [\\w]+ \\(value must contain foo\\)"),
expectedErr: regexp.MustCompile(`invalid value for [\w]+ \(value must contain foo\)`),
},
})
}
Expand All @@ -363,12 +363,12 @@ func TestValidationStringDoesNotMatch(t *testing.T) {
{
val: "bar",
f: StringDoesNotMatch(regexp.MustCompile(".*bar.*"), ""),
expectedErr: regexp.MustCompile("expected value of [\\w]+ to not match regular expression " + regexp.QuoteMeta(`".*bar.*"`)),
expectedErr: regexp.MustCompile(`expected value of [\w]+ to not match regular expression ` + regexp.QuoteMeta(`".*bar.*"`)),
},
{
val: "bar",
f: StringDoesNotMatch(regexp.MustCompile(".*bar.*"), "value must not contain foo"),
expectedErr: regexp.MustCompile("invalid value for [\\w]+ \\(value must not contain foo\\)"),
expectedErr: regexp.MustCompile(`invalid value for [\w]+ \(value must not contain foo\)`),
},
})
}
Expand Down
2 changes: 0 additions & 2 deletions internal/addrs/module_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ func parseModuleInstancePrefix(traversal hcl.Traversal) (ModuleInstance, hcl.Tra
"Invalid address operator",
"Module address prefix must be followed by dot and then a name.",
))
break
}

if next != "module" {
Expand Down Expand Up @@ -122,7 +121,6 @@ func parseModuleInstancePrefix(traversal hcl.Traversal) (ModuleInstance, hcl.Tra
"Invalid address operator",
"Prefix \"module.\" must be followed by a module name.",
))
break
}
remain = remain[1:]
step := ModuleInstanceStep{
Expand Down
8 changes: 4 additions & 4 deletions internal/plugin/convert/diagnostics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,13 @@ func AttributePathToPath(ap *tftypes.AttributePath) cty.Path {
return p
}
for _, step := range ap.Steps() {
switch step.(type) {
switch step := step.(type) {
case tftypes.AttributeName:
p = p.GetAttr(string(step.(tftypes.AttributeName)))
p = p.GetAttr(string(step))
case tftypes.ElementKeyString:
p = p.Index(cty.StringVal(string(step.(tftypes.ElementKeyString))))
p = p.Index(cty.StringVal(string(step)))
case tftypes.ElementKeyInt:
p = p.Index(cty.NumberIntVal(int64(step.(tftypes.ElementKeyInt))))
Comment on lines -96 to -102
Copy link
Contributor

@detro detro Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, to understand: here we are getting rid of a "double casting". Before it was going (for example) from interface{} to tftypes.AttributeName to string. Now, just interface{} to string. Correct?

Copy link
Contributor

@detro detro Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correction: it's not casting. You are removing the type assertion because the assertion was already done by the switch step.(type). So, it's safe to then do just the type conversion.

Copy link
Contributor Author

@bflad bflad Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the original issue reported in the linter:

internal/plugin/convert/diagnostics.go:96:10: S1034: assigning the result of this type assertion to a variable (switch step := step.(type)) could eliminate type assertions in switch cases (gosimple)
                switch step.(type) {
                       ^
internal/plugin/convert/diagnostics.go:98:25: S1034(related information): could eliminate this type assertion (gosimple)
                        p = p.GetAttr(string(step.(tftypes.AttributeName)))
                                             ^
internal/plugin/convert/diagnostics.go:100:37: S1034(related information): could eliminate this type assertion (gosimple)
                        p = p.Index(cty.StringVal(string(step.(tftypes.ElementKeyString))))
                                                         ^
internal/plugin/convert/diagnostics.go:102:39: S1034(related information): could eliminate this type assertion (gosimple)
                        p = p.Index(cty.NumberIntVal(int64(step.(tftypes.ElementKeyInt))))
                                                           ^

There is a fun feature in the Go specification that allows switch statements to automatically handle type assertions when using the .(type) syntax with assignment. It's really handy for simplifying code that needs to handle multiple types, if you haven't run across it before to avoid the duplicate type handling within a case.

Copy link
Contributor

@detro detro Feb 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep yep: essentially, by the time we are in any of the specific cases of the switch, the assertion has already happened and we can avoid doing it again. Makes sense.

p = p.Index(cty.NumberIntVal(int64(step)))
}
}
return p
Expand Down
8 changes: 2 additions & 6 deletions terraform/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1023,7 +1023,7 @@ func (m *ModuleState) String() string {
}

if len(rs.Dependencies) > 0 {
buf.WriteString(fmt.Sprintf("\n Dependencies:\n"))
buf.WriteString("\n Dependencies:\n")
for _, dep := range rs.Dependencies {
buf.WriteString(fmt.Sprintf(" %s\n", dep))
}
Expand Down Expand Up @@ -1237,11 +1237,7 @@ func (s *ResourceState) Equal(other *ResourceState) bool {
}

// States must be equal
if !s.Primary.Equal(other.Primary) {
return false
}

return true
Comment on lines -1240 to -1244
Copy link
Contributor

@detro detro Feb 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another one.

return s.Primary.Equal(other.Primary)
}

// Taint marks a resource as tainted.
Expand Down