Enable unparam linter and fix issues (#873)

bflad · web-flow · commit 5c039b1a4393 · 2022-02-02T08:18:07.000-05:00
Reference: #865 To reduce potential confusion caused by function signatures. Previously: ```text terraform/state.go:387:30: `(*State).removeModule` - `path` is unused (unparam) func (s *State) removeModule(path []string, v *ModuleState) { ^ terraform/state.go:415:32: `(*State).removeInstance` - `path` is unused (unparam) func (s *State) removeInstance(path []string, r *ResourceState, v *InstanceState) { ^ helper/resource/testing.go:549:58: Test$1 - result 1 (error) is always nil (unparam) c.ProviderFactories[name] = func() (*schema.Provider, error) { ^ helper/resource/testing.go:1007:33: `modulePrimaryInstanceState` - `s` is unused (unparam) func modulePrimaryInstanceState(s *terraform.State, ms *terraform.ModuleState, name string) (*terraform.InstanceState, error) { ^ helper/resource/testing_test.go:39:21: TestTest_factoryError$1 - result 0 (*github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema.Provider) is always nil (unparam) factory := func() (*schema.Provider, error) { ^ helper/schema/field_reader.go:174:32: `readListField` - `schema` is unused (unparam) r FieldReader, addr []string, schema *Schema) (FieldReadResult, error) { ^ helper/schema/field_reader_config_test.go:538:17: `testConfig` - `t` is unused (unparam) func testConfig(t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig { ^ helper/schema/field_writer_map.go:118:2: `(*MapFieldWriter).setList` - `schema` is unused (unparam) schema *Schema) error { ^ helper/schema/field_writer_map.go:164:2: `(*MapFieldWriter).setMap` - `schema` is unused (unparam) schema *Schema) error { ^ helper/schema/field_writer_map.go:211:2: `(*MapFieldWriter).setObject` - `schema` is unused (unparam) schema *Schema) error { ^ helper/schema/grpc_provider.go:359:50: `(*GRPCProviderServer).upgradeFlatmapState` - `ctx` is unused (unparam) func (s *GRPCProviderServer) upgradeFlatmapState(ctx context.Context, version int, m map[string]string, res *Resource) (map[string]interface{}, int, error) { ^ helper/schema/resource.go:853:47: `isReservedResourceFieldName` - `s` is unused (unparam) func isReservedResourceFieldName(name string, s *Schema) bool { ^ helper/schema/resource_diff_test.go:41:34: `testDiffCases` - `oldPrefix` is unused (unparam) func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) []resourceDiffTestCase { ^ helper/schema/resource_test.go:557:38: `TestResourceApply_isNewResource$1` - `m` is unused (unparam) updateFunc := func(d *ResourceData, m interface{}) error { ```
diff --git a/.golangci.yml b/.golangci.yml
@@ -23,6 +23,6 @@ linters:
     # - staticcheck # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
     # - tenv # TODO: Enable when upgrading Go 1.16 to 1.17
     - unconvert
-    # - unparam # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
+    - unparam
     - varcheck
     - vet
diff --git a/helper/resource/testing.go b/helper/resource/testing.go
@@ -546,7 +546,7 @@ func Test(t testing.T, c TestCase) {
 				t.Fatalf("ProviderFactory for %q already exists, cannot overwrite with Provider", name)
 			}
 			prov := p
-			c.ProviderFactories[name] = func() (*schema.Provider, error) {
+			c.ProviderFactories[name] = func() (*schema.Provider, error) { //nolint:unparam // required signature
 				return prov, nil
 			}
 		}
@@ -1004,7 +1004,7 @@ func TestMatchOutput(name string, r *regexp.Regexp) TestCheckFunc {
 
 // modulePrimaryInstanceState returns the instance state for the given resource
 // name in a ModuleState
-func modulePrimaryInstanceState(s *terraform.State, ms *terraform.ModuleState, name string) (*terraform.InstanceState, error) {
+func modulePrimaryInstanceState(ms *terraform.ModuleState, name string) (*terraform.InstanceState, error) {
 	rs, ok := ms.Resources[name]
 	if !ok {
 		return nil, fmt.Errorf("Not found: %s in %s", name, ms.Path)
@@ -1026,14 +1026,14 @@ func modulePathPrimaryInstanceState(s *terraform.State, mp addrs.ModuleInstance,
 		return nil, fmt.Errorf("No module found at: %s", mp)
 	}
 
-	return modulePrimaryInstanceState(s, ms, name)
+	return modulePrimaryInstanceState(ms, name)
 }
 
 // primaryInstanceState returns the primary instance state for the given
 // resource name in the root module.
 func primaryInstanceState(s *terraform.State, name string) (*terraform.InstanceState, error) {
 	ms := s.RootModule()
-	return modulePrimaryInstanceState(s, ms, name)
+	return modulePrimaryInstanceState(ms, name)
 }
 
 // indexesIntoTypeSet is a heuristic to try and identify if a flatmap style
diff --git a/helper/resource/testing_test.go b/helper/resource/testing_test.go
@@ -36,7 +36,7 @@ func TestParallelTest(t *testing.T) {
 func TestTest_factoryError(t *testing.T) {
 	resourceFactoryError := fmt.Errorf("resource factory error")
 
-	factory := func() (*schema.Provider, error) {
+	factory := func() (*schema.Provider, error) { //nolint:unparam // required signature
 		return nil, resourceFactoryError
 	}
 	mt := new(mockT)
diff --git a/helper/schema/field_reader.go b/helper/schema/field_reader.go
@@ -171,7 +171,7 @@ func addrToSchema(addr []string, schemaMap map[string]*Schema) []*Schema {
 // "foo.#" for a list "foo" and that the indexes are "foo.0", "foo.1", etc.
 // after that point.
 func readListField(
-	r FieldReader, addr []string, schema *Schema) (FieldReadResult, error) {
+	r FieldReader, addr []string) (FieldReadResult, error) {
 	addrPadded := make([]string, len(addr)+1)
 	copy(addrPadded, addr)
 	addrPadded[len(addrPadded)-1] = "#"
diff --git a/helper/schema/field_reader_config.go b/helper/schema/field_reader_config.go
@@ -100,7 +100,7 @@ func (r *ConfigFieldReader) readField(
 	case TypeBool, TypeFloat, TypeInt, TypeString:
 		return r.readPrimitive(k, schema)
 	case TypeList:
-		return readListField(&nestedConfigFieldReader{r}, address, schema)
+		return readListField(&nestedConfigFieldReader{r}, address)
 	case TypeMap:
 		return r.readMap(k, schema)
 	case TypeSet:
@@ -258,7 +258,7 @@ func (r *ConfigFieldReader) readSet(
 	// Create the set that will be our result
 	set := schema.ZeroValue().(*Set)
 
-	raw, err := readListField(&nestedConfigFieldReader{r}, address, schema)
+	raw, err := readListField(&nestedConfigFieldReader{r}, address)
 	if err != nil {
 		return FieldReadResult{}, err
 	}
diff --git a/helper/schema/field_reader_config_test.go b/helper/schema/field_reader_config_test.go
@@ -536,5 +536,6 @@ func TestConfigFieldReader_computedComplexSet(t *testing.T) {
 }
 
 func testConfig(t *testing.T, raw map[string]interface{}) *terraform.ResourceConfig {
+	t.Helper()
 	return terraform.NewResourceConfigRaw(raw)
 }
diff --git a/helper/schema/field_reader_diff.go b/helper/schema/field_reader_diff.go
@@ -67,7 +67,7 @@ func (r *DiffFieldReader) ReadField(address []string) (FieldReadResult, error) {
 	case TypeBool, TypeInt, TypeFloat, TypeString:
 		res, err = r.readPrimitive(address, schema)
 	case TypeList:
-		res, err = readListField(r, address, schema)
+		res, err = readListField(r, address)
 	case TypeMap:
 		res, err = r.readMap(address, schema)
 	case TypeSet:
diff --git a/helper/schema/field_reader_map.go b/helper/schema/field_reader_map.go
@@ -24,7 +24,7 @@ func (r *MapFieldReader) ReadField(address []string) (FieldReadResult, error) {
 	case TypeBool, TypeInt, TypeFloat, TypeString:
 		return r.readPrimitive(address, schema)
 	case TypeList:
-		return readListField(r, address, schema)
+		return readListField(r, address)
 	case TypeMap:
 		return r.readMap(k, schema)
 	case TypeSet:
diff --git a/helper/schema/field_writer_map.go b/helper/schema/field_writer_map.go
@@ -100,22 +100,21 @@ func (w *MapFieldWriter) set(addr []string, value interface{}) error {
 	case TypeBool, TypeInt, TypeFloat, TypeString:
 		return w.setPrimitive(addr, value, schema)
 	case TypeList:
-		return w.setList(addr, value, schema)
+		return w.setList(addr, value)
 	case TypeMap:
-		return w.setMap(addr, value, schema)
+		return w.setMap(addr, value)
 	case TypeSet:
 		return w.setSet(addr, value, schema)
 	case typeObject:
-		return w.setObject(addr, value, schema)
+		return w.setObject(addr, value)
 	default:
 		panic(fmt.Sprintf("Unknown type: %#v", schema.Type))
 	}
 }
 
 func (w *MapFieldWriter) setList(
 	addr []string,
-	v interface{},
-	schema *Schema) error {
+	v interface{}) error {
 	k := strings.Join(addr, ".")
 	setElement := func(idx string, value interface{}) error {
 		addrCopy := make([]string, len(addr), len(addr)+1)
@@ -160,8 +159,7 @@ func (w *MapFieldWriter) setList(
 
 func (w *MapFieldWriter) setMap(
 	addr []string,
-	value interface{},
-	schema *Schema) error {
+	value interface{}) error {
 	k := strings.Join(addr, ".")
 	v := reflect.ValueOf(value)
 	vs := make(map[string]interface{})
@@ -207,8 +205,7 @@ func (w *MapFieldWriter) setMap(
 
 func (w *MapFieldWriter) setObject(
 	addr []string,
-	value interface{},
-	schema *Schema) error {
+	value interface{}) error {
 	// Set the entire object. First decode into a proper structure
 	var v map[string]interface{}
 	if err := mapstructure.Decode(value, &v); err != nil {
diff --git a/helper/schema/grpc_provider.go b/helper/schema/grpc_provider.go
@@ -355,7 +355,7 @@ func (s *GRPCProviderServer) UpgradeResourceState(ctx context.Context, req *tfpr
 // map[string]interface{}.
 // upgradeFlatmapState returns the json map along with the corresponding schema
 // version.
-func (s *GRPCProviderServer) upgradeFlatmapState(ctx context.Context, version int, m map[string]string, res *Resource) (map[string]interface{}, int, error) {
+func (s *GRPCProviderServer) upgradeFlatmapState(_ context.Context, version int, m map[string]string, res *Resource) (map[string]interface{}, int, error) {
 	// this will be the version we've upgraded so, defaulting to the given
 	// version in case no migration was called.
 	upgradedVersion := version
diff --git a/helper/schema/resource.go b/helper/schema/resource.go
@@ -737,8 +737,8 @@ func (r *Resource) InternalValidate(topSchemaMap schemaMap, writable bool) error
 			}
 		}
 
-		for k, f := range tsm {
-			if isReservedResourceFieldName(k, f) {
+		for k := range tsm {
+			if isReservedResourceFieldName(k) {
 				return fmt.Errorf("%s is a reserved field name", k)
 			}
 		}
@@ -850,7 +850,7 @@ func validateResourceID(s *Schema) error {
 	return nil
 }
 
-func isReservedResourceFieldName(name string, s *Schema) bool {
+func isReservedResourceFieldName(name string) bool {
 	for _, reservedName := range ReservedResourceFields {
 		if name == reservedName {
 			return true
diff --git a/helper/schema/resource_diff_test.go b/helper/schema/resource_diff_test.go
@@ -38,7 +38,7 @@ type resourceDiffTestCase struct {
 }
 
 // testDiffCases produces a list of test cases for use with SetNew and SetDiff.
-func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool) []resourceDiffTestCase {
+func testDiffCases(t *testing.T, computed bool) []resourceDiffTestCase {
 	return []resourceDiffTestCase{
 		{
 			Name: "basic primitive diff",
@@ -634,7 +634,7 @@ func testDiffCases(t *testing.T, oldPrefix string, oldOffset int, computed bool)
 }
 
 func TestSetNew(t *testing.T) {
-	testCases := testDiffCases(t, "", 0, false)
+	testCases := testDiffCases(t, false)
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
 			m := schemaMap(tc.Schema)
@@ -661,7 +661,7 @@ func TestSetNew(t *testing.T) {
 }
 
 func TestSetNewComputed(t *testing.T) {
-	testCases := testDiffCases(t, "", 0, true)
+	testCases := testDiffCases(t, true)
 	for _, tc := range testCases {
 		t.Run(tc.Name, func(t *testing.T) {
 			m := schemaMap(tc.Schema)
diff --git a/helper/schema/resource_test.go b/helper/schema/resource_test.go
@@ -554,7 +554,7 @@ func TestResourceApply_isNewResource(t *testing.T) {
 		},
 	}
 
-	updateFunc := func(d *ResourceData, m interface{}) error {
+	updateFunc := func(d *ResourceData, _ interface{}) error {
 		if err := d.Set("foo", "updated"); err != nil {
 			return fmt.Errorf("unexpected Set error: %s", err)
 		}
diff --git a/terraform/state.go b/terraform/state.go
@@ -366,11 +366,11 @@ func (s *State) Remove(addr ...string) error {
 
 		switch v := r.Value.(type) {
 		case *ModuleState:
-			s.removeModule(path, v)
+			s.removeModule(v)
 		case *ResourceState:
 			s.removeResource(path, v)
 		case *InstanceState:
-			s.removeInstance(path, r.Parent.Value.(*ResourceState), v)
+			s.removeInstance(r.Parent.Value.(*ResourceState), v)
 		default:
 			return fmt.Errorf("unknown type to delete: %T", r.Value)
 		}
@@ -384,7 +384,7 @@ func (s *State) Remove(addr ...string) error {
 	return nil
 }
 
-func (s *State) removeModule(path []string, v *ModuleState) {
+func (s *State) removeModule(v *ModuleState) {
 	for i, m := range s.Modules {
 		if m == v {
 			s.Modules, s.Modules[len(s.Modules)-1] = append(s.Modules[:i], s.Modules[i+1:]...), nil
@@ -412,7 +412,7 @@ func (s *State) removeResource(path []string, v *ResourceState) {
 	}
 }
 
-func (s *State) removeInstance(path []string, r *ResourceState, v *InstanceState) {
+func (s *State) removeInstance(r *ResourceState, v *InstanceState) {
 	// Go through the resource and find the instance that matches this
 	// (if any) and remove it.
 

Original file line number	Diff line number	Diff line change
`@@ -546,7 +546,7 @@ func Test(t testing.T, c TestCase) {`
`546`	`546`	`t.Fatalf("ProviderFactory for %q already exists, cannot overwrite with Provider", name)`
`547`	`547`	`}`
`548`	`548`	`prov := p`
`549`		`- c.ProviderFactories[name] = func() (*schema.Provider, error) {`
	`549`	`+ c.ProviderFactories[name] = func() (*schema.Provider, error) { //nolint:unparam // required signature`
`550`	`550`	`return prov, nil`
`551`	`551`	`}`
`552`	`552`	`}`
`@@ -1004,7 +1004,7 @@ func TestMatchOutput(name string, r *regexp.Regexp) TestCheckFunc {`
`1004`	`1004`
`1005`	`1005`	`// modulePrimaryInstanceState returns the instance state for the given resource`
`1006`	`1006`	`// name in a ModuleState`
`1007`		`-func modulePrimaryInstanceState(s terraform.State, ms terraform.ModuleState, name string) (*terraform.InstanceState, error) {`
	`1007`	`+func modulePrimaryInstanceState(ms terraform.ModuleState, name string) (terraform.InstanceState, error) {`
`1008`	`1008`	`rs, ok := ms.Resources[name]`
`1009`	`1009`	`if !ok {`
`1010`	`1010`	`return nil, fmt.Errorf("Not found: %s in %s", name, ms.Path)`
`@@ -1026,14 +1026,14 @@ func modulePathPrimaryInstanceState(s *terraform.State, mp addrs.ModuleInstance,`
`1026`	`1026`	`return nil, fmt.Errorf("No module found at: %s", mp)`
`1027`	`1027`	`}`
`1028`	`1028`
`1029`		`- return modulePrimaryInstanceState(s, ms, name)`
	`1029`	`+ return modulePrimaryInstanceState(ms, name)`
`1030`	`1030`	`}`
`1031`	`1031`
`1032`	`1032`	`// primaryInstanceState returns the primary instance state for the given`
`1033`	`1033`	`// resource name in the root module.`
`1034`	`1034`	`func primaryInstanceState(s terraform.State, name string) (terraform.InstanceState, error) {`
`1035`	`1035`	`ms := s.RootModule()`
`1036`		`- return modulePrimaryInstanceState(s, ms, name)`
	`1036`	`+ return modulePrimaryInstanceState(ms, name)`
`1037`	`1037`	`}`
`1038`	`1038`
`1039`	`1039`	`// indexesIntoTypeSet is a heuristic to try and identify if a flatmap style`
Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ func TestParallelTest(t *testing.T) {`
`36`	`36`	`func TestTest_factoryError(t *testing.T) {`
`37`	`37`	`resourceFactoryError := fmt.Errorf("resource factory error")`
`38`	`38`
`39`		`- factory := func() (*schema.Provider, error) {`
	`39`	`+ factory := func() (*schema.Provider, error) { //nolint:unparam // required signature`
`40`	`40`	`return nil, resourceFactoryError`
`41`	`41`	`}`
`42`	`42`	`mt := new(mockT)`
Original file line number	Diff line number	Diff line change
`@@ -536,5 +536,6 @@ func TestConfigFieldReader_computedComplexSet(t *testing.T) {`
`536`	`536`	`}`
`537`	`537`
`538`	`538`	`func testConfig(t testing.T, raw map[string]interface{}) terraform.ResourceConfig {`
	`539`	`+ t.Helper()`
`539`	`540`	`return terraform.NewResourceConfigRaw(raw)`
`540`	`541`	`}`