Merge branch 'master' into gocritic

.golangci.yml

@@ -11,6 +11,7 @@ linters:
     - asciicheck # Simple linter to check that your code does not contain non-ASCII identifiers [fast: true, auto-fix: false]
     - bidichk # Checks for dangerous unicode character sequences [fast: true, auto-fix: false]
     - bodyclose # checks whether HTTP response body is closed successfully [fast: false, auto-fix: false]
+    - canonicalheader # canonicalheader checks whether net/http.Header uses canonical header [fast: false, auto-fix: false]
     - copyloopvar # copyloopvar is a linter detects places where loop variables are copied [fast: true, auto-fix: false]
     - decorder # check declaration order and count of types, constants, variables and functions [fast: true, auto-fix: false]
     - dogsled # Checks assignments with too many blank identifiers (e.g. x, _, _, _, := f()) [fast: true, auto-fix: false]
@@ -51,6 +52,7 @@ linters:
     - predeclared # find code that shadows one of Go's predeclared identifiers [fast: true, auto-fix: false]
     - promlinter # Check Prometheus metrics naming via promlint [fast: true, auto-fix: false]
     - protogetter # Reports direct reads from proto message fields when getters should be used [fast: false, auto-fix: true]
+    - reassign # Checks that package variables are not reassigned [fast: false, auto-fix: false]
     - revive # Fast, configurable, extensible, flexible, and beautiful linter for Go. Drop-in replacement of golint. [fast: false, auto-fix: false]
     - rowserrcheck # checks whether Err of rows is checked successfully [fast: false, auto-fix: false]
     - sloglint # ensure consistent code style when using log/slog [fast: false, auto-fix: false]
@@ -59,6 +61,7 @@ linters:
     - tenv # tenv is analyzer that detects using os.Setenv instead of t.Setenv since Go1.17 [fast: false, auto-fix: false]
     - testifylint # Checks usage of github.com/stretchr/testify. [fast: false, auto-fix: false]
     - testpackage # linter that makes you use a separate _test package [fast: true, auto-fix: false]
+    - thelper # thelper detects golang test helpers without t.Helper() call and checks the consistency of test helpers [fast: false, auto-fix: false]
     - tparallel # tparallel detects inappropriate usage of t.Parallel() method in your Go test codes [fast: false, auto-fix: false]
     - typecheck # Like the front-end of a Go compiler, parses and type-checks Go code [fast: false, auto-fix: false]
     - unconvert # Remove unnecessary type conversions [fast: false, auto-fix: false]

internal/args/args_test.go

@@ -170,6 +170,7 @@ func TestGetArgType(t *testing.T) {
 
 	run := func(tc *TestCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			res, err := args.GetArgType(tc.ArgType, tc.Name)
 			if tc.expectedError == "" {
 				require.NoError(t, err)

internal/args/marshal_test.go

@@ -22,6 +22,7 @@ func TestMarshal(t *testing.T) {
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			args, err := args.MarshalStruct(testCase.data)
 
 			if testCase.error == "" {
@@ -235,6 +236,7 @@ func TestMarshalValue(t *testing.T) {
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			value, err := args.MarshalValue(testCase.data)
 
 			if testCase.error == "" {

internal/args/unmarshal_test.go

@@ -29,6 +29,7 @@ func TestUnmarshalStruct(t *testing.T) {
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			if testCase.data == nil {
 				testCase.data = reflect.New(reflect.TypeOf(testCase.expected).Elem()).Interface()
 			}
@@ -540,6 +541,7 @@ func TestIsUmarshalableValue(t *testing.T) {
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			value := args.IsUmarshalableValue(testCase.data)
 			assert.Equal(t, testCase.expected, value)
 		}

internal/core/autocomplete_test.go

@@ -86,6 +86,7 @@ type autoCompleteTestCase struct {
 
 func runAutocompleteTest(ctx context.Context, tc *autoCompleteTestCase) func(*testing.T) {
 	return func(t *testing.T) {
+		t.Helper()
 		words := tc.Words
 		if len(words) == 0 {
 			name := strings.ReplaceAll(t.Name(), "TestAutocomplete/", "")

internal/core/bootstrap_test.go

@@ -149,6 +149,7 @@ func TestInterruptError(t *testing.T) {
 		),
 		Cmd: "scw -o json test empty success",
 		Check: func(t *testing.T, ctx *core.CheckFuncCtx) {
+			t.Helper()
 			assert.Equal(t, "[]\n", string(ctx.Stdout))
 		},
 	}))

internal/core/build_info_test.go

@@ -34,6 +34,7 @@ func Test_CheckVersion(t *testing.T) {
 		Cmd: "scw plop",
 		Check: core.TestCheckCombine(
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.Equal(t, "A new version of scw is available (2.5.4), beware that you are currently running 1.20.0\n", ctx.LogBuffer)
 			},
 		),
@@ -48,6 +49,7 @@ func Test_CheckVersion(t *testing.T) {
 		Cmd: "scw plop -D",
 		Check: core.TestCheckCombine(
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.Contains(t, ctx.LogBuffer, "version is up to date (99.99.0)\n")
 			},
 		),
@@ -65,6 +67,7 @@ func Test_CheckVersion(t *testing.T) {
 		Cmd: "scw plop -D",
 		Check: core.TestCheckCombine(
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.Contains(t, ctx.LogBuffer, "version was already checked during past 24 hours\n")
 			},
 		),
@@ -86,6 +89,7 @@ func Test_CheckVersion(t *testing.T) {
 		Cmd: "scw plop",
 		Check: core.TestCheckCombine(
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.Contains(t, ctx.LogBuffer, "A new version of scw is available (2.5.4), beware that you are currently running 1.0.0\n")
 			},
 		),

internal/core/checks_test.go

@@ -75,6 +75,7 @@ func TestCheckAPIKey(t *testing.T) {
 		Check: core.TestCheckCombine(
 			core.TestCheckExitCode(0),
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.True(t, strings.HasPrefix(ctx.LogBuffer, "Current api key expires in"))
 			},
 		),

internal/core/cobra_utils_test.go

@@ -280,6 +280,7 @@ func Test_MultiPositionalArg(t *testing.T) {
 			core.TestCheckExitCode(0),
 			core.TestCheckGolden(),
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				res := ctx.Result.(*testAcceptMultiPositionalArgsType)
 				assert.Len(t, res.NameIDs, 1)
 				assert.Equal(t, "pos1", res.NameIDs[0])
@@ -295,6 +296,7 @@ func Test_MultiPositionalArg(t *testing.T) {
 			core.TestCheckExitCode(0),
 			core.TestCheckGolden(),
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				res := ctx.Result.(*testAcceptMultiPositionalArgsType)
 				assert.Len(t, res.NameIDs, 3)
 				assert.Equal(t, "pos1", res.NameIDs[0])

internal/core/command_interceptor_test.go

@@ -27,6 +27,7 @@ func Test_CombineCommandInterceptor(t *testing.T) {
 
 	run := func(tc *TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			interceptor := core.CombineCommandInterceptor(tc.Interceptors...)
 			res, _ := interceptor(nil, nil, runner)
 			assert.Equal(t, tc.Expected, res)

internal/core/default_test.go

@@ -18,6 +18,7 @@ func Test_ApplyDefaultValues(t *testing.T) {
 
 	run := func(tc *testCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			result := core.ApplyDefaultValues(context.Background(), tc.argSpecs, tc.rawArgs)
 			assert.Equal(t, tc.expected, result)
 		}

internal/core/reflect_test.go

@@ -87,6 +87,7 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				expectedValues: []reflect.Value{reflect.ValueOf("value1")},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, tc.expectedError, err.Error())
@@ -115,6 +116,7 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				expectedValues: []reflect.Value{reflect.ValueOf("value1")},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, tc.expectedError, err.Error())
@@ -156,6 +158,7 @@ func Test_getValuesForFieldByName(t *testing.T) {
 				expectedValues: []reflect.Value{reflect.ValueOf(expectedServiceIP)},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				values, err := core.GetValuesForFieldByName(reflect.ValueOf(tc.cmdArgs), strings.Split(tc.fieldName, "."))
 				if err != nil {
 					assert.Equal(t, nil, err.Error())

internal/core/testing.go

@@ -200,6 +200,7 @@ type TestConfig struct {
 
 // getTestFilePath returns a valid filename path based on the go test name and suffix. (Take care of non fs friendly char)
 func getTestFilePath(t *testing.T, suffix string) string {
+	t.Helper()
 	specialChars := regexp.MustCompile(`[\\?%*:|"<>. ]`)
 
 	// Replace nested tests separators.
@@ -214,6 +215,7 @@ func getTestFilePath(t *testing.T, suffix string) string {
 }
 
 func createTestClient(t *testing.T, testConfig *TestConfig, httpClient *http.Client) (client *scw.Client) {
+	t.Helper()
 	var err error
 
 	// Init default options
@@ -277,6 +279,7 @@ var DefaultRetryInterval *time.Duration
 // Run a CLI integration test. See TestConfig for configuration option
 func Test(config *TestConfig) func(t *testing.T) {
 	return func(t *testing.T) {
+		t.Helper()
 		if !config.DisableParallel {
 			t.Parallel()
 		}
@@ -588,6 +591,7 @@ func ExecAfterCmd(cmd string) AfterFunc {
 // TestCheckCombine combines multiple check functions into one.
 func TestCheckCombine(checks ...TestCheck) TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		for _, check := range checks {
 			check(t, ctx)
 		}
@@ -597,6 +601,7 @@ func TestCheckCombine(checks ...TestCheck) TestCheck {
 // TestCheckExitCode assert exitCode
 func TestCheckExitCode(expectedCode int) TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		assert.Equal(t, expectedCode, ctx.ExitCode, "Invalid exit code\n%s", string(ctx.Stderr))
 	}
 }
@@ -640,6 +645,7 @@ func GoldenReplacePatterns(golden string, replacements ...GoldenReplacement) (st
 // golden are matched against given regex and edited with replacements
 func TestCheckGoldenAndReplacePatterns(replacements ...GoldenReplacement) TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		actual := marshalGolden(t, ctx)
 		actual, actualReplaceErr := GoldenReplacePatterns(actual, replacements...)
 
@@ -660,6 +666,7 @@ func TestCheckGoldenAndReplacePatterns(replacements ...GoldenReplacement) TestCh
 // TestCheckGolden assert stderr and stdout using golden
 func TestCheckGolden() TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		actual := marshalGolden(t, ctx)
 
 		goldenPath := getTestFilePath(t, ".golden")
@@ -678,6 +685,7 @@ func TestCheckGolden() TestCheck {
 // TestCheckS3Golden assert stderr and stdout using golden, and omits the random suffix in the bucket name
 func TestCheckS3Golden() TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		actual := marshalGolden(t, ctx)
 		normalizedActual := removeRandomPrefixFromOutput(actual)
 
@@ -710,13 +718,15 @@ func removeRandomPrefixFromOutput(output string) string {
 // TestCheckError asserts error
 func TestCheckError(err error) TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		assert.Equal(t, err, ctx.Err, "Invalid error")
 	}
 }
 
 // TestCheckStdout asserts stdout using string
 func TestCheckStdout(stdout string) TestCheck {
 	return func(t *testing.T, ctx *CheckFuncCtx) {
+		t.Helper()
 		assert.Equal(t, stdout, string(ctx.Stdout), "Invalid stdout")
 	}
 }
@@ -736,6 +746,7 @@ func uniformTimestamps(input string) string {
 }
 
 func validateJSONGolden(t *testing.T, jsonStdout, jsonStderr *bytes.Buffer) {
+	t.Helper()
 	var jsonInterface interface{}
 	if jsonStdout.Len() > 0 {
 		err := json.Unmarshal(jsonStdout.Bytes(), &jsonInterface)
@@ -748,6 +759,7 @@ func validateJSONGolden(t *testing.T, jsonStdout, jsonStderr *bytes.Buffer) {
 }
 
 func marshalGolden(t *testing.T, ctx *CheckFuncCtx) string {
+	t.Helper()
 	jsonStderr := &bytes.Buffer{}
 	jsonStdout := &bytes.Buffer{}
 

internal/core/testing_recorder.go

@@ -114,6 +114,7 @@ func customS3Matcher(r *http.Request, i cassette.Request) bool {
 // It is important to call add a `defer cleanup()` so the given cassette files are correctly
 // closed and saved after the requests.
 func getHTTPRecoder(t *testing.T, update bool) (client *http.Client, cleanup func(), err error) {
+	t.Helper()
 	recorderMode := recorder.ModeReplaying
 	if update {
 		recorderMode = recorder.ModeRecording

internal/core/validate_test.go

@@ -40,6 +40,7 @@ func Test_DefaultCommandValidateFunc(t *testing.T) {
 
 	run := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			err := core.DefaultCommandValidateFunc()(context.Background(), testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, errors.New("arg validation called"), err)
 		}
@@ -197,13 +198,15 @@ func Test_DefaultCommandRequiredFunc(t *testing.T) {
 
 	runOK := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			err := core.DefaultCommandValidateFunc()(context.Background(), testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, nil, err)
 		}
 	}
 
 	runErr := func(testCase TestCase, argName string) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			err := core.DefaultCommandValidateFunc()(context.Background(), testCase.command, testCase.parsedArguments, testCase.rawArgs)
 			assert.Equal(t, core.MissingRequiredArgumentError(argName), err)
 		}
@@ -301,13 +304,15 @@ func Test_ValidateNoConflict(t *testing.T) {
 
 	runOK := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			err := core.ValidateNoConflict(testCase.command, testCase.rawArgs)
 			assert.Equal(t, nil, err)
 		}
 	}
 
 	runErr := func(testCase TestCase) func(t *testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			err := core.ValidateNoConflict(testCase.command, testCase.rawArgs)
 			assert.Equal(t, core.ArgumentConflictError(testCase.arg1, testCase.arg2), err)
 		}
@@ -366,6 +371,7 @@ func Test_ValidateDeprecated(t *testing.T) {
 		Cmd: "scw plop a=yo",
 		Check: core.TestCheckCombine(
 			func(t *testing.T, ctx *core.CheckFuncCtx) {
+				t.Helper()
 				assert.Equal(t, "The argument 'a' is deprecated, more info with: scw plop --help\n", ctx.LogBuffer)
 			},
 		),
@@ -397,6 +403,7 @@ func TestNewOneOfGroupManager(t *testing.T) {
 				expectedRequiredGroups: map[string]bool{},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				manager := core.NewOneOfGroupManager(tc.command)
 				assert.Equal(t, tc.expectedGroups, manager.Groups)
 				assert.Equal(t, tc.expectedRequiredGroups, manager.RequiredGroups)
@@ -415,6 +422,7 @@ func TestNewOneOfGroupManager(t *testing.T) {
 				expectedRequiredGroups: map[string]bool{"group1": true},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				manager := core.NewOneOfGroupManager(tc.command)
 				assert.Equal(t, tc.expectedGroups, manager.Groups)
 				assert.Equal(t, tc.expectedRequiredGroups, manager.RequiredGroups)
@@ -438,6 +446,7 @@ func TestNewOneOfGroupManager(t *testing.T) {
 				expectedRequiredGroups: map[string]bool{},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				manager := core.NewOneOfGroupManager(tc.command)
 				assert.Equal(t, tc.expectedGroups, manager.Groups)
 				assert.Equal(t, tc.expectedRequiredGroups, manager.RequiredGroups)
@@ -463,6 +472,7 @@ func TestNewOneOfGroupManager(t *testing.T) {
 				},
 			},
 			testFunc: func(t *testing.T, tc TestCase) {
+				t.Helper()
 				manager := core.NewOneOfGroupManager(tc.command)
 				assert.Equal(t, tc.expectedGroups, manager.Groups)
 				assert.Equal(t, tc.expectedRequiredGroups, manager.RequiredGroups)

internal/gofields/gofields_test.go

@@ -45,6 +45,7 @@ func TestGetValue(t *testing.T) {
 
 	run := func(tc *TestCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			actual, err := gofields.GetValue(tc.Data, tc.Path)
 			if err != nil {
 				assert.Equal(t, tc.Expected, err.Error())
@@ -154,6 +155,7 @@ func TestGetType(t *testing.T) {
 
 	run := func(tc *TestCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			actual, err := gofields.GetType(tc.Data, tc.Path)
 			if err != nil {
 				assert.Equal(t, tc.Expected, err.Error())
@@ -229,6 +231,7 @@ func TestListFields(t *testing.T) {
 
 	run := func(tc *TestCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			actual := gofields.ListFields(tc.Data)
 			assert.Equal(t, tc.Expected, actual)
 		}
@@ -249,6 +252,7 @@ func TestListFieldsWithFilter(t *testing.T) {
 
 	run := func(tc *TestCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			actual := gofields.ListFieldsWithFilter(tc.Data, tc.Filter)
 			assert.Equal(t, tc.Expected, actual)
 		}

internal/human/marshal_test.go

@@ -76,6 +76,7 @@ func TestMarshal(t *testing.T) {
 
 	run := func(tc *testCase) func(*testing.T) {
 		return func(t *testing.T) {
+			t.Helper()
 			result, err := human.Marshal(tc.data, tc.opt)
 
 			// Format expected to allow indentation when writing test