add revive to golangci-lint (#28303)

go/.golangci.yml

@@ -2,15 +2,46 @@ issues:
   exclude-files:
     - prot.go
     - 'cmd_wallet.*.go'
+    - protocol
 
 linters-settings:
   gocritic:
     disabled-checks:
       - ifElseChain
       - elseif
+  revive:
+    rules:
+      - name: unused-parameter
+        disabled: true
+      - name: error-strings
+        disabled: true
+      - name: exported
+        disabled: true
+      - name: blank-imports
+      - name: context-as-argument
+      - name: context-keys-type
+      - name: dot-imports
+      - name: empty-block
+      - name: error-naming
+      - name: error-return
+      - name: errorf
+      - name: increment-decrement
+      - name: indent-error-flow
+      - name: package-comments
+      - name: range
+      - name: receiver-naming
+      - name: redefines-builtin-id
+      - name: superfluous-else
+      - name: time-naming
+      - name: unexported-return
+      - name: unreachable-code
+      - name: var-declaration
+      - name: var-naming
 
 linters:
   enable:
     - gofmt
     - gocritic
     - unconvert
+    - govet
+    - revive

go/badges/badger.go

@@ -48,7 +48,7 @@ func NewBadger(g *libkb.GlobalContext) *Badger {
 		shutdownCh:     make(chan struct{}),
 	}
 	go b.notifyLoop()
-	g.PushShutdownHook(func(mctx libkb.MetaContext) error {
+	g.PushShutdownHook(func(_ libkb.MetaContext) error {
 		close(b.shutdownCh)
 		return nil
 	})

go/chat/attachment_httpsrv.go

@@ -37,7 +37,7 @@ import (
 
 const keyPrefixLen = 2
 
-var blankProgress = func(bytesComplete, bytesTotal int64) {}
+var blankProgress = func(_, _ int64) {}
 
 type AttachmentHTTPSrv struct {
 	sync.Mutex

go/chat/attachments/store.go

@@ -144,11 +144,11 @@ func (a *S3Store) UploadAsset(ctx context.Context, task *UploadTask, encryptedOu
 
 	// encrypt the stream
 	enc := NewSignEncrypter()
-	len := enc.EncryptedLen(task.FileSize)
+	size := enc.EncryptedLen(task.FileSize)
 
 	// check for previous interrupted upload attempt
 	var previous *AttachmentInfo
-	resumable := len > minMultiSize // can only resume multi uploads
+	resumable := size > minMultiSize // can only resume multi uploads
 	if resumable {
 		previous = a.previousUpload(ctx, task)
 	}

go/chat/bots/commands.go

@@ -348,9 +348,8 @@ func (b *CachingBotCommandManager) ListCommands(ctx context.Context, convID chat
 			return true
 		} else if l.Username > r.Username {
 			return false
-		} else {
-			return l.Name < r.Name
 		}
+		return l.Name < r.Name
 	})
 	return res, alias, nil
 }

go/chat/boxer_canned_test.go

@@ -118,7 +118,7 @@ func getCannedMessage(t *testing.T, tag string) cannedMessage {
 	}
 	errStr := fmt.Sprintf("Cannot find canned message: %q", tag)
 	t.Fatalf("%s", errStr)
-	panic(errStr)
+	return cannedMessage{}
 }
 
 func (cm cannedMessage) AsBoxed(t *testing.T) (res chat1.MessageBoxed) {

go/chat/boxer_test.go

@@ -68,7 +68,7 @@ func textMsgWithSender(t *testing.T, text string, uid gregor1.UID, mbVersion cha
 	return textMsgWithHeader(t, text, header)
 }
 
-func textMsgWithHeader(t *testing.T, text string, header chat1.MessageClientHeader) chat1.MessagePlaintext {
+func textMsgWithHeader(_ *testing.T, text string, header chat1.MessageClientHeader) chat1.MessagePlaintext {
 	return chat1.MessagePlaintext{
 		ClientHeader: header,
 		MessageBody:  chat1.NewMessageBodyWithText(chat1.MessageText{Body: text}),

go/chat/commands_test.go

@@ -33,7 +33,7 @@ func TestChatCommands(t *testing.T) {
 		case msg := <-list.newMessageRemote:
 			require.True(t, msg.Message.IsValid())
 			require.True(t, msg.Message.Valid().MessageBody.IsType(chat1.MessageType_HEADLINE))
-			require.Equal(t, "chat about some pointless stuff",
+			require.Equal(t, headline,
 				msg.Message.Valid().MessageBody.Headline().Headline)
 		case <-time.After(timeout):
 			require.Fail(t, "no msg")
@@ -113,12 +113,12 @@ func TestChatCommands(t *testing.T) {
 	mustPostLocalForTest(t, ctc, users[0], impConv, chat1.NewMessageBodyWithText(chat1.MessageText{
 		Body: "/topic chat about some pointless stuff",
 	}))
-	checkHeadline(listener0, "/topic chat about some pointless stuff")
+	checkHeadline(listener0, "chat about some pointless stuff")
 	mustPostLocalForTest(t, ctc, users[0], teamConv, chat1.NewMessageBodyWithText(chat1.MessageText{
 		Body: "/topic chat about some pointless stuff",
 	}))
-	checkHeadline(listener0, "/topic chat about some pointless stuff")
-	checkHeadline(listener1, "/topic chat about some pointless stuff")
+	checkHeadline(listener0, "chat about some pointless stuff")
+	checkHeadline(listener1, "chat about some pointless stuff")
 
 	testLeave := func() {
 		mustPostLocalForTest(t, ctc, users[0], ncres.Conv.Info,

go/chat/convloader_test.go

@@ -77,12 +77,12 @@ func (s slowestRemote) delay(ctx context.Context) {
 	}
 }
 
-func (s slowestRemote) GetThreadRemote(ctx context.Context, arg chat1.GetThreadRemoteArg) (res chat1.GetThreadRemoteRes, err error) {
+func (s slowestRemote) GetThreadRemote(ctx context.Context, _ chat1.GetThreadRemoteArg) (res chat1.GetThreadRemoteRes, err error) {
 	s.delay(ctx)
 	return res, context.Canceled
 }
 
-func (s slowestRemote) GetMessagesRemote(ctx context.Context, arg chat1.GetMessagesRemoteArg) (res chat1.GetMessagesRemoteRes, err error) {
+func (s slowestRemote) GetMessagesRemote(ctx context.Context, _ chat1.GetMessagesRemoteArg) (res chat1.GetMessagesRemoteRes, err error) {
 	s.delay(ctx)
 	return res, context.Canceled
 }

go/chat/convsource.go

@@ -62,7 +62,7 @@ func (s *baseConversationSource) DeleteAssets(ctx context.Context, uid gregor1.U
 	// Fire off a background load of the thread with a post hook to delete the bodies cache
 	err := s.G().ConvLoader.Queue(ctx, types.NewConvLoaderJob(convID, &chat1.Pagination{Num: 0},
 		types.ConvLoaderPriorityHighest, types.ConvLoaderUnique,
-		func(ctx context.Context, tv chat1.ThreadView, job types.ConvLoaderJob) {
+		func(ctx context.Context, _ chat1.ThreadView, job types.ConvLoaderJob) {
 			fetcher := s.G().AttachmentURLSrv.GetAttachmentFetcher()
 			if err := fetcher.DeleteAssets(ctx, convID, assets, s.ri, s); err != nil {
 				s.Debug(ctx, "DeleteAssets: Error purging ephemeral attachments %v", err)

go/chat/deliverer.go

@@ -321,7 +321,7 @@ func (s *Deliverer) alertFailureChannels(obrs []chat1.OutboxRecord) {
 	s.notifyFailureChs = make(map[string]chan []chat1.OutboxRecord)
 }
 
-func (s *Deliverer) doNotRetryFailure(ctx context.Context, obr chat1.OutboxRecord, err error) (resType chat1.OutboxErrorType, resErr error, resFail bool) {
+func (s *Deliverer) doNotRetryFailure(ctx context.Context, obr chat1.OutboxRecord, err error) (resType chat1.OutboxErrorType, resErr error, resFail bool) { // nolint
 	defer func() {
 		if resErr != nil && resFail {
 			s.Debug(ctx, "doNotRetryFailure: sending back to not retry: err: %s: typ: %T", resErr, resErr)

go/chat/ephemeral_purger.go

@@ -343,7 +343,7 @@ func (b *BackgroundEphemeralPurger) resetTimer(ctx context.Context, purgeInfo ch
 }
 
 func newConvLoaderEphemeralPurgeHook(g *globals.Context, uid gregor1.UID, purgeInfo *chat1.EphemeralPurgeInfo) func(ctx context.Context, tv chat1.ThreadView, job types.ConvLoaderJob) {
-	return func(ctx context.Context, tv chat1.ThreadView, job types.ConvLoaderJob) {
+	return func(ctx context.Context, _ chat1.ThreadView, job types.ConvLoaderJob) {
 		if _, _, err := g.ConvSource.EphemeralPurge(ctx, job.ConvID, uid, purgeInfo); err != nil {
 			g.GetLog().CDebugf(ctx, "ephemeralPurge: %s", err)
 		}

go/chat/flip/chat_test.go

@@ -229,7 +229,7 @@ func (c *chatClient) consumeRevealsAndError(t *testing.T, nReveals int) {
 	require.True(t, revealsReceived <= nReveals)
 }
 
-func (c *chatClient) consumeTimeoutError(t *testing.T) {
+func (c *chatClient) consumeTimeoutError(_ *testing.T) {
 	msg := <-c.dealer.UpdateCh()
 	testPrintf("ERR %+v\n", msg)
 }

go/chat/flip/dealer.go

@@ -760,7 +760,7 @@ func (d *Dealer) handleMessageStart(ctx context.Context, msg *GameMessageWrapped
 	return nil
 }
 
-func (d *Dealer) handleMessageOthers(c context.Context, msg *GameMessageWrapped) error {
+func (d *Dealer) handleMessageOthers(_ context.Context, msg *GameMessageWrapped) error {
 	d.Lock()
 	defer d.Unlock()
 	md := msg.GameMetadata()

go/chat/flip/prng.go

@@ -24,13 +24,6 @@ func NewPRNG(s Secret) *PRNG {
 	}
 }
 
-func min(x, y int) int {
-	if x < y {
-		return x
-	}
-	return y
-}
-
 func (p *PRNG) read(ret []byte) int {
 	n := min(len(p.buf), len(ret))
 	copy(ret[0:n], p.buf[0:n])

go/chat/flip/prng_test.go

@@ -1,9 +1,10 @@
 package flip
 
 import (
-	"github.com/stretchr/testify/require"
 	"math/big"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestPRNG(t *testing.T) {
@@ -88,19 +89,19 @@ func TestPRNGRanges(t *testing.T) {
 		found0 := false
 		foundMax := false
 		prng := NewPRNG(secret)
-		max := n
-		if max > 0 {
-			max--
+		maxV := n
+		if maxV > 0 {
+			maxV--
 		} else {
-			max++
+			maxV++
 		}
 
 		for i := 0; i < 1000 && (!found0 || !foundMax); i++ {
 			val := prng.Int(n)
 			if val == 0 {
 				found0 = true
 			}
-			if val == max {
+			if val == maxV {
 				foundMax = true
 			}
 		}

go/chat/flip/simple.go

@@ -88,7 +88,7 @@ func Flip(cp CommitmentPayload, players []PlayerState) (*PRNG, error) {
 // FlipOneBig takes all the completed PlayerStates, and checks them.  If no error,
 // then outputs one random number between 0 and the given modulus, which is an arbitrarily
 // big number. If there was an error in the game setup, then it will return nil and the error.
-func FlipOneBig(cp CommitmentPayload, players []PlayerState, modulus *big.Int) (*big.Int, error) {
+func FlipOneBig(cp CommitmentPayload, players []PlayerState, modulus *big.Int) (*big.Int, error) { // nolint
 	prng, err := Flip(cp, players)
 	if err != nil {
 		return nil, err
@@ -99,7 +99,7 @@ func FlipOneBig(cp CommitmentPayload, players []PlayerState, modulus *big.Int) (
 // FlipOneInt takes all the completed PlayerStates, and checks them.  If no error,
 // then outputs one random number between 0 and the given modulus, a signed 64-bit int.
 // If there was an  error in the game setup, then it will return 0 and the error.
-func FlipInt(cp CommitmentPayload, players []PlayerState, modulus int64) (int64, error) {
+func FlipInt(cp CommitmentPayload, players []PlayerState, modulus int64) (int64, error) { // nolint
 	prng, err := Flip(cp, players)
 	if err != nil {
 		return 0, err
@@ -110,7 +110,7 @@ func FlipInt(cp CommitmentPayload, players []PlayerState, modulus int64) (int64,
 // FlipOneBool takes all the completed PlayerStates, and checks them. If no error,
 // then outputs one random bool. If there was an error in the game setup, then it will
 // return false and the error.
-func FlipBool(cp CommitmentPayload, players []PlayerState) (bool, error) {
+func FlipBool(cp CommitmentPayload, players []PlayerState) (bool, error) { // nolint
 	prng, err := Flip(cp, players)
 	if err != nil {
 		return false, err

go/chat/helper.go

@@ -505,7 +505,7 @@ func PresentConversationLocalWithFetchRetry(ctx context.Context, g *globals.Cont
 	return pc
 }
 
-func GetTopicNameState(ctx context.Context, g *globals.Context, debugger utils.DebugLabeler,
+func GetTopicNameState(ctx context.Context, _ *globals.Context, debugger utils.DebugLabeler,
 	convs []chat1.ConversationLocal,
 	uid gregor1.UID, tlfID chat1.TLFID, topicType chat1.TopicType,
 	membersType chat1.ConversationMembersType) (res chat1.TopicNameState, err error) {

go/chat/inboxsource.go

@@ -181,7 +181,7 @@ func (b *baseInboxSource) Localize(ctx context.Context, uid gregor1.UID, convs [
 	return res, localizeCb, err
 }
 
-func (b *baseInboxSource) RemoteSetConversationStatus(ctx context.Context, uid gregor1.UID,
+func (b *baseInboxSource) RemoteSetConversationStatus(ctx context.Context, _ gregor1.UID,
 	convID chat1.ConversationID, status chat1.ConversationStatus) (err error) {
 	defer b.Trace(ctx, &err, "RemoteSetConversationStatus")()
 	if _, err = b.getChatInterface().SetConversationStatus(ctx, chat1.SetConversationStatusArg{
@@ -268,9 +268,8 @@ func GetInboxQueryNameInfo(ctx context.Context, g *globals.Context,
 		}
 		return CreateNameInfoSource(ctx, g, lquery.Name.MembersType).LookupID(ctx, lquery.Name.Name,
 			lquery.Visibility() == keybase1.TLFVisibility_PUBLIC)
-	} else {
-		return res, errors.New("invalid name query")
 	}
+	return res, errors.New("invalid name query")
 }
 
 type RemoteInboxSource struct {

go/chat/journey_card_manager.go

@@ -420,9 +420,8 @@ func (cc *JourneyCardManagerSingleUser) PickCard(ctx context.Context,
 			if jcd.hasShownOrDismissedOrLockout(convID, cardType) {
 				if breakOnShown {
 					break
-				} else {
-					continue
 				}
+				continue
 			}
 			if cond, ok := cardConditions[cardType]; ok && cond(ctx) {
 				cc.Debug(ctx, "selected new card: %v", cardType)
@@ -500,7 +499,7 @@ func (cc *JourneyCardManagerSingleUser) PickCard(ctx context.Context,
 // Card type: WELCOME (1 on design)
 // Condition: Only in #general channel
 // Condition: Less than 4 weeks have passed since the user joined the team (ish: see JoinedTime).
-func (cc *JourneyCardManagerSingleUser) cardWelcome(ctx context.Context, convID chat1.ConversationID, conv convForJourneycard, jcd journeycardData, debugDebug logFn) bool {
+func (cc *JourneyCardManagerSingleUser) cardWelcome(ctx context.Context, _ chat1.ConversationID, conv convForJourneycard, jcd journeycardData, debugDebug logFn) bool {
 	// TODO PICNIC-593 Welcome's interaction with existing system message
 	// Welcome cards show not show for all pre-existing teams when a client upgrades to first support journey cards. That would be a bad transition.
 	// The server gates whether welcome cards are allowed for a conv. After MarkAsRead-ing a conv, welcome cards are banned.
@@ -816,7 +815,7 @@ func (cc *JourneyCardManagerSingleUser) timeSinceJoinedLE(ctx context.Context, t
 }
 
 func (cc *JourneyCardManagerSingleUser) messageSince(ctx context.Context, msgID chat1.MessageID,
-	conv convForJourneycard, thread *chat1.ThreadView, debugDebug logFn) bool {
+	_ convForJourneycard, thread *chat1.ThreadView, debugDebug logFn) bool {
 	for _, msg := range thread.Messages {
 		state, err := msg.State()
 		if err != nil {

go/chat/keyfinder.go

@@ -163,8 +163,8 @@ func NewKeyFinderMock(cryptKeys []keybase1.CryptKey) types.KeyFinder {
 func (k *KeyFinderMock) Reset() {}
 
 func (k *KeyFinderMock) FindForEncryption(ctx context.Context,
-	tlfName string, teamID chat1.TLFID,
-	membersType chat1.ConversationMembersType, public bool,
+	_ string, _ chat1.TLFID,
+	_ chat1.ConversationMembersType, _ bool,
 	botUID *gregor1.UID) (res types.CryptKey, ni types.NameInfo, err error) {
 	if botUID != nil {
 		return res, ni, fmt.Errorf("bot keys not supported in KeyFinderMock")
@@ -173,8 +173,8 @@ func (k *KeyFinderMock) FindForEncryption(ctx context.Context,
 }
 
 func (k *KeyFinderMock) FindForDecryption(ctx context.Context,
-	tlfName string, teamID chat1.TLFID,
-	membersType chat1.ConversationMembersType, public bool,
+	_ string, teamID chat1.TLFID,
+	_ chat1.ConversationMembersType, public bool,
 	keyGeneration int, kbfsEncrypted bool, botUID *gregor1.UID) (res types.CryptKey, err error) {
 	if botUID != nil {
 		return res, fmt.Errorf("TeambotKeys not supported in KeyFinderMock")
@@ -187,20 +187,20 @@ func (k *KeyFinderMock) FindForDecryption(ctx context.Context,
 	return res, NewDecryptionKeyNotFoundError(keyGeneration, public, kbfsEncrypted)
 }
 
-func (k *KeyFinderMock) EphemeralKeyForEncryption(mctx libkb.MetaContext, tlfName string, tlfID chat1.TLFID,
-	membersType chat1.ConversationMembersType, public bool, botUID *gregor1.UID) (types.EphemeralCryptKey, error) {
+func (k *KeyFinderMock) EphemeralKeyForEncryption(_ libkb.MetaContext, _ string, _ chat1.TLFID,
+	_ chat1.ConversationMembersType, _ bool, _ *gregor1.UID) (types.EphemeralCryptKey, error) {
 	panic("unimplemented")
 }
 
-func (k *KeyFinderMock) EphemeralKeyForDecryption(mctx libkb.MetaContext, tlfName string, tlfID chat1.TLFID,
-	membersType chat1.ConversationMembersType, public bool, botUID *gregor1.UID,
-	generation keybase1.EkGeneration, contentCtime *gregor1.Time) (types.EphemeralCryptKey, error) {
+func (k *KeyFinderMock) EphemeralKeyForDecryption(_ libkb.MetaContext, _ string, _ chat1.TLFID,
+	_ chat1.ConversationMembersType, _ bool, _ *gregor1.UID,
+	_ keybase1.EkGeneration, _ *gregor1.Time) (types.EphemeralCryptKey, error) {
 	panic("unimplemented")
 }
 
-func (k *KeyFinderMock) ShouldPairwiseMAC(ctx context.Context, tlfName string, tlfID chat1.TLFID,
-	membersType chat1.ConversationMembersType, public bool) (bool, []keybase1.KID, error) {
+func (k *KeyFinderMock) ShouldPairwiseMAC(_ context.Context, _ string, _ chat1.TLFID,
+	_ chat1.ConversationMembersType, _ bool) (bool, []keybase1.KID, error) {
 	panic("unimplemented")
 }
 
-func (k *KeyFinderMock) SetNameInfoSourceOverride(ni types.NameInfoSource) {}
+func (k *KeyFinderMock) SetNameInfoSourceOverride(_ types.NameInfoSource) {}

go/chat/msg_grouper.go

@@ -58,8 +58,8 @@ type joinLeaveGrouper struct {
 
 var _ msgGrouper = (*joinLeaveGrouper)(nil)
 
-func newJoinLeaveGrouper(g *globals.Context, uid gregor1.UID, convID chat1.ConversationID,
-	dataSource types.InboxSourceDataSourceTyp) *joinLeaveGrouper {
+func newJoinLeaveGrouper(_ *globals.Context, uid gregor1.UID, _ chat1.ConversationID,
+	_ types.InboxSourceDataSourceTyp) *joinLeaveGrouper {
 	return &joinLeaveGrouper{
 		uid: uid,
 	}

go/chat/participantsource.go

@@ -55,7 +55,7 @@ func NewCachingParticipantSource(g *globals.Context, ri func() chat1.RemoteInter
 		return make(map[chat1.ConvIDStr][]chat1.UIParticipant)
 	},
 		200*time.Millisecond, true)
-	g.PushShutdownHook(func(mctx libkb.MetaContext) error {
+	g.PushShutdownHook(func(_ libkb.MetaContext) error {
 		notifyCancel()
 		return nil
 	})