Prefer %v over %d,%s and add fmtpercentv custom linter (#3756)

Author: gmlewis

.custom-gcl.yml

@@ -1,4 +1,6 @@
 version: v2.2.2
 plugins:
-  - module: "github.com/google/go-github/v68/tools/sliceofpointers"
+  - module: "github.com/google/go-github/v75/tools/sliceofpointers"
     path: ./tools/sliceofpointers
+  - module: "github.com/google/go-github/v75/tools/fmtpercentv"
+    path: ./tools/fmtpercentv

.golangci.yml

@@ -9,6 +9,7 @@ linters:
     - dogsled
     - dupl
     - errorlint
+    - fmtpercentv
     - forbidigo
     - gocritic
     - godot
@@ -136,10 +137,14 @@ linters:
       os-create-temp: true
       os-temp-dir: true
     custom:
+      fmtpercentv:
+        type: module
+        description: Reports usage of %d or %s in format strings.
+        original-url: github.com/google/go-github/v75/tools/fmtpercentv
       sliceofpointers:
         type: module
         description: Reports usage of []*string and slices of structs without pointers.
-        original-url: github.com/google/go-github/v68/tools/sliceofpointers
+        original-url: github.com/google/go-github/v75/tools/sliceofpointers
   exclusions:
     rules:
       - linters:

README.md

@@ -215,7 +215,7 @@ To detect a primary API rate limit error, you can check if the error is a
 repos, _, err := client.Repositories.List(ctx, "", nil)
 var rateErr *github.RateLimitError
 if errors.As(err, &rateError) {
-	log.Printf("hit primary rate limit, used %d of %d\n", rateErr.Rate.Used, rateErr.rate.Limit)
+	log.Printf("hit primary rate limit, used %v of %v\n", rateErr.Rate.Used, rateErr.rate.Limit)
 }
 ```
 

example/actionpermissions/main.go

@@ -42,36 +42,36 @@ func main() {
 		log.Fatal(err)
 	}
 
-	fmt.Printf("Current ActionsPermissions %s\n", actionsPermissionsRepository.String())
+	fmt.Printf("Current ActionsPermissions %v\n", actionsPermissionsRepository)
 
 	actionsPermissionsRepository = &github.ActionsPermissionsRepository{Enabled: github.Ptr(true), AllowedActions: github.Ptr("selected")}
 	_, _, err = client.Repositories.UpdateActionsPermissions(ctx, *owner, *name, *actionsPermissionsRepository)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	fmt.Printf("Current ActionsPermissions %s\n", actionsPermissionsRepository.String())
+	fmt.Printf("Current ActionsPermissions %v\n", actionsPermissionsRepository)
 
 	actionsAllowed, _, err := client.Repositories.GetActionsAllowed(ctx, *owner, *name)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	fmt.Printf("Current ActionsAllowed %s\n", actionsAllowed.String())
+	fmt.Printf("Current ActionsAllowed %v\n", actionsAllowed)
 
 	actionsAllowed = &github.ActionsAllowed{GithubOwnedAllowed: github.Ptr(true), VerifiedAllowed: github.Ptr(false), PatternsAllowed: []string{"a/b"}}
 	_, _, err = client.Repositories.EditActionsAllowed(ctx, *owner, *name, *actionsAllowed)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	fmt.Printf("Current ActionsAllowed %s\n", actionsAllowed.String())
+	fmt.Printf("Current ActionsAllowed %v\n", actionsAllowed)
 
 	actionsPermissionsRepository = &github.ActionsPermissionsRepository{Enabled: github.Ptr(true), AllowedActions: github.Ptr("all")}
 	_, _, err = client.Repositories.UpdateActionsPermissions(ctx, *owner, *name, *actionsPermissionsRepository)
 	if err != nil {
 		log.Fatal(err)
 	}
 
-	fmt.Printf("Current ActionsPermissions %s\n", actionsPermissionsRepository.String())
+	fmt.Printf("Current ActionsPermissions %v\n", actionsPermissionsRepository)
 }

example/commitpr/main.go

@@ -185,7 +185,7 @@ func createPR() (err error) {
 	}
 
 	if *prRepoOwner != "" && *prRepoOwner != *sourceOwner {
-		*commitBranch = fmt.Sprintf("%s:%s", *sourceOwner, *commitBranch)
+		*commitBranch = fmt.Sprintf("%v:%v", *sourceOwner, *commitBranch)
 	} else {
 		prRepoOwner = sourceOwner
 	}
@@ -208,7 +208,7 @@ func createPR() (err error) {
 		return err
 	}
 
-	fmt.Printf("PR created: %s\n", pr.GetHTMLURL())
+	fmt.Printf("PR created: %v\n", pr.GetHTMLURL())
 	return nil
 }
 
@@ -225,22 +225,22 @@ func main() {
 
 	ref, err := getRef()
 	if err != nil {
-		log.Fatalf("Unable to get/create the commit reference: %s\n", err)
+		log.Fatalf("Unable to get/create the commit reference: %v\n", err)
 	}
 	if ref == nil {
 		log.Fatal("No error where returned but the reference is nil")
 	}
 
 	tree, err := getTree(ref)
 	if err != nil {
-		log.Fatalf("Unable to create the tree based on the provided files: %s\n", err)
+		log.Fatalf("Unable to create the tree based on the provided files: %v\n", err)
 	}
 
 	if err := pushCommit(ref, tree); err != nil {
-		log.Fatalf("Unable to create the commit: %s\n", err)
+		log.Fatalf("Unable to create the commit: %v\n", err)
 	}
 
 	if err := createPR(); err != nil {
-		log.Fatalf("Error while creating the pull request: %s", err)
+		log.Fatalf("Error while creating the pull request: %v", err)
 	}
 }

example/listenvironments/main.go

@@ -46,5 +46,5 @@ func main() {
 	}
 
 	// The number of environments here should be equal to expectedPageSize
-	fmt.Printf("%d environments returned\n", len(envResponse.Environments))
+	fmt.Printf("%v environments returned\n", len(envResponse.Environments))
 }

example/ratelimit/main.go

@@ -27,7 +27,7 @@ func main() {
 
 	rateLimiter := github_ratelimit.New(nil,
 		github_primary_ratelimit.WithLimitDetectedCallback(func(ctx *github_primary_ratelimit.CallbackContext) {
-			fmt.Printf("Primary rate limit detected: category %s, reset time: %v\n", ctx.Category, ctx.ResetTime)
+			fmt.Printf("Primary rate limit detected: category %v, reset time: %v\n", ctx.Category, ctx.ResetTime)
 		}),
 		github_secondary_ratelimit.WithLimitDetectedCallback(func(ctx *github_secondary_ratelimit.CallbackContext) {
 			fmt.Printf("Secondary rate limit detected: reset time: %v, total sleep time: %v\n", ctx.ResetTime, ctx.TotalSleepTime)

example/verifyartifact/main.go

@@ -49,13 +49,13 @@ var (
 
 func usage() {
 	fmt.Fprintln(os.Stderr, "This is an example of how to verify the provenance of an artifact using GitHub Attestations and the sigstore-go library.")
-	fmt.Fprintf(os.Stderr, "\nUsage: %s [flags]\n", os.Args[0])
+	fmt.Fprintf(os.Stderr, "\nUsage: %v [flags]\n", os.Args[0])
 	fmt.Fprint(os.Stderr, "\nThe flags are:\n")
 	flag.PrintDefaults()
 	fmt.Fprintf(os.Stderr, `
 Example:
 Verifying a GitHub CLI artifact
-	%s -owner cli \
+	%v -owner cli \
 		-artifact-digest 2ce2e480e3c3f7ca0af83418d3ebaeedacee135dbac94bd946d7d84edabcdb64 \
 		-expected-san https://github.com/cli/cli/.github/workflows/deployment.yml@refs/heads/trunk
 
@@ -118,7 +118,7 @@ func main() {
 func getTrustedMaterial() (root.TrustedMaterialCollection, error) {
 	trustedRootJSON, err := os.ReadFile(*trustedRootJSONPath)
 	if err != nil {
-		return nil, fmt.Errorf("failed to read %s: %w", *trustedRootJSONPath, err)
+		return nil, fmt.Errorf("failed to read %v: %w", *trustedRootJSONPath, err)
 	}
 
 	trustedRoot, err := root.NewTrustedRootFromJSON(trustedRootJSON)

github/actions_artifacts_test.go

@@ -94,7 +94,7 @@ func TestActionsService_ListArtifacts_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.ListArtifacts return status %d, want %d", got, want)
+		t.Errorf("Actions.ListArtifacts return status %v, want %v", got, want)
 	}
 	if artifacts != nil {
 		t.Errorf("Actions.ListArtifacts return %+v, want nil", artifacts)
@@ -176,7 +176,7 @@ func TestActionsService_ListWorkflowRunArtifacts_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.ListWorkflowRunArtifacts return status %d, want %d", got, want)
+		t.Errorf("Actions.ListWorkflowRunArtifacts return status %v, want %v", got, want)
 	}
 	if artifacts != nil {
 		t.Errorf("Actions.ListWorkflowRunArtifacts return %+v, want nil", artifacts)
@@ -263,7 +263,7 @@ func TestActionsService_GetArtifact_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.GetArtifact return status %d, want %d", got, want)
+		t.Errorf("Actions.GetArtifact return status %v, want %v", got, want)
 	}
 	if artifact != nil {
 		t.Errorf("Actions.GetArtifact return %+v, want nil", artifact)
@@ -304,12 +304,12 @@ func TestActionsService_DownloadArtifact(t *testing.T) {
 				t.Errorf("Actions.DownloadArtifact returned error: %v", err)
 			}
 			if resp.StatusCode != http.StatusFound {
-				t.Errorf("Actions.DownloadArtifact returned status: %d, want %d", resp.StatusCode, http.StatusFound)
+				t.Errorf("Actions.DownloadArtifact returned status: %v, want %v", resp.StatusCode, http.StatusFound)
 			}
 
 			want := "https://github.com/artifact"
 			if url.String() != want {
-				t.Errorf("Actions.DownloadArtifact returned %+v, want %+v", url.String(), want)
+				t.Errorf("Actions.DownloadArtifact returned %+v, want %+v", url, want)
 			}
 
 			const methodName = "DownloadArtifact"
@@ -420,7 +420,7 @@ func TestActionsService_DownloadArtifact_StatusMovedPermanently_dontFollowRedire
 			ctx := t.Context()
 			_, resp, _ := client.Actions.DownloadArtifact(ctx, "o", "r", 1, 0)
 			if resp.StatusCode != http.StatusMovedPermanently {
-				t.Errorf("Actions.DownloadArtifact return status %d, want %d", resp.StatusCode, http.StatusMovedPermanently)
+				t.Errorf("Actions.DownloadArtifact return status %v, want %v", resp.StatusCode, http.StatusMovedPermanently)
 			}
 		})
 	}
@@ -464,11 +464,11 @@ func TestActionsService_DownloadArtifact_StatusMovedPermanently_followRedirects(
 				t.Errorf("Actions.DownloadArtifact return error: %v", err)
 			}
 			if resp.StatusCode != http.StatusFound {
-				t.Errorf("Actions.DownloadArtifact return status %d, want %d", resp.StatusCode, http.StatusFound)
+				t.Errorf("Actions.DownloadArtifact return status %v, want %v", resp.StatusCode, http.StatusFound)
 			}
 			want := "https://github.com/artifact"
 			if url.String() != want {
-				t.Errorf("Actions.DownloadArtifact returned %+v, want %+v", url.String(), want)
+				t.Errorf("Actions.DownloadArtifact returned %+v, want %+v", url, want)
 			}
 		})
 	}
@@ -515,7 +515,7 @@ func TestActionsService_DownloadArtifact_unexpectedCode(t *testing.T) {
 				t.Error("Actions.DownloadArtifact should return unexpected status code")
 			}
 			if got, want := resp.Response.StatusCode, http.StatusNoContent; got != want {
-				t.Errorf("Actions.DownloadArtifact return status %d, want %d", got, want)
+				t.Errorf("Actions.DownloadArtifact return status %v, want %v", got, want)
 			}
 			if url != nil {
 				t.Errorf("Actions.DownloadArtifact return %+v, want nil", url)
@@ -582,7 +582,7 @@ func TestActionsService_DeleteArtifact_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.DeleteArtifact return status %d, want %d", got, want)
+		t.Errorf("Actions.DeleteArtifact return status %v, want %v", got, want)
 	}
 }
 

github/actions_cache_test.go

@@ -88,7 +88,7 @@ func TestActionsService_ListCaches_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.ListCaches return status %d, want %d", got, want)
+		t.Errorf("Actions.ListCaches return status %v, want %v", got, want)
 	}
 	if caches != nil {
 		t.Errorf("Actions.ListCaches return %+v, want nil", caches)
@@ -154,7 +154,7 @@ func TestActionsService_DeleteCachesByKey_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.DeleteCachesByKey return status %d, want %d", got, want)
+		t.Errorf("Actions.DeleteCachesByKey return status %v, want %v", got, want)
 	}
 }
 
@@ -216,7 +216,7 @@ func TestActionsService_DeleteCachesByID_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.DeleteCachesByID return status %d, want %d", got, want)
+		t.Errorf("Actions.DeleteCachesByID return status %v, want %v", got, want)
 	}
 }
 
@@ -294,7 +294,7 @@ func TestActionsService_GetCacheUsageForRepo_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.GetCacheUsageForRepo return status %d, want %d", got, want)
+		t.Errorf("Actions.GetCacheUsageForRepo return status %v, want %v", got, want)
 	}
 	if caches != nil {
 		t.Errorf("Actions.GetCacheUsageForRepo return %+v, want nil", caches)
@@ -367,7 +367,7 @@ func TestActionsService_ListCacheUsageByRepoForOrg_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.ListCacheUsageByRepoForOrg return status %d, want %d", got, want)
+		t.Errorf("Actions.ListCacheUsageByRepoForOrg return status %v, want %v", got, want)
 	}
 	if caches != nil {
 		t.Errorf("Actions.ListCacheUsageByRepoForOrg return %+v, want nil", caches)
@@ -438,7 +438,7 @@ func TestActionsService_GetCacheUsageForOrg_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.GetTotalCacheUsageForOrg return status %d, want %d", got, want)
+		t.Errorf("Actions.GetTotalCacheUsageForOrg return status %v, want %v", got, want)
 	}
 	if caches != nil {
 		t.Errorf("Actions.GetTotalCacheUsageForOrg return %+v, want nil", caches)
@@ -509,7 +509,7 @@ func TestActionsService_GetCacheUsageForEnterprise_notFound(t *testing.T) {
 		t.Error("Expected HTTP 404 response")
 	}
 	if got, want := resp.Response.StatusCode, http.StatusNotFound; got != want {
-		t.Errorf("Actions.GetTotalCacheUsageForEnterprise return status %d, want %d", got, want)
+		t.Errorf("Actions.GetTotalCacheUsageForEnterprise return status %v, want %v", got, want)
 	}
 	if caches != nil {
 		t.Errorf("Actions.GetTotalCacheUsageForEnterprise return %+v, want nil", caches)