*: enable bodyclose linter (tikv#4437)

* enable bodyclose linter ref tikv#4322 Signed-off-by: Ryan Leung <rleungx@gmail.com> * address the comment Signed-off-by: Ryan Leung <rleungx@gmail.com>
JmPotato · Dec 8, 2021 · 4ec9e14 · 4ec9e14
1 parent 2246ef6
commit 4ec9e14
Show file tree

Hide file tree

Showing 14 changed files with 48 additions and 40 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -10,6 +10,7 @@ linters:
     - whitespace
     - makezero
     - gosec
+    - bodyclose
   disable:
     - errcheck
 linters-settings:

diff --git a/server/api/admin_test.go b/server/api/admin_test.go
@@ -75,7 +75,7 @@ func (s *testAdminSuite) TestDropRegion(c *C) {
 
 	// After drop region from cache, lower version is accepted.
 	url := fmt.Sprintf("%s/admin/cache/region/%d", s.urlPrefix, region.GetID())
-	req, err := http.NewRequest("DELETE", url, nil)
+	req, err := http.NewRequest(http.MethodDelete, url, nil)
 	c.Assert(err, IsNil)
 	res, err := testDialClient.Do(req)
 	c.Assert(err, IsNil)

diff --git a/server/api/component_test.go b/server/api/component_test.go
@@ -101,9 +101,9 @@ func (s *testComponentSuite) TestComponent(c *C) {
 
 	// unregister address
 	addr3 := fmt.Sprintf("%s/component/c1/127.0.0.1:1", urlPrefix)
-	res, err := doDelete(testDialClient, addr3)
+	statusCode, err := doDelete(testDialClient, addr3)
 	c.Assert(err, IsNil)
-	c.Assert(res.StatusCode, Equals, 200)
+	c.Assert(statusCode, Equals, 200)
 
 	expected3 := map[string][]string{
 		"c1": {"127.0.0.1:2"},
@@ -116,9 +116,9 @@ func (s *testComponentSuite) TestComponent(c *C) {
 	c.Assert(output, DeepEquals, expected3)
 
 	addr4 := fmt.Sprintf("%s/component/c1/127.0.0.1:2", urlPrefix)
-	res, err = doDelete(testDialClient, addr4)
+	statusCode, err = doDelete(testDialClient, addr4)
 	c.Assert(err, IsNil)
-	c.Assert(res.StatusCode, Equals, 200)
+	c.Assert(statusCode, Equals, 200)
 	expected4 := map[string][]string{
 		"c2": {"127.0.0.1:3"},
 		"c3": {"example.com"},

diff --git a/server/api/member_test.go b/server/api/member_test.go
@@ -142,7 +142,7 @@ func changeLeaderPeerUrls(c *C, leader *pdpb.Member, id uint64, urls []string) {
 	data := map[string][]string{"peerURLs": urls}
 	postData, err := json.Marshal(data)
 	c.Assert(err, IsNil)
-	req, err := http.NewRequest("PUT", fmt.Sprintf("%s/v2/members/%s", leader.GetClientUrls()[0], fmt.Sprintf("%x", id)), bytes.NewBuffer(postData))
+	req, err := http.NewRequest(http.MethodPut, fmt.Sprintf("%s/v2/members/%s", leader.GetClientUrls()[0], fmt.Sprintf("%x", id)), bytes.NewBuffer(postData))
 	c.Assert(err, IsNil)
 	req.Header.Set("Content-Type", "application/json")
 	resp, err := testDialClient.Do(req)

diff --git a/server/api/rule_test.go b/server/api/rule_test.go
@@ -521,9 +521,9 @@ func (s *testRuleSuite) TestDelete(c *C) {
 		url := fmt.Sprintf("%s/rule/%s/%s", s.urlPrefix, testcase.groupID, testcase.id)
 		// clear suspect keyRanges to prevent test case from others
 		s.svr.GetRaftCluster().ClearSuspectKeyRanges()
-		resp, err := doDelete(testDialClient, url)
+		statusCode, err := doDelete(testDialClient, url)
 		c.Assert(err, IsNil)
-		c.Assert(resp.StatusCode, Equals, http.StatusOK)
+		c.Assert(statusCode, Equals, http.StatusOK)
 		if len(testcase.popKeyRange) > 0 {
 			popKeyRangeMap := map[string]struct{}{}
 			for i := 0; i < len(testcase.popKeyRange)/2; i++ {

diff --git a/server/api/scheduler.go b/server/api/scheduler.go
@@ -301,12 +301,12 @@ func (h *schedulerHandler) redirectSchedulerDelete(w http.ResponseWriter, name,
 	args := strings.Split(name, "-")
 	args = args[len(args)-1:]
 	url := fmt.Sprintf("%s/%s/%s/delete/%s", h.GetAddr(), schedulerConfigPrefix, schedulerName, args[0])
-	resp, err := doDelete(h.svr.GetHTTPClient(), url)
+	statusCode, err := doDelete(h.svr.GetHTTPClient(), url)
 	if err != nil {
-		h.r.JSON(w, resp.StatusCode, err.Error())
+		h.r.JSON(w, statusCode, err.Error())
 		return
 	}
-	h.r.JSON(w, resp.StatusCode, nil)
+	h.r.JSON(w, statusCode, nil)
 }
 
 // FIXME: details of input json body params

diff --git a/server/api/scheduler_test.go b/server/api/scheduler_test.go
@@ -91,20 +91,20 @@ func (s *testScheduleSuite) TestOriginAPI(c *C) {
 	c.Assert(resp1["store-id-ranges"], HasLen, 1)
 	deleteURL = fmt.Sprintf("%s/%s", s.urlPrefix, "evict-leader-scheduler-2")
 	c.Assert(failpoint.Enable("github.com/tikv/pd/server/config/persistFail", "return(true)"), IsNil)
-	res, err := doDelete(testDialClient, deleteURL)
+	statusCode, err := doDelete(testDialClient, deleteURL)
 	c.Assert(err, IsNil)
-	c.Assert(res.StatusCode, Equals, 500)
+	c.Assert(statusCode, Equals, 500)
 	c.Assert(rc.GetSchedulers(), HasLen, 1)
 	c.Assert(failpoint.Disable("github.com/tikv/pd/server/config/persistFail"), IsNil)
-	res, err = doDelete(testDialClient, deleteURL)
+	statusCode, err = doDelete(testDialClient, deleteURL)
 	c.Assert(err, IsNil)
-	c.Assert(res.StatusCode, Equals, 200)
+	c.Assert(statusCode, Equals, 200)
 	c.Assert(rc.GetSchedulers(), HasLen, 0)
 	resp2 := make(map[string]interface{})
 	c.Assert(readJSON(testDialClient, listURL, &resp2), NotNil)
 
-	r, _ := doDelete(testDialClient, deleteURL)
-	c.Assert(r.StatusCode, Equals, 404)
+	statusCode, _ = doDelete(testDialClient, deleteURL)
+	c.Assert(statusCode, Equals, 404)
 }
 
 func (s *testScheduleSuite) TestAPI(c *C) {
@@ -196,9 +196,9 @@ func (s *testScheduleSuite) TestAPI(c *C) {
 				c.Assert(readJSON(testDialClient, listURL, &resp), IsNil)
 				delete(exceptMap, "2")
 				c.Assert(resp["store-id-ranges"], DeepEquals, exceptMap)
-				res, err := doDelete(testDialClient, deleteURL)
+				statusCode, err := doDelete(testDialClient, deleteURL)
 				c.Assert(err, IsNil)
-				c.Assert(res.StatusCode, Equals, 404)
+				c.Assert(statusCode, Equals, 404)
 			},
 		},
 		{
@@ -260,9 +260,9 @@ func (s *testScheduleSuite) TestAPI(c *C) {
 				c.Assert(readJSON(testDialClient, listURL, &resp), IsNil)
 				delete(exceptMap, "2")
 				c.Assert(resp["store-id-ranges"], DeepEquals, exceptMap)
-				res, err := doDelete(testDialClient, deleteURL)
+				statusCode, err := doDelete(testDialClient, deleteURL)
 				c.Assert(err, IsNil)
-				c.Assert(res.StatusCode, Equals, 404)
+				c.Assert(statusCode, Equals, 404)
 			},
 		},
 	}

diff --git a/server/api/service_gc_safepoint_test.go b/server/api/service_gc_safepoint_test.go
@@ -81,14 +81,15 @@ func (s *testServiceGCSafepointSuite) TestRegionStats(c *C) {
 
 	res, err := testDialClient.Get(sspURL)
 	c.Assert(err, IsNil)
+	defer res.Body.Close()
 	listResp := &listServiceGCSafepoint{}
 	err = apiutil.ReadJSON(res.Body, listResp)
 	c.Assert(err, IsNil)
 	c.Assert(listResp, DeepEquals, list)
 
-	res, err = doDelete(testDialClient, sspURL+"/a")
+	statusCode, err := doDelete(testDialClient, sspURL+"/a")
 	c.Assert(err, IsNil)
-	c.Assert(res.StatusCode, Equals, http.StatusOK)
+	c.Assert(statusCode, Equals, http.StatusOK)
 
 	left, err := storage.GetAllServiceGCSafePoints()
 	c.Assert(err, IsNil)

diff --git a/server/api/store_test.go b/server/api/store_test.go
@@ -202,8 +202,11 @@ func (s *testStoreSuite) TestStoreInfoGet(c *C) {
 	c.Assert(info.Store.DeployPath, Equals, "/home/test")
 	c.Assert(info.Store.LastHeartbeat, Equals, timeStamp)
 
-	resp, _ := testDialClient.Get(url)
-	b, _ := io.ReadAll(resp.Body)
+	resp, err := testDialClient.Get(url)
+	c.Assert(err, IsNil)
+	defer resp.Body.Close()
+	b, err := io.ReadAll(resp.Body)
+	c.Assert(err, IsNil)
 	str := string(b)
 	c.Assert(strings.Contains(str, "\"state\""), Equals, false)
 	s.cleanup()

diff --git a/server/api/util.go b/server/api/util.go
@@ -64,7 +64,7 @@ func readJSON(client *http.Client, url string, data interface{}) error {
 }
 
 func readJSONWithBody(client *http.Client, url string, body []byte, data interface{}) error {
-	req, err := http.NewRequest("GET", url, bytes.NewBuffer(body))
+	req, err := http.NewRequest(http.MethodGet, url, bytes.NewBuffer(body))
 	if err != nil {
 		return err
 	}
@@ -91,7 +91,7 @@ func extractJSON(resp *http.Response, data interface{}) error {
 }
 
 func postJSON(client *http.Client, url string, data []byte, checkOpts ...func([]byte, int)) error {
-	req, err := http.NewRequest("POST", url, bytes.NewBuffer(data))
+	req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(data))
 	if err != nil {
 		return err
 	}
@@ -100,15 +100,15 @@ func postJSON(client *http.Client, url string, data []byte, checkOpts ...func([]
 }
 
 func getJSON(client *http.Client, url string, data []byte, checkOpts ...func([]byte, int)) error {
-	req, err := http.NewRequest("GET", url, bytes.NewBuffer(data))
+	req, err := http.NewRequest(http.MethodGet, url, bytes.NewBuffer(data))
 	if err != nil {
 		return err
 	}
 	return doJSON(client, req, checkOpts...)
 }
 
 func patchJSON(client *http.Client, url string, body []byte) error {
-	req, err := http.NewRequest("PATCH", url, bytes.NewBuffer(body))
+	req, err := http.NewRequest(http.MethodPatch, url, bytes.NewBuffer(body))
 	if err != nil {
 		return err
 	}
@@ -127,17 +127,17 @@ func patchJSON(client *http.Client, url string, body []byte) error {
 	return nil
 }
 
-func doDelete(client *http.Client, url string) (*http.Response, error) {
-	req, err := http.NewRequest("DELETE", url, nil)
+func doDelete(client *http.Client, url string) (int, error) {
+	req, err := http.NewRequest(http.MethodDelete, url, nil)
 	if err != nil {
-		return nil, err
+		return http.StatusBadRequest, err
 	}
 	res, err := client.Do(req)
 	if err != nil {
-		return nil, err
+		return 0, err
 	}
-	res.Body.Close()
-	return res, nil
+	defer res.Body.Close()
+	return res.StatusCode, nil
 }
 
 func parseKey(name string, input map[string]interface{}) ([]byte, string, error) {

diff --git a/server/cluster/cluster.go b/server/cluster/cluster.go
@@ -1826,7 +1826,7 @@ func CheckHealth(client *http.Client, members []*pdpb.Member) map[uint64]*pdpb.M
 	for _, member := range members {
 		for _, cURL := range member.ClientUrls {
 			ctx, cancel := context.WithTimeout(context.Background(), clientTimeout)
-			req, err := http.NewRequestWithContext(ctx, "GET", fmt.Sprintf("%s%s", cURL, healthURL), nil)
+			req, err := http.NewRequestWithContext(ctx, http.MethodGet, fmt.Sprintf("%s%s", cURL, healthURL), nil)
 			if err != nil {
 				log.Error("failed to new request", errs.ZapError(errs.ErrNewHTTPRequest, err))
 				cancel()

diff --git a/server/server.go b/server/server.go
@@ -1390,7 +1390,7 @@ func (s *Server) replicateFileToMember(ctx context.Context, member *pdpb.Member,
 		return errs.ErrClientURLEmpty.FastGenByArgs()
 	}
 	url := clientUrls[0] + filepath.Join("/pd/api/v1/admin/persist-file", name)
-	req, _ := http.NewRequestWithContext(ctx, "POST", url, bytes.NewBuffer(data))
+	req, _ := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewBuffer(data))
 	req.Header.Set("PD-Allow-follower-handle", "true")
 	res, err := s.httpClient.Do(req)
 	if err != nil {

diff --git a/tests/server/api/api_test.go b/tests/server/api/api_test.go
@@ -70,6 +70,7 @@ func (s *serverTestSuite) TestReconnect(c *C) {
 		if name != leader {
 			res, e := http.Get(s.GetConfig().AdvertiseClientUrls + "/pd/api/v1/version")
 			c.Assert(e, IsNil)
+			res.Body.Close()
 			c.Assert(res.StatusCode, Equals, http.StatusOK)
 		}
 	}
@@ -86,6 +87,7 @@ func (s *serverTestSuite) TestReconnect(c *C) {
 			testutil.WaitUntil(c, func(c *C) bool {
 				res, e := http.Get(s.GetConfig().AdvertiseClientUrls + "/pd/api/v1/version")
 				c.Assert(e, IsNil)
+				defer res.Body.Close()
 				return res.StatusCode == http.StatusOK
 			})
 		}
@@ -101,6 +103,7 @@ func (s *serverTestSuite) TestReconnect(c *C) {
 			testutil.WaitUntil(c, func(c *C) bool {
 				res, err := http.Get(s.GetConfig().AdvertiseClientUrls + "/pd/api/v1/version")
 				c.Assert(err, IsNil)
+				defer res.Body.Close()
 				return res.StatusCode == http.StatusServiceUnavailable
 			})
 		}
@@ -159,7 +162,7 @@ func (s *testRedirectorSuite) TestAllowFollowerHandle(c *C) {
 	}
 
 	addr := follower.GetAddr() + "/pd/api/v1/version"
-	request, err := http.NewRequest("GET", addr, nil)
+	request, err := http.NewRequest(http.MethodGet, addr, nil)
 	c.Assert(err, IsNil)
 	request.Header.Add(serverapi.AllowFollowerHandle, "true")
 	resp, err := dialClient.Do(request)
@@ -184,7 +187,7 @@ func (s *testRedirectorSuite) TestNotLeader(c *C) {
 
 	addr := follower.GetAddr() + "/pd/api/v1/version"
 	// Request to follower without redirectorHeader is OK.
-	request, err := http.NewRequest("GET", addr, nil)
+	request, err := http.NewRequest(http.MethodGet, addr, nil)
 	c.Assert(err, IsNil)
 	resp, err := dialClient.Do(request)
 	c.Assert(err, IsNil)

diff --git a/tests/server/member/member_test.go b/tests/server/member/member_test.go
@@ -113,7 +113,7 @@ func (s *memberTestSuite) TestMemberDelete(c *C) {
 		c.Log(time.Now(), "try to delete:", t.path)
 		testutil.WaitUntil(c, func(c *C) bool {
 			addr := leader.GetConfig().ClientUrls + "/pd/api/v1/members/" + t.path
-			req, err := http.NewRequest("DELETE", addr, nil)
+			req, err := http.NewRequest(http.MethodDelete, addr, nil)
 			c.Assert(err, IsNil)
 			res, err := httpClient.Do(req)
 			c.Assert(err, IsNil)