Skip to content
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.

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

[19.03 backport] Replace gometalinter with Golangci lint [carry 1797] #2239

Merged

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jan 6, 2020

backport of:

Minor conflict in 75c60c1 because #2105 is not in 19.03

conflict in 6047259

both modified:   cli/command/service/client_test.go
both modified:   cli/command/stack/services_test.go

conflict in 542f802

both modified:   cli/command/container/create_test.go

skipped 1e77742

thaJeztah and others added 30 commits January 6, 2020 13:12
The configuration abused "Exclude" to exclude file-paths by filtering
on the output, however, the `Skip` option was designed for that, whereas
`Exclude` is for matching warnings.

An explicit "Skip" was added for "vendor", because even though the vendor
directory should already be ignored by the linter, in some situations,
it still seemed to warn on issues, so let's explicitly ignore it.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 71e525f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Looks like we're just on the edge of the deadline, and it's sometimes
failing;

```
cli/command/image/trust.go:346:1:warning: nolint directive did not match any issue (nolint)
cli/command/manifest/push.go:211:1:warning: nolint directive did not match any issue (nolint)
internal/pkg/containerized/snapshot.go:95:1:warning: nolint directive did not match any issue (nolint)
internal/pkg/containerized/snapshot.go:138:1:warning: nolint directive did not match any issue (nolint)
WARNING: deadline exceeded by linter interfacer (try increasing --deadline)
Exited with code 3
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 3e78cbc)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This test was intending to run all tests, but didn't, which was
caught by golangci-lint;

    cli/compose/loader/windows_path_test.go:46:17: SA4010: this result of append is never used, except maybe in other appends (staticcheck)
    	tests := append(isabstests, winisabstests...)
    	               ^

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 0a21de0)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…taticcheck)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2962971)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…of append is never used, except maybe in other appends (staticcheck)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 8018a85)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…the underlying ticker, consider using it only in endless functions, tests and the main package, and use time.NewTicker here (staticcheck)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 7da9360)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…check)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 3a42820)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…n: nil != nil (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit f5e8387)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…dition: non-nil != nil (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 85cfd4e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…pe assertion (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 9afeb6f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…nil != nil (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 5ceed30)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…,inline", json:"-"` not compatible with reflect.StructTag.Get: key:"value" pairs not separated by spaces (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 1bfe813)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…/github.com/docker/go-units.Ulimit` composite literal uses unkeyed fields (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit b3d4c6a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…: nil != nil (govet)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 584da37)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit e1c0c79)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… `dir` always receives `""` (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 70bd64d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…- `perm` always receives `0777` (`511`) (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 0ce2eae)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…r) is always nil (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 28ac2f8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… 1 (error) is always nil (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 75c60c1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…sed (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit ab1aeed)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ces` - `stackName` always receives `"test"` (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit a3c7cb4)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…error) is always nil (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 47741f8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
cli/compose/convert/service.go:592:76: convertDNSConfig - result 1 (error) is always nil (unparam)
cli/compose/convert/service.go:538:110: convertEndpointSpec - result 1 (error) is always nil (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit d640f44)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…se function signatures

cli/compose/loader/loader.go:756:66: transformServiceNetworkMap - result 1 (error) is always nil (unparam)
cli/compose/loader/loader.go:767:67: transformStringOrNumberList - result 1 (error) is always nil (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 6eb0c9c)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Also explicitly type transformer-functions

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 9118b2b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…always receives `"argument"` (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 7d82343)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…sphrase` - `pwd` always receives `"foo"` (unparam)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit f123e43)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ert)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 0153624)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…nconvert)

Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit c237379)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
(cherry picked from commit 6047259)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah and others added 20 commits January 6, 2020 13:16
…checked (errcheck)

```
cli/command/formatter/container_test.go:315:17: Error return value of `ContainerWrite` is not checked (errcheck)
		ContainerWrite(context.context, containers)
		              ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit e74e2c7)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…l (scopelint)

```
cli/config/config_test.go:590:11: Using the variable on range scope `tc` in function literal (scopelint)
			SetDir(tc.dir)
			       ^
cli/config/config_test.go:591:19: Using the variable on range scope `tc` in function literal (scopelint)
			f, err := Path(tc.path...)
			               ^
cli/config/config_test.go:592:23: Using the variable on range scope `tc` in function literal (scopelint)
			assert.Equal(t, f, tc.expected)
			                   ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 5a2a9d9)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…n literal (scopelint)

```
service/logs/parse_logs_test.go:26:35: Using the variable on range scope `testcase` in function literal (scopelint)
			actual, err := ParseLogDetails(testcase.line)
			                               ^
service/logs/parse_logs_test.go:27:7: Using the variable on range scope `testcase` in function literal (scopelint)
			if testcase.err != nil {
			   ^
service/logs/parse_logs_test.go:28:26: Using the variable on range scope `testcase` in function literal (scopelint)
				assert.Error(t, err, testcase.err.Error())
				                     ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit c828fa1)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…iteral (scopelint)

```
templates/templates_test.go:74:29: Using the variable on range scope `testCase` in function literal (scopelint)
			assert.Check(t, is.Equal(testCase.expected, b.String()))
			                         ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 54d48de)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… `overrideService` (scopelint)

```
cli/compose/loader/merge.go:64:41: Using a reference for the variable on range scope `overrideService` (scopelint)
			if err := mergo.Merge(&baseService, &overrideService, mergo.WithAppendSlice, mergo.WithOverride, mergo.WithTransformers(specials)); err != nil {
			                                     ^
cli/compose/loader/loader_test.go:1587:28: Using the variable on range scope `testcase` in function literal (scopelint)
			config, err := loadYAML(testcase.yaml)
			                        ^
cli/compose/loader/loader_test.go:1590:58: Using the variable on range scope `testcase` in function literal (scopelint)
			assert.Check(t, is.DeepEqual(config.Services[0].Init, testcase.init))
			                                                      ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 96ec729)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… literal (scopelint)

```
e2e/cli-plugins/flags_test.go:135:27: Using the variable on range scope `args` in function literal (scopelint)
			res := icmd.RunCmd(run(args...))
			                       ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 1736662)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…n literal (scopelint)

```
cli/command/context/create_test.go:270:31: Using the variable on range scope `c` in function literal (scopelint)
				Name:                     c.name,
				                          ^
cli/command/context/create_test.go:271:31: Using the variable on range scope `c` in function literal (scopelint)
				Description:              c.description,
				                          ^
cli/command/context/create_test.go:272:31: Using the variable on range scope `c` in function literal (scopelint)
				DefaultStackOrchestrator: c.orchestrator,

cli/command/context/create_test.go:346:31: Using the variable on range scope `c` in function literal (scopelint)
				Name:                     c.name,
				                          ^
cli/command/context/create_test.go:347:31: Using the variable on range scope `c` in function literal (scopelint)
				Description:              c.description,
				                          ^
cli/command/context/create_test.go:348:31: Using the variable on range scope `c` in function literal (scopelint)
				DefaultStackOrchestrator: c.orchestrator,
				                          ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit a269e17)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…nction literal (scopelint)

```
cli/command/trust/key_load_test.go:121:27: Using the variable on range scope `keyID` in function literal (scopelint)
			testLoadKeyFromPath(t, keyID, keyBytes)
			                       ^
cli/command/trust/key_load_test.go:176:32: Using the variable on range scope `keyBytes` in function literal (scopelint)
			testLoadKeyTooPermissive(t, keyBytes)
			                            ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 7c4b63b)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
cli/config/config_test.go:465: unnecessary trailing newline (whitespace)

}
cli/compose/interpolation/interpolation.go:56: unnecessary leading newline (whitespace)
	switch value := value.(type) {

cli/compose/interpolation/interpolation.go:94: unnecessary trailing newline (whitespace)

	}
cli/command/image/build/context.go:348: unnecessary trailing newline (whitespace)

		}
internal/licenseutils/client_test.go:98: unnecessary leading newline (whitespace)
func (c *fakeLicensingClient) LoadLocalLicense(ctx context.Context, dclnt licensing.WrappedDockerClient) (*model.Subscription, error) {

cli/registry/client/fetcher.go:211: unnecessary leading newline (whitespace)
	for _, endpoint := range endpoints {
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 612d83d)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
… literal (scopelint)

```
cli/command/cli_test.go:157:15: Using the variable on range scope `testcase` in function literal (scopelint)
				pingFunc: testcase.pingFunc,
				          ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 2ec424a)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
internal/licenseutils/client_test.go:98: unnecessary leading newline (whitespace)
func (c *fakeLicensingClient) LoadLocalLicense(ctx context.Context, dclnt licensing.WrappedDockerClient) (*model.Subscription, error) {
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 63e45e6)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit dd4d216)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
```
cli/command/container/opts_test.go:68:2: declaration has 3 blank identifiers (dogsled)
	_, _, _, err := parseRun(strings.Split(args+" ubuntu bash", " "))
	^
cli/command/container/opts_test.go:542:2: declaration has 3 blank identifiers (dogsled)
	_, _, _, err = parseRun([]string{"--uts=container:", "img", "cmd"})
	^
cli/command/container/opts_test.go:603:2: declaration has 3 blank identifiers (dogsled)
	_, _, _, err := parseRun([]string{"--rm", "--restart=always", "img", "cmd"})
	^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 79dc83e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ion literal (scopelint)

```
cli/compose/template/template_test.go:279:31: Using the variable on range scope `tc` in function literal (scopelint)
			actual := ExtractVariables(tc.dict, defaultPattern)
			                           ^
cli/compose/template/template_test.go:280:41: Using the variable on range scope `tc` in function literal (scopelint)
			assert.Check(t, is.DeepEqual(actual, tc.expected))
			                                     ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit aafe3df)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…n literal (scopelint)

```
cli/manifest/store/store_test.go:97:29: Using the variable on range scope `testcase` in function literal (scopelint)
			actual, err := store.Get(testcase.listRef, testcase.manifestRef)
			                         ^
cli/manifest/store/store_test.go:98:7: Using the variable on range scope `testcase` in function literal (scopelint)
			if testcase.expectedErr != "" {
			   ^
cli/manifest/store/store_test.go:99:26: Using the variable on range scope `testcase` in function literal (scopelint)
				assert.Error(t, err, testcase.expectedErr)
				                     ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit cd3dca3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…pelint)

```
opts/network_test.go:74:35: Using the variable on range scope `tc` in function literal (scopelint)
			assert.NilError(t, network.Set(tc.value))
			                               ^
opts/network_test.go:102:40: Using the variable on range scope `tc` in function literal (scopelint)
			assert.ErrorContains(t, network.Set(tc.value), tc.expectedError)
			                                    ^
opts/opts_test.go:270:30: Using the variable on range scope `tc` in function literal (scopelint)
			val, err := ValidateLabel(tc.value)
			                          ^
opts/opts_test.go:271:7: Using the variable on range scope `tc` in function literal (scopelint)
			if tc.expectedErr != "" {
			   ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit c2b069f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ange scope `obj` (scopelint)

```
cli/command/stack/kubernetes/watcher_test.go:44:20: Using a reference for the variable on range scope `obj` (scopelint)
		if err := o.Add(&obj); err != nil {
		                 ^
cli/command/stack/kubernetes/watcher_test.go:49:20: Using a reference for the variable on range scope `obj` (scopelint)
		if err := o.Add(&obj); err != nil {
		                 ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 754fc6f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ion literal (scopelint)

```
cli/command/container/create_test.go:120:20: Using the variable on range scope `c` in function literal (scopelint)
				defer func() { c.ResponseCounter++ }()
				               ^
cli/command/container/create_test.go:121:12: Using the variable on range scope `c` in function literal (scopelint)
				switch c.ResponseCounter {
				       ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 542f802)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…n function literal (scopelint)

```
cli/command/stack/kubernetes/convert_test.go:199:35: Using the variable on range scope `c` in function literal (scopelint)
			conv, err := NewStackConverter(c.version)
			                               ^
```

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 640305f)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b7e06f2)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

did not yet backport #2180 (kubernetes/conversion_test: use test-builders package)

Copy link
Contributor

@silvin-lubecki silvin-lubecki left a comment

Choose a reason for hiding this comment

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

LGTM

@silvin-lubecki silvin-lubecki merged commit 2c7de20 into docker:19.03 Jan 6, 2020
@thaJeztah thaJeztah deleted the 19.03_backport_carry_golangci_lint branch January 6, 2020 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants