Merge branch '785-twofactor-lint' into 'main'

Lint fixes for twofactor packages

Closes #785

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1143



Merged-by: Ash McKenzie <amckenzie@gitlab.com>
Approved-by: Ash McKenzie <amckenzie@gitlab.com>
Co-authored-by: Archish <archishthakkar@gmail.com>

internal/command/twofactorrecover/twofactorrecover.go

@@ -34,7 +34,7 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) {
 		c.displayRecoveryCodes(ctx)
 	} else {
 		ctxlog.Debug("twofactorrecover: execute: User chose not to continue")
-		fmt.Fprintln(c.ReadWriter.Out, "\nNew recovery codes have *not* been generated. Existing codes will remain valid.")
+		_, _ = fmt.Fprintln(c.ReadWriter.Out, "\nNew recovery codes have *not* been generated. Existing codes will remain valid.")
 	}
 
 	return ctx, nil
@@ -44,7 +44,7 @@ func (c *Command) getUserAnswer(ctx context.Context) string {
 	question :=
 		"Are you sure you want to generate new two-factor recovery codes?\n" +
 			"Any existing recovery codes you saved will be invalidated. (yes/no)"
-	fmt.Fprintln(c.ReadWriter.Out, question)
+	_, _ = fmt.Fprintln(c.ReadWriter.Out, question)
 
 	var answer string
 	if _, err := fmt.Fscanln(io.LimitReader(c.ReadWriter.In, readerLimit), &answer); err != nil {
@@ -67,10 +67,10 @@ func (c *Command) displayRecoveryCodes(ctx context.Context) {
 				"\n\nDuring sign in, use one of the codes above when prompted for\n" +
 				"your two-factor code. Then, visit your Profile Settings and add\n" +
 				"a new device so you do not lose access to your account again.\n"
-		fmt.Fprint(c.ReadWriter.Out, messageWithCodes)
+		_, _ = fmt.Fprint(c.ReadWriter.Out, messageWithCodes)
 	} else {
 		ctxlog.WithError(err).Error("twofactorrecover: displayRecoveryCodes: failed to generate recovery codes")
-		fmt.Fprintf(c.ReadWriter.Out, "\nAn error occurred while trying to generate new recovery codes.\n%v\n", err)
+		_, _ = fmt.Fprintf(c.ReadWriter.Out, "\nAn error occurred while trying to generate new recovery codes.\n%v\n", err)
 	}
 }
 

internal/command/twofactorrecover/twofactorrecover_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
@@ -28,7 +29,7 @@ func setup(t *testing.T) {
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
 
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				var requestBody *twofactorrecover.RequestBody
 				json.Unmarshal(b, &requestBody)

internal/command/twofactorverify/twofactorverify.go

@@ -37,7 +37,7 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) {
 	ctx, cancel := context.WithTimeout(ctx, timeout)
 	defer cancel()
 
-	fmt.Fprint(c.ReadWriter.Out, prompt)
+	_, _ = fmt.Fprint(c.ReadWriter.Out, prompt)
 
 	resultCh := make(chan string)
 	go func() {
@@ -68,7 +68,7 @@ func (c *Command) Execute(ctx context.Context) (context.Context, error) {
 	}
 
 	log.WithContextFields(ctx, log.Fields{"message": message}).Info("Two factor verify command finished")
-	fmt.Fprintf(c.ReadWriter.Out, "\n%v\n", message)
+	_, _ = fmt.Fprintf(c.ReadWriter.Out, "\n%v\n", message)
 
 	return ctx, nil
 }

internal/command/twofactorverify/twofactorverify_test.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
@@ -35,10 +36,10 @@ func setup(t *testing.T) []testserver.TestRequestHandler {
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
 
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				var requestBody *twofactorverify.RequestBody
-				require.NoError(t, json.Unmarshal(b, &requestBody))
+				assert.NoError(t, json.Unmarshal(b, &requestBody))
 
 				switch requestBody.KeyID {
 				case "verify_via_otp", "verify_via_otp_with_push_error":
@@ -53,7 +54,7 @@ func setup(t *testing.T) []testserver.TestRequestHandler {
 						"success": false,
 						"message": "error message",
 					}
-					require.NoError(t, json.NewEncoder(w).Encode(body))
+					assert.NoError(t, json.NewEncoder(w).Encode(body))
 				case "broken":
 					w.WriteHeader(http.StatusInternalServerError)
 				}
@@ -65,10 +66,10 @@ func setup(t *testing.T) []testserver.TestRequestHandler {
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
 
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				var requestBody *twofactorverify.RequestBody
-				require.NoError(t, json.Unmarshal(b, &requestBody))
+				assert.NoError(t, json.Unmarshal(b, &requestBody))
 
 				switch requestBody.KeyID {
 				case "verify_via_push":

internal/gitlabnet/twofactorrecover/client_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
@@ -27,7 +28,7 @@ func initialize(t *testing.T) {
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
 
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				var requestBody *RequestBody
 				json.Unmarshal(b, &requestBody)

internal/gitlabnet/twofactorverify/client_test.go

@@ -9,6 +9,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet/discover"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client"
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
@@ -21,29 +22,29 @@ func initialize(t *testing.T) []testserver.TestRequestHandler {
 		b, err := io.ReadAll(r.Body)
 		defer r.Body.Close()
 
-		require.NoError(t, err)
+		assert.NoError(t, err)
 
 		var requestBody *RequestBody
-		require.NoError(t, json.Unmarshal(b, &requestBody))
+		assert.NoError(t, json.Unmarshal(b, &requestBody))
 
 		switch requestBody.KeyID {
 		case "0":
 			body := map[string]interface{}{
 				"success": true,
 			}
-			require.NoError(t, json.NewEncoder(w).Encode(body))
+			assert.NoError(t, json.NewEncoder(w).Encode(body))
 		case "1":
 			body := map[string]interface{}{
 				"success": false,
 				"message": "error message",
 			}
-			require.NoError(t, json.NewEncoder(w).Encode(body))
+			assert.NoError(t, json.NewEncoder(w).Encode(body))
 		case "2":
 			w.WriteHeader(http.StatusForbidden)
 			body := &client.ErrorResponse{
 				Message: "Not allowed!",
 			}
-			require.NoError(t, json.NewEncoder(w).Encode(body))
+			assert.NoError(t, json.NewEncoder(w).Encode(body))
 		case "3":
 			w.Write([]byte("{ \"message\": \"broken json!\""))
 		case "4":
@@ -54,7 +55,7 @@ func initialize(t *testing.T) []testserver.TestRequestHandler {
 			body := map[string]interface{}{
 				"success": true,
 			}
-			require.NoError(t, json.NewEncoder(w).Encode(body))
+			assert.NoError(t, json.NewEncoder(w).Encode(body))
 		}
 	}
 
@@ -75,7 +76,7 @@ func initialize(t *testing.T) []testserver.TestRequestHandler {
 					Username: "jane-doe",
 					Name:     "Jane Doe",
 				}
-				require.NoError(t, json.NewEncoder(w).Encode(body))
+				assert.NoError(t, json.NewEncoder(w).Encode(body))
 			},
 		},
 	}

support/lint_last_known_acceptable.txt

@@ -278,18 +278,6 @@ internal/command/shared/customaction/customaction_test.go:118:5: go-require: do
 internal/command/shared/customaction/customaction_test.go:121:5: go-require: do not use require in http handlers (testifylint)
 internal/command/shared/disallowedcommand/disallowedcommand.go:1:1: package-comments: should have a package comment (revive)
 internal/command/shared/disallowedcommand/disallowedcommand.go:6:2: exported: exported var Error should have comment or be unexported (revive)
-internal/command/twofactorrecover/twofactorrecover.go:37:15: Error return value of `fmt.Fprintln` is not checked (errcheck)
-internal/command/twofactorrecover/twofactorrecover.go:47:14: Error return value of `fmt.Fprintln` is not checked (errcheck)
-internal/command/twofactorrecover/twofactorrecover.go:70:13: Error return value of `fmt.Fprint` is not checked (errcheck)
-internal/command/twofactorrecover/twofactorrecover.go:73:14: Error return value of `fmt.Fprintf` is not checked (errcheck)
-internal/command/twofactorrecover/twofactorrecover_test.go:31:5: go-require: do not use require in http handlers (testifylint)
-internal/command/twofactorverify/twofactorverify.go:40:12: Error return value of `fmt.Fprint` is not checked (errcheck)
-internal/command/twofactorverify/twofactorverify.go:71:13: Error return value of `fmt.Fprintf` is not checked (errcheck)
-internal/command/twofactorverify/twofactorverify_test.go:38:5: go-require: do not use require in http handlers (testifylint)
-internal/command/twofactorverify/twofactorverify_test.go:41:5: go-require: do not use require in http handlers (testifylint)
-internal/command/twofactorverify/twofactorverify_test.go:56:6: go-require: do not use require in http handlers (testifylint)
-internal/command/twofactorverify/twofactorverify_test.go:68:5: go-require: do not use require in http handlers (testifylint)
-internal/command/twofactorverify/twofactorverify_test.go:71:5: go-require: do not use require in http handlers (testifylint)
 internal/command/uploadarchive/gitalycall_test.go:24:4: S1038: should use t.Logf(...) instead of t.Log(fmt.Sprintf(...)) (gosimple)
 internal/command/uploadarchive/gitalycall_test.go:32:4: var-naming: var userId should be userID (revive)
 internal/config/config.go:1:1: package-comments: should have a package comment (revive)
@@ -381,14 +369,6 @@ internal/gitlabnet/lfstransfer/client.go:214:1: exported: exported method Client
 internal/gitlabnet/lfstransfer/client.go:271:1: exported: exported method Client.Unlock should have comment or be unexported (revive)
 internal/gitlabnet/lfstransfer/client.go:321:1: exported: exported method Client.ListLocksVerify should have comment or be unexported (revive)
 internal/gitlabnet/personalaccesstoken/client_test.go:30:5: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorrecover/client_test.go:30:5: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:24:3: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:27:3: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:34:4: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:40:4: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:46:4: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:57:4: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/twofactorverify/client_test.go:78:5: go-require: do not use require in http handlers (testifylint)
 internal/logger/logger.go:1:1: package-comments: should have a package comment (revive)
 internal/logger/logger.go:55:9: ST1005: error strings should not be capitalized (stylecheck)
 internal/logger/logger.go:69:27: printf: non-constant format string in call to fmt.Fprintf (govet)