Enable the 'scopelint' and 'unconvert' linters (#1299)

'scopelint' checks for unpinned variables in 'range'-loops. This helps to avoid subtle bugs. 'unconvert' checks for unnecessary type conversions.
kudobuilder · Jan 23, 2020 · abb1219 · abb1219
1 parent 423acdb
commit abb1219
Show file tree

Hide file tree

Showing 16 changed files with 49 additions and 1 deletion.
diff --git a/.golangci.yml b/.golangci.yml
@@ -6,6 +6,8 @@ linters:
     - golint
     - gosec
     - misspell
+    - scopelint
+    - unconvert
     - unparam
 run:
   skip-dirs:

diff --git a/pkg/engine/renderer/enhancer.go b/pkg/engine/renderer/enhancer.go
@@ -31,7 +31,7 @@ func (k *DefaultEnhancer) Apply(templates map[string]string, metadata Metadata)
 	objs := make([]runtime.Object, 0, len(templates))
 
 	for _, v := range templates {
-		parsed, err := YamlToObject(string(v))
+		parsed, err := YamlToObject(v)
 		if err != nil {
 			return nil, err
 		}

diff --git a/pkg/engine/task/task_pipe_test.go b/pkg/engine/task/task_pipe_test.go
@@ -68,6 +68,8 @@ func Test_isRelative(t *testing.T) {
 	}
 
 	for i, test := range tests {
+		test := test
+
 		t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
 			if test.relative != isRelative(test.base, test.file) {
 				t.Errorf("unexpected result for: base %q, file %q", test.base, test.file)
@@ -112,6 +114,8 @@ func TestPipeNames(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			if got := PipePodName(tt.meta); got != tt.wantPodName {
 				t.Errorf("PipePodName() = %v, want %v", got, tt.wantPodName)
@@ -348,6 +352,8 @@ spec:
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			pod, err := unmarshal(tt.podYaml)
 			assert.NoError(t, err, "error during pipe pod unmarshaling")
@@ -423,6 +429,8 @@ func Test_pipeFiles(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			fs := afero.NewMemMapFs()
 

diff --git a/pkg/engine/task/task_test.go b/pkg/engine/task/task_test.go
@@ -162,6 +162,8 @@ spec:
 	}
 
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			task := &v1beta1.Task{}
 			err := yaml.Unmarshal([]byte(tt.taskYaml), task)

diff --git a/pkg/kudoctl/cmd/init_test.go b/pkg/kudoctl/cmd/init_test.go
@@ -178,6 +178,8 @@ func TestNewInitCmd(t *testing.T) {
 	}
 
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			out := &bytes.Buffer{}
 			initCmd := newInitCmd(fs, out)

diff --git a/pkg/kudoctl/cmd/repo_index_test.go b/pkg/kudoctl/cmd/repo_index_test.go
@@ -29,6 +29,8 @@ func TestRepoIndexCmd(t *testing.T) {
 	}
 
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			out := &bytes.Buffer{}
 			time := time.Now()

diff --git a/pkg/kudoctl/env/environment_test.go b/pkg/kudoctl/env/environment_test.go
@@ -58,6 +58,8 @@ func TestEnvSettings(t *testing.T) {
 	}
 
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			for k, v := range tt.envars {
 				if err := os.Setenv(k, v); err != nil {

diff --git a/pkg/kudoctl/http/http_client_test.go b/pkg/kudoctl/http/http_client_test.go
@@ -17,6 +17,8 @@ func TestIsValidURL(t *testing.T) {
 		{name: "no http prefix", uri: "kudo.dev", want: false},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			if got := IsValidURL(tt.uri); got != tt.want {
 				t.Errorf("IsValidURL() = %v, want %v", got, tt.want)

diff --git a/pkg/kudoctl/kudoinit/setup/wait.go b/pkg/kudoctl/kudoinit/setup/wait.go
@@ -59,6 +59,8 @@ func getFirstRunningPod(client corev1.PodsGetter, namespace string, selector lab
 		return nil, errors.New("could not find KUDO manager")
 	}
 	for _, p := range pods.Items {
+		p := p
+
 		if health.IsHealthy(&p) == nil {
 			return &p, nil
 		}

diff --git a/pkg/kudoctl/packages/reader/reader_test.go b/pkg/kudoctl/packages/reader/reader_test.go
@@ -30,6 +30,8 @@ func TestReadFileSystemPackage(t *testing.T) {
 	var fs = afero.NewOsFs()
 
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(fmt.Sprintf("%s-from-%s", tt.name, tt.path), func(t *testing.T) {
 			var err error
 			var pkg *packages.Package
@@ -142,6 +144,8 @@ parameters:
 		{"parameters", oneParam, packages.ParamsFile{APIVersion: APIVersion, Parameters: example}, false},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := readParametersFile([]byte(tt.paramsYaml))
 			fmt.Printf("%s got: %v\n", tt.name, got)

diff --git a/pkg/test/case_test.go b/pkg/test/case_test.go
@@ -222,6 +222,8 @@ func TestLoadTestSteps(t *testing.T) {
 			},
 		},
 	} {
+		tt := tt
+
 		t.Run(tt.path, func(t *testing.T) {
 			test := &Case{Dir: tt.path, Logger: testutils.NewTestLogger(t, tt.path)}
 
@@ -285,6 +287,8 @@ func TestCollectTestStepFiles(t *testing.T) {
 			},
 		},
 	} {
+		tt := tt
+
 		t.Run(tt.path, func(t *testing.T) {
 			test := &Case{Dir: tt.path, Logger: testutils.NewTestLogger(t, tt.path)}
 			testStepFiles, err := test.CollectTestStepFiles()

diff --git a/pkg/test/harness.go b/pkg/test/harness.go
@@ -356,6 +356,8 @@ func (h *Harness) RunTests() {
 
 	h.T.Run("harness", func(t *testing.T) {
 		for _, test := range tests {
+			test := test
+
 			test.Client = h.Client
 			test.DiscoveryClient = h.DiscoveryClient
 

diff --git a/pkg/test/step.go b/pkg/test/step.go
@@ -260,6 +260,8 @@ func (s *Step) CheckResource(expected runtime.Object, namespace string) []error
 	}
 
 	for _, actual := range actuals {
+		actual := actual
+
 		tmpTestErrors := []error{}
 
 		if err := testutils.IsSubset(expectedObj, actual.UnstructuredContent()); err != nil {

diff --git a/pkg/test/step_test.go b/pkg/test/step_test.go
@@ -166,6 +166,8 @@ func TestCheckResource(t *testing.T) {
 			shouldError: true,
 		},
 	} {
+		test := test
+
 		t.Run(test.testName, func(t *testing.T) {
 			fakeDiscovery := testutils.FakeDiscoveryClient()
 			namespace := "world"
@@ -214,6 +216,8 @@ func TestCheckResourceAbsent(t *testing.T) {
 			expected: testutils.NewPod("hello", ""),
 		},
 	} {
+		test := test
+
 		t.Run(test.name, func(t *testing.T) {
 			fakeDiscovery := testutils.FakeDiscoveryClient()
 			namespace := "world"
@@ -285,6 +289,8 @@ func TestRun(t *testing.T) {
 			},
 		},
 	} {
+		test := test
+
 		t.Run(test.testName, func(t *testing.T) {
 			test.Step.Assert = &harness.TestAssert{
 				Timeout: 1,

diff --git a/pkg/test/utils/kubernetes_test.go b/pkg/test/utils/kubernetes_test.go
@@ -46,6 +46,8 @@ func TestNamespaced(t *testing.T) {
 			shouldError: true,
 		},
 	} {
+		test := test
+
 		t.Run(test.testName, func(t *testing.T) {
 			m, _ := meta.Accessor(test.resource)
 
@@ -403,6 +405,8 @@ func TestGetKubectlArgs(t *testing.T) {
 			},
 		},
 	} {
+		test := test
+
 		t.Run(test.testName, func(t *testing.T) {
 			cmd, err := GetArgs(context.TODO(), "kubectl", harness.Command{
 				Command:    test.args,

diff --git a/pkg/version/version_test.go b/pkg/version/version_test.go
@@ -20,6 +20,8 @@ func Test_validVersion(t *testing.T) {
 		{"full semver is not a factor", MustParse("1.5.8"), MustParse("1.5.0"), 0},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			val := tt.expected.CompareMajorMinor(tt.actual)
 			assert.Equal(t, val, tt.val)
@@ -38,6 +40,8 @@ func TestClean(t *testing.T) {
 		{"short ver", "v1.0", "1.0"},
 	}
 	for _, tt := range tests {
+		tt := tt
+
 		t.Run(tt.name, func(t *testing.T) {
 			result := Clean(tt.actual)
 			assert.Equal(t, tt.expected, result)