Skip to content

Commit 9bdb216

Browse files
authored
Enable predeclared linter and fix issues (#872)
Reference: #865 Shadowing language keywords can cause confusing programming issues and may not provide enough clarity on the purpose of a variable. Previously: ```text helper/acctest/random.go:123:5: variable len has same name as predeclared identifier (predeclared) if len := r.BitLen(); len > 31 { ^ helper/customdiff/condition.go:15:62: param new has same name as predeclared identifier (predeclared) type ValueChangeConditionFunc func(ctx context.Context, old, new, meta interface{}) bool ^ helper/customdiff/condition.go:44:8: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange(key) ^ helper/customdiff/force_new.go:39:8: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange(key) ^ helper/customdiff/validate.go:12:63: param new has same name as predeclared identifier (predeclared) type ValueChangeValidationFunc func(ctx context.Context, old, new, meta interface{}) error ^ helper/customdiff/validate.go:22:8: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange(key) ^ helper/customdiff/computed_test.go:34:10: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/computed_test.go:89:10: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/condition_test.go:26:11: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/condition_test.go:84:11: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/condition_test.go:141:34: param new has same name as predeclared identifier (predeclared) func(_ context.Context, old, new, meta interface{}) bool { ^ helper/customdiff/condition_test.go:199:34: param new has same name as predeclared identifier (predeclared) func(_ context.Context, old, new, meta interface{}) bool { ^ helper/customdiff/force_new_test.go:30:10: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/force_new_test.go:93:10: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange("foo") ^ helper/customdiff/force_new_test.go:144:57: param new has same name as predeclared identifier (predeclared) ForceNewIfChange("foo", func(_ context.Context, old, new, meta interface{}) bool { ^ helper/customdiff/force_new_test.go:212:57: param new has same name as predeclared identifier (predeclared) ForceNewIfChange("foo", func(_ context.Context, old, new, meta interface{}) bool { ^ helper/customdiff/testing_test.go:21:47: param new has same name as predeclared identifier (predeclared) func testDiff(provider *schema.Provider, old, new map[string]string) (*terraform.InstanceDiff, error) { ^ helper/customdiff/validate_test.go:22:54: param new has same name as predeclared identifier (predeclared) ValidateChange("foo", func(_ context.Context, old, new, meta interface{}) error { ^ helper/resource/testing_new_import_state.go:127:3: variable new has same name as predeclared identifier (predeclared) new := importState.RootModule().Resources ^ helper/schema/resource_diff.go:273:7: variable new has same name as predeclared identifier (predeclared) old, new, customized := d.getChange(key) ^ helper/schema/resource_diff.go:311:44: param new has same name as predeclared identifier (predeclared) func (d *ResourceDiff) setDiff(key string, new interface{}, computed bool) error { ^ helper/schema/resource_diff.go:377:7: variable new has same name as predeclared identifier (predeclared) old, new, _ := d.getChange(key) ^ helper/schema/resource_diff.go:438:7: variable new has same name as predeclared identifier (predeclared) old, new := d.GetChange(key) ^ helper/schema/resource_diff.go:520:6: variable new has same name as predeclared identifier (predeclared) var new getResult ^ helper/schema/schema.go:267:42: param new has same name as predeclared identifier (predeclared) type SchemaDiffSuppressFunc func(k, old, new string, d *ResourceData) bool ^ helper/schema/schema.go:494:2: variable copy has same name as predeclared identifier (predeclared) copy, err := copystructure.Config{Lock: true}.Copy(m) ^ helper/schema/resource_timeout_test.go:356:52: param delete has same name as predeclared identifier (predeclared) func expectedConfigForValues(create, read, update, delete, def int) map[string]interface{} { ^ helper/schema/schema_test.go:4796:37: param new has same name as predeclared identifier (predeclared) DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool { return false }, ^ helper/schema/schema_test.go:5062:37: param new has same name as predeclared identifier (predeclared) DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool { ^ helper/schema/schema_test.go:5085:37: param new has same name as predeclared identifier (predeclared) DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool { ^ helper/schema/schema_test.go:5116:37: param new has same name as predeclared identifier (predeclared) DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool { ^ helper/schema/schema_test.go:5137:37: param new has same name as predeclared identifier (predeclared) DiffSuppressFunc: func(k, old, new string, d *ResourceData) bool { ^ helper/structure/suppress_json_diff.go:9:31: param new has same name as predeclared identifier (predeclared) func SuppressJsonDiff(k, old, new string, d *schema.ResourceData) bool { ^ helper/structure/suppress_json_diff_test.go:9:2: variable new has same name as predeclared identifier (predeclared) new := `{ "enabled": true }` ^ helper/structure/suppress_json_diff_test.go:22:2: variable new has same name as predeclared identifier (predeclared) new := `{ "enabled": true }` ^ helper/structure/suppress_json_diff_test.go:33:2: variable new has same name as predeclared identifier (predeclared) new := `{ "enabled": false }` ^ helper/structure/suppress_json_diff_test.go:44:2: variable new has same name as predeclared identifier (predeclared) new := `{ "enabled": false, "world": "round" }` ^ helper/validation/map.go:20:4: variable len has same name as predeclared identifier (predeclared) len := len(key) ^ helper/validation/map.go:56:4: variable len has same name as predeclared identifier (predeclared) len := len(val.(string)) ^ internal/configs/hcl2shim/flatmap.go:88:2: variable len has same name as predeclared identifier (predeclared) len := 0 ^ terraform/resource.go:157:2: variable copy has same name as predeclared identifier (predeclared) copy, err := copystructure.Config{Lock: true}.Copy(c) ^ terraform/state.go:565:2: variable copy has same name as predeclared identifier (predeclared) copy, err := copystructure.Config{Lock: true}.Copy(s) ^ terraform/state.go:1433:2: variable copy has same name as predeclared identifier (predeclared) copy, err := copystructure.Config{Lock: true}.Copy(s) ^ terraform/resource_test.go:200:4: variable copy has same name as predeclared identifier (predeclared) copy := rc.DeepCopy() ^ ```
1 parent 5c039b1 commit 9bdb216

22 files changed

+157
-156
lines changed

.golangci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ linters:
1919
# - makezero # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
2020
- nilerr
2121
# - paralleltest # Reference: https://github.com/kunwardeep/paralleltest/issues/14
22-
# - predeclared # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
22+
- predeclared
2323
# - staticcheck # TODO: https://github.com/hashicorp/terraform-plugin-sdk/issues/865
2424
# - tenv # TODO: Enable when upgrading Go 1.16 to 1.17
2525
- unconvert

helper/acctest/random.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ func RandIpAddress(s string) (string, error) {
120120
last.SetBytes([]byte(lastIp))
121121
r := &big.Int{}
122122
r.Sub(last, first)
123-
if len := r.BitLen(); len > 31 {
124-
return "", fmt.Errorf("CIDR range is too large: %d", len)
123+
if bitLen := r.BitLen(); bitLen > 31 {
124+
return "", fmt.Errorf("CIDR range is too large: %d", bitLen)
125125
}
126126

127127
max := int(r.Int64())

helper/customdiff/computed_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,9 @@ func TestComputedIf(t *testing.T) {
3131
// updated.
3232

3333
condCalls++
34-
old, new := d.GetChange("foo")
35-
gotOld = old.(string)
36-
gotNew = new.(string)
34+
oldValue, newValue := d.GetChange("foo")
35+
gotOld = oldValue.(string)
36+
gotNew = newValue.(string)
3737

3838
return true
3939
}),
@@ -86,9 +86,9 @@ func TestComputedIf(t *testing.T) {
8686
},
8787
ComputedIf("comp", func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
8888
condCalls++
89-
old, new := d.GetChange("foo")
90-
gotOld = old.(string)
91-
gotNew = new.(string)
89+
oldValue, newValue := d.GetChange("foo")
90+
gotOld = oldValue.(string)
91+
gotNew = newValue.(string)
9292

9393
return false
9494
}),

helper/customdiff/condition.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type ResourceConditionFunc func(ctx context.Context, d *schema.ResourceDiff, met
1212

1313
// ValueChangeConditionFunc is a function type that makes a boolean decision
1414
// by comparing two values.
15-
type ValueChangeConditionFunc func(ctx context.Context, old, new, meta interface{}) bool
15+
type ValueChangeConditionFunc func(ctx context.Context, oldValue, newValue, meta interface{}) bool
1616

1717
// ValueConditionFunc is a function type that makes a boolean decision based
1818
// on a given value.
@@ -41,8 +41,8 @@ func If(cond ResourceConditionFunc, f schema.CustomizeDiffFunc) schema.Customize
4141
// given CustomizeDiffFunc only if the condition function returns true.
4242
func IfValueChange(key string, cond ValueChangeConditionFunc, f schema.CustomizeDiffFunc) schema.CustomizeDiffFunc {
4343
return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
44-
old, new := d.GetChange(key)
45-
if cond(ctx, old, new, meta) {
44+
oldValue, newValue := d.GetChange(key)
45+
if cond(ctx, oldValue, newValue, meta) {
4646
return f(ctx, d, meta)
4747
}
4848
return nil

helper/customdiff/condition_test.go

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ func TestIf(t *testing.T) {
2323
If(
2424
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
2525
condCalled = true
26-
old, new := d.GetChange("foo")
27-
gotOld = old.(string)
28-
gotNew = new.(string)
26+
oldValue, newValue := d.GetChange("foo")
27+
gotOld = oldValue.(string)
28+
gotNew = newValue.(string)
2929
return true
3030
},
3131
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
@@ -81,9 +81,9 @@ func TestIf(t *testing.T) {
8181
If(
8282
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) bool {
8383
condCalled = true
84-
old, new := d.GetChange("foo")
85-
gotOld = old.(string)
86-
gotNew = new.(string)
84+
oldValue, newValue := d.GetChange("foo")
85+
gotOld = oldValue.(string)
86+
gotNew = newValue.(string)
8787
return false
8888
},
8989
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
@@ -138,10 +138,10 @@ func TestIfValueChange(t *testing.T) {
138138
},
139139
IfValueChange(
140140
"foo",
141-
func(_ context.Context, old, new, meta interface{}) bool {
141+
func(_ context.Context, oldValue, newValue, meta interface{}) bool {
142142
condCalled = true
143-
gotOld = old.(string)
144-
gotNew = new.(string)
143+
gotOld = oldValue.(string)
144+
gotNew = newValue.(string)
145145
return true
146146
},
147147
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {
@@ -196,10 +196,10 @@ func TestIfValueChange(t *testing.T) {
196196
},
197197
IfValueChange(
198198
"foo",
199-
func(_ context.Context, old, new, meta interface{}) bool {
199+
func(_ context.Context, oldValue, newValue, meta interface{}) bool {
200200
condCalled = true
201-
gotOld = old.(string)
202-
gotNew = new.(string)
201+
gotOld = oldValue.(string)
202+
gotNew = newValue.(string)
203203
return false
204204
},
205205
func(_ context.Context, d *schema.ResourceDiff, meta interface{}) error {

helper/customdiff/force_new.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ func ForceNewIf(key string, f ResourceConditionFunc) schema.CustomizeDiffFunc {
3636
// only the specific field value.
3737
func ForceNewIfChange(key string, f ValueChangeConditionFunc) schema.CustomizeDiffFunc {
3838
return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
39-
old, new := d.GetChange(key)
40-
if f(ctx, old, new, meta) {
39+
oldValue, newValue := d.GetChange(key)
40+
if f(ctx, oldValue, newValue, meta) {
4141
if err := d.ForceNew(key); err != nil {
4242
return fmt.Errorf("unable to set %q to require replacement: %w", key, err)
4343
}

helper/customdiff/force_new_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ func TestForceNewIf(t *testing.T) {
2727
// updated.
2828

2929
condCalls++
30-
old, new := d.GetChange("foo")
30+
oldValue, newValue := d.GetChange("foo")
3131

3232
switch condCalls {
3333
case 1:
34-
gotOld1 = old.(string)
35-
gotNew1 = new.(string)
34+
gotOld1 = oldValue.(string)
35+
gotNew1 = newValue.(string)
3636
case 2:
37-
gotOld2 = old.(string)
38-
gotNew2 = new.(string)
37+
gotOld2 = oldValue.(string)
38+
gotNew2 = newValue.(string)
3939
}
4040

4141
return true
@@ -90,9 +90,9 @@ func TestForceNewIf(t *testing.T) {
9090
},
9191
ForceNewIf("foo", func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) bool {
9292
condCalls++
93-
old, new := d.GetChange("foo")
94-
gotOld = old.(string)
95-
gotNew = new.(string)
93+
oldValue, newValue := d.GetChange("foo")
94+
gotOld = oldValue.(string)
95+
gotNew = newValue.(string)
9696

9797
return false
9898
}),
@@ -141,7 +141,7 @@ func TestForceNewIfChange(t *testing.T) {
141141
Optional: true,
142142
},
143143
},
144-
ForceNewIfChange("foo", func(_ context.Context, old, new, meta interface{}) bool {
144+
ForceNewIfChange("foo", func(_ context.Context, oldValue, newValue, meta interface{}) bool {
145145
// When we set "ForceNew", our CustomizeDiff function is actually
146146
// called a second time to construct the "create" portion of
147147
// the replace diff. On the second call, the old value is masked
@@ -152,11 +152,11 @@ func TestForceNewIfChange(t *testing.T) {
152152

153153
switch condCalls {
154154
case 1:
155-
gotOld1 = old.(string)
156-
gotNew1 = new.(string)
155+
gotOld1 = oldValue.(string)
156+
gotNew1 = newValue.(string)
157157
case 2:
158-
gotOld2 = old.(string)
159-
gotNew2 = new.(string)
158+
gotOld2 = oldValue.(string)
159+
gotNew2 = newValue.(string)
160160
}
161161

162162
return true
@@ -209,10 +209,10 @@ func TestForceNewIfChange(t *testing.T) {
209209
Optional: true,
210210
},
211211
},
212-
ForceNewIfChange("foo", func(_ context.Context, old, new, meta interface{}) bool {
212+
ForceNewIfChange("foo", func(_ context.Context, oldValue, newValue, meta interface{}) bool {
213213
condCalls++
214-
gotOld = old.(string)
215-
gotNew = new.(string)
214+
gotOld = oldValue.(string)
215+
gotNew = newValue.(string)
216216

217217
return false
218218
}),

helper/customdiff/testing_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,16 @@ func testProvider(s map[string]*schema.Schema, cd schema.CustomizeDiffFunc) *sch
1818
}
1919
}
2020

21-
func testDiff(provider *schema.Provider, old, new map[string]string) (*terraform.InstanceDiff, error) {
22-
newI := make(map[string]interface{}, len(new))
23-
for k, v := range new {
21+
func testDiff(provider *schema.Provider, oldValue, newValue map[string]string) (*terraform.InstanceDiff, error) {
22+
newI := make(map[string]interface{}, len(newValue))
23+
for k, v := range newValue {
2424
newI[k] = v
2525
}
2626

2727
return provider.ResourcesMap["test"].Diff(
2828
context.Background(),
2929
&terraform.InstanceState{
30-
Attributes: old,
30+
Attributes: oldValue,
3131
},
3232
&terraform.ResourceConfig{
3333
Config: newI,

helper/customdiff/validate.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
// ValueChangeValidationFunc is a function type that validates the difference
1010
// (or lack thereof) between two values, returning an error if the change
1111
// is invalid.
12-
type ValueChangeValidationFunc func(ctx context.Context, old, new, meta interface{}) error
12+
type ValueChangeValidationFunc func(ctx context.Context, oldValue, newValue, meta interface{}) error
1313

1414
// ValueValidationFunc is a function type that validates a particular value,
1515
// returning an error if the value is invalid.
@@ -19,8 +19,8 @@ type ValueValidationFunc func(ctx context.Context, value, meta interface{}) erro
1919
// function to the change for the given key, returning any error produced.
2020
func ValidateChange(key string, f ValueChangeValidationFunc) schema.CustomizeDiffFunc {
2121
return func(ctx context.Context, d *schema.ResourceDiff, meta interface{}) error {
22-
old, new := d.GetChange(key)
23-
return f(ctx, old, new, meta)
22+
oldValue, newValue := d.GetChange(key)
23+
return f(ctx, oldValue, newValue, meta)
2424
}
2525
}
2626

helper/customdiff/validate_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,10 @@ func TestValidateChange(t *testing.T) {
1919
Optional: true,
2020
},
2121
},
22-
ValidateChange("foo", func(_ context.Context, old, new, meta interface{}) error {
22+
ValidateChange("foo", func(_ context.Context, oldValue, newValue, meta interface{}) error {
2323
called = true
24-
gotOld = old.(string)
25-
gotNew = new.(string)
24+
gotOld = oldValue.(string)
25+
gotNew = newValue.(string)
2626
return errors.New("bad")
2727
}),
2828
)

0 commit comments

Comments
 (0)