enable linters: misspell, bodyclose (#28736)

Author: zoom-ua

go/.golangci.yml

@@ -43,15 +43,15 @@ linters:
     - ineffassign # Phase 1: Ineffectual assignments
     - unused # Phase 1: Unused code
     - staticcheck # Phase 1: Static analysis
+    - misspell # Phase 2: Spelling checks
+    - bodyclose # Phase 2: HTTP response body checks
 
   disable:
     # Disabled by default in v1, keeping disabled for incremental migration
     - errcheck # Error checking - enabled by default in v2, many unchecked errors
 
     # Additional linters from kbfs-server/keybase examples (disabled for now)
     # These can be enabled incrementally in future work
-    # - bodyclose     # HTTP response body checks
-    # - misspell      # Spelling checks
     # - gosec         # Security checks
     # - unparam       # Unused parameters
 

go/avatars/fullcaching.go

@@ -392,59 +392,60 @@ func (c *FullCachingSource) removeFile(m libkb.MetaContext, ent *lru.DiskLRUEntr
 
 func (c *FullCachingSource) populateCacheWorker(m libkb.MetaContext) {
 	for arg := range c.populateCacheCh {
-		c.debug(m, "populateCacheWorker: fetching: name: %s format: %s url: %s", arg.name,
-			arg.format, arg.url)
-		// Grab image data first
-		url := arg.url.String()
-		resp, err := libkb.ProxyHTTPGet(m.G(), m.G().GetEnv(), url, "FullCachingSource: Avatar")
+		err := c.populateCacheJob(m, arg)
 		if err != nil {
-			c.debug(m, "populateCacheWorker: failed to download avatar: %s", err)
-			continue
-		}
-		// Find any previous path we stored this image at on the disk
-		var previousEntry lruEntry
-		var previousPath string
-		key := c.avatarKey(arg.name, arg.format)
-		found, ent, err := c.diskLRU.Get(m.Ctx(), m.G(), key)
-		if err != nil {
-			c.debug(m, "populateCacheWorker: failed to read previous entry in LRU: %s", err)
-			err = libkb.DiscardAndCloseBody(resp)
-			if err != nil {
-				c.debug(m, "populateCacheWorker: error closing body: %+v", err)
-			}
-			continue
-		}
-		if found {
-			previousEntry = c.processLRUHit(ent)
-			previousPath = c.normalizeFilenameFromCache(m, previousEntry.Path)
+			c.debug(m, "populateCacheWorker: %s", err)
 		}
+	}
+}
 
-		// Save to disk
-		path, err := c.commitAvatarToDisk(m, resp.Body, previousPath)
-		discardErr := libkb.DiscardAndCloseBody(resp)
-		if discardErr != nil {
-			c.debug(m, "populateCacheWorker: error closing body: %+v", discardErr)
-		}
-		if err != nil {
-			c.debug(m, "populateCacheWorker: failed to write to disk: %s", err)
-			continue
-		}
-		v := lruEntry{
-			Path: path,
-			URL:  &url,
-		}
-		evicted, err := c.diskLRU.Put(m.Ctx(), m.G(), key, v)
-		if err != nil {
-			c.debug(m, "populateCacheWorker: failed to put into LRU: %s", err)
-			continue
-		}
-		// Remove any evicted file (if there is one)
-		c.removeFile(m, evicted)
+func (c *FullCachingSource) populateCacheJob(m libkb.MetaContext, arg populateArg) (err error) {
+	c.debug(m, "populateCacheWorker: fetching: name: %s format: %s url: %s", arg.name,
+		arg.format, arg.url)
+	// Grab image data first
+	url := arg.url.String()
+	resp, err := libkb.ProxyHTTPGet(m.G(), m.G().GetEnv(), url, "FullCachingSource: Avatar")
+	defer func() { _ = libkb.DiscardAndCloseBody(resp) }()
+	if err != nil {
+		c.debug(m, "populateCacheWorker: failed to download avatar: %s", err)
+		return err
+	}
+	// Find any previous path we stored this image at on the disk
+	var previousEntry lruEntry
+	var previousPath string
+	key := c.avatarKey(arg.name, arg.format)
+	found, ent, err := c.diskLRU.Get(m.Ctx(), m.G(), key)
+	if err != nil {
+		c.debug(m, "populateCacheWorker: failed to read previous entry in LRU: %s", err)
+		return err
+	}
+	if found {
+		previousEntry = c.processLRUHit(ent)
+		previousPath = c.normalizeFilenameFromCache(m, previousEntry.Path)
+	}
 
-		if c.populateSuccessCh != nil {
-			c.populateSuccessCh <- struct{}{}
-		}
+	// Save to disk
+	path, err := c.commitAvatarToDisk(m, resp.Body, previousPath)
+	if err != nil {
+		c.debug(m, "populateCacheWorker: failed to write to disk: %s", err)
+		return err
 	}
+	v := lruEntry{
+		Path: path,
+		URL:  &url,
+	}
+	evicted, err := c.diskLRU.Put(m.Ctx(), m.G(), key, v)
+	if err != nil {
+		c.debug(m, "populateCacheWorker: failed to put into LRU: %s", err)
+		return err
+	}
+	// Remove any evicted file (if there is one)
+	c.removeFile(m, evicted)
+
+	if c.populateSuccessCh != nil {
+		c.populateSuccessCh <- struct{}{}
+	}
+	return nil
 }
 
 func (c *FullCachingSource) dispatchPopulateFromRes(m libkb.MetaContext, res keybase1.LoadAvatarsRes,

go/avatars/utils.go

@@ -56,6 +56,7 @@ func FetchAvatar(ctx context.Context, g *globals.Context, username string) (res
 			return res, err
 		}
 		if resp.StatusCode >= 400 {
+			resp.Body.Close()
 			avatarReader = getAvatarPlaceholder()
 		} else {
 			avatarReader = resp.Body

go/chat/attachment_httpsrv_test.go

@@ -155,6 +155,7 @@ func TestChatSrvAttachmentHTTPSrv(t *testing.T) {
 	readAsset := func(msg chat1.UIMessage, cacheHit bool) {
 		httpRes, err := http.Get(msg.Valid().AssetUrlInfo.FullUrl)
 		require.NoError(t, err)
+		defer httpRes.Body.Close()
 		body, err := io.ReadAll(httpRes.Body)
 		require.NoError(t, err)
 		require.Equal(t, "HI", string(body))

go/chat/attachments/utils.go

@@ -65,7 +65,7 @@ func AssetFromMessage(ctx context.Context, g *globals.Context, uid gregor1.UID,
 		attachment = chat1.MessageAttachment{
 			Object:   uploaded.Object,
 			Previews: uploaded.Previews,
-			Metadata: uploaded.Metadata,
+			Metadata: uploaded.Metadata, //nolint:govet // keeping metadata for completeness
 		}
 	default:
 		return res, errors.New("not an attachment message")

go/chat/bots/commands.go

@@ -159,7 +159,7 @@ func (b *CachingBotCommandManager) createConv(ctx context.Context, typ chat1.Bot
 		if teamName == nil {
 			return res, errors.New("missing team name")
 		} else if convID != nil {
-			return res, errors.New("convID cannot be specified for team advertisments use type 'conv'")
+			return res, errors.New("convID cannot be specified for team advertisements use type 'conv'")
 		}
 
 		topicName := fmt.Sprintf("___keybase_botcommands_team_%s_%v", username, typ)

go/chat/boxer_test.go

@@ -1068,7 +1068,7 @@ func TestChatMessageDeletedNotSuperseded(t *testing.T) {
 		// The server was not setting supersededBy on EDITs when their TEXT got deleted.
 		// So there are existing messages which have no supersededBy but are legitimately deleted.
 		// Tracked in CORE-4662
-		require.NoError(t, err, "suprisingly, should be able to unbox with deleted but no supersededby")
+		require.NoError(t, err, "surprisingly, should be able to unbox with deleted but no supersededby")
 		require.Equal(t, chat1.MessageBody{}, unboxed.MessageBody)
 	})
 }

go/chat/emojisource.go

@@ -765,7 +765,7 @@ func (s *DevConvEmojiSource) getCrossTeamConv(ctx context.Context, uid gregor1.U
 			s.testingCreatedSyncConv <- struct{}{}
 		}
 	} else {
-		s.Debug(ctx, "getCrossTeamConv: using exising sync conv: %s (topicID: %s)", res.GetConvID(), topicID)
+		s.Debug(ctx, "getCrossTeamConv: using existing sync conv: %s (topicID: %s)", res.GetConvID(), topicID)
 	}
 	return res, nil
 }

go/chat/flip/flip.go

@@ -47,7 +47,7 @@ type GameStateUpdateMessage struct {
 	Result             *Result
 }
 
-// Dealer is a peristent process that runs in the chat client that deals out a game. It can have multiple
+// Dealer is a persistent process that runs in the chat client that deals out a game. It can have multiple
 // games running at once.
 type Dealer struct {
 	sync.Mutex

go/chat/maps/livelocation.go

@@ -55,7 +55,7 @@ func (l *LiveLocationTracker) Start(ctx context.Context, uid gregor1.UID) {
 	l.Lock()
 	defer l.Unlock()
 	l.uid = uid
-	// bring back any trackers that we have stored. This is most relavent when being woken
+	// bring back any trackers that we have stored. This is most relevant when being woken
 	// up on iOS due to a location update. THe app might need to recreate all of its trackers
 	// if the app had been killed.
 	l.restoreLocked(ctx)