Repository: google/go-github
Pull request #3070 (2024-02-09 22:44:38)

Turn `RequiredStatusChecks` `Checks` and `Contexts` into pointers
By making the both `Contexts` and `Checks` pointers, the JSON will allow sending an empty slice as `[]` to the GitHub API, instead of taking those out and sending `nil` or `null`.

This will actually solve both https://github.com/google/go-github/issues/2976 and https://github.com/google/go-github/issues/2467.

I also clarified in the comments that empty slices are allowed, implying that if the user wants to set no checks or contexts, to allow passing an empty slice.

These changes allow the Go API to properly reflect the behavior of the GitHub API.

google-cla[bot]: Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this [failed invocation](https://github.com/google/go-github/pull/3070/checks?check_run_id=21425849507) of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/google/go-github/pull/3070?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`2b8c7fa`)](https://app.codecov.io/gh/google/go-github/commit/2b8c7fa86fcd1cb4a89a44c8e7b5a49fe4be8a45?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) 97.72% compared to head [(`4741b40`)](https://app.codecov.io/gh/google/go-github/pull/3070?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google) 98.00%.
> Report is 4 commits behind head on master.


<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #3070      +/-   ##
==========================================
+ Coverage   97.72%   98.00%   +0.27%     
==========================================
  Files         153      153              
  Lines       13390    10803    -2587     
==========================================
- Hits        13085    10587    -2498     
+ Misses        215      126      -89     
  Partials       90       90              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/google/go-github/pull/3070?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=google).

gmlewis: Thank you, @valbeat !
Merging.

Patch:
diff --git a/github/github-accessors.go b/github/github-accessors.go
index e28fd275..b645390e 100644
--- a/github/github-accessors.go
+++ b/github/github-accessors.go
@@ -20246,6 +20246,22 @@ func (r *RequiredStatusCheck) GetAppID() int64 {
 	return *r.AppID
 }
 
+// GetChecks returns the Checks field if it's non-nil, zero value otherwise.
+func (r *RequiredStatusChecks) GetChecks() []*RequiredStatusCheck {
+	if r == nil || r.Checks == nil {
+		return nil
+	}
+	return *r.Checks
+}
+
+// GetContexts returns the Contexts field if it's non-nil, zero value otherwise.
+func (r *RequiredStatusChecks) GetContexts() []string {
+	if r == nil || r.Contexts == nil {
+		return nil
+	}
+	return *r.Contexts
+}
+
 // GetContextsURL returns the ContextsURL field if it's non-nil, zero value otherwise.
 func (r *RequiredStatusChecks) GetContextsURL() string {
 	if r == nil || r.ContextsURL == nil {
diff --git a/github/repos.go b/github/repos.go
index b492e55b..2fb4c6f1 100644
--- a/github/repos.go
+++ b/github/repos.go
@@ -1191,20 +1191,20 @@ type RequiredStatusChecks struct {
 	// Require branches to be up to date before merging. (Required.)
 	Strict bool `json:"strict"`
 	// The list of status checks to require in order to merge into this
-	// branch. (Deprecated. Note: only one of Contexts/Checks can be populated,
-	// but at least one must be populated).
-	Contexts []string `json:"contexts,omitempty"`
+	// branch. An empty slice is valid. (Deprecated. Note: only one of
+	// Contexts/Checks can be populated, but at least one must be populated).
+	Contexts *[]string `json:"contexts,omitempty"`
 	// The list of status checks to require in order to merge into this
-	// branch.
-	Checks      []*RequiredStatusCheck `json:"checks,omitempty"`
-	ContextsURL *string                `json:"contexts_url,omitempty"`
-	URL         *string                `json:"url,omitempty"`
+	// branch. An empty slice is valid.
+	Checks      *[]*RequiredStatusCheck `json:"checks,omitempty"`
+	ContextsURL *string                 `json:"contexts_url,omitempty"`
+	URL         *string                 `json:"url,omitempty"`
 }
 
 // RequiredStatusChecksRequest represents a request to edit a protected branch's status checks.
 type RequiredStatusChecksRequest struct {
 	Strict *bool `json:"strict,omitempty"`
-	// Note: if both Contexts and Checks are populated,
+	// Deprecated. Note: if both Contexts and Checks are populated,
 	// the GitHub API will only use Checks.
 	Contexts []string               `json:"contexts,omitempty"`
 	Checks   []*RequiredStatusCheck `json:"checks,omitempty"`


Test patch:
diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go
index 4c349a05..8d5b4380 100644
--- a/github/github-accessors_test.go
+++ b/github/github-accessors_test.go
@@ -23517,6 +23517,26 @@ func TestRequiredStatusCheck_GetAppID(tt *testing.T) {
 	r.GetAppID()
 }
 
+func TestRequiredStatusChecks_GetChecks(tt *testing.T) {
+	var zeroValue []*RequiredStatusCheck
+	r := &RequiredStatusChecks{Checks: &zeroValue}
+	r.GetChecks()
+	r = &RequiredStatusChecks{}
+	r.GetChecks()
+	r = nil
+	r.GetChecks()
+}
+
+func TestRequiredStatusChecks_GetContexts(tt *testing.T) {
+	var zeroValue []string
+	r := &RequiredStatusChecks{Contexts: &zeroValue}
+	r.GetContexts()
+	r = &RequiredStatusChecks{}
+	r.GetContexts()
+	r = nil
+	r.GetContexts()
+}
+
 func TestRequiredStatusChecks_GetContextsURL(tt *testing.T) {
 	var zeroValue string
 	r := &RequiredStatusChecks{ContextsURL: &zeroValue}
diff --git a/github/repos_test.go b/github/repos_test.go
index 2e61aeb1..0ea381eb 100644
--- a/github/repos_test.go
+++ b/github/repos_test.go
@@ -1207,8 +1207,8 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) {
 			want := &Protection{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict:   true,
-					Contexts: []string{"continuous-integration"},
-					Checks: []*RequiredStatusCheck{
+					Contexts: &[]string{"continuous-integration"},
+					Checks: &[]*RequiredStatusCheck{
 						{
 							Context: "continuous-integration",
 						},
@@ -1334,8 +1334,8 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test
 		want := &Protection{
 			RequiredStatusChecks: &RequiredStatusChecks{
 				Strict:   true,
-				Contexts: []string{"continuous-integration"},
-				Checks: []*RequiredStatusCheck{
+				Contexts: &[]string{"continuous-integration"},
+				Checks: &[]*RequiredStatusCheck{
 					{
 						Context: "continuous-integration",
 					},
@@ -1421,7 +1421,7 @@ func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) {
 			input := &ProtectionRequest{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict:   true,
-					Contexts: []string{"continuous-integration"},
+					Contexts: &[]string{"continuous-integration"},
 				},
 				RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{
 					DismissStaleReviews: true,
@@ -1517,8 +1517,8 @@ func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) {
 			want := &Protection{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict:   true,
-					Contexts: []string{"continuous-integration"},
-					Checks: []*RequiredStatusCheck{
+					Contexts: &[]string{"continuous-integration"},
+					Checks: &[]*RequiredStatusCheck{
 						{
 							Context: "continuous-integration",
 						},
@@ -1592,6 +1592,184 @@ func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) {
 	}
 }
 
+func TestRepositoriesService_UpdateBranchProtection_EmptyContexts(t *testing.T) {
+	tests := []struct {
+		branch  string
+		urlPath string
+	}{
+		{branch: "b", urlPath: "/repos/o/r/branches/b/protection"},
+		{branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.branch, func(t *testing.T) {
+			client, mux, _, teardown := setup()
+			defer teardown()
+
+			input := &ProtectionRequest{
+				RequiredStatusChecks: &RequiredStatusChecks{
+					Strict:   true,
+					Contexts: &[]string{},
+				},
+				RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{
+					DismissStaleReviews: true,
+					DismissalRestrictionsRequest: &DismissalRestrictionsRequest{
+						Users: &[]string{"uu"},
+						Teams: &[]string{"tt"},
+						Apps:  &[]string{"aa"},
+					},
+					BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{
+						Users: []string{"uuu"},
+						Teams: []string{"ttt"},
+						Apps:  []string{"aaa"},
+					},
+				},
+				Restrictions: &BranchRestrictionsRequest{
+					Users: []string{"u"},
+					Teams: []string{"t"},
+					Apps:  []string{"a"},
+				},
+				BlockCreations:   Bool(true),
+				LockBranch:       Bool(true),
+				AllowForkSyncing: Bool(true),
+			}
+
+			mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) {
+				v := new(ProtectionRequest)
+				assertNilError(t, json.NewDecoder(r.Body).Decode(v))
+
+				testMethod(t, r, "PUT")
+				if !cmp.Equal(v, input) {
+					t.Errorf("Request body = %+v, want %+v", v, input)
+				}
+
+				// TODO: remove custom Accept header when this API fully launches
+				testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview)
+				fmt.Fprintf(w, `{
+					"required_status_checks":{
+						"strict":true,
+						"contexts":[],
+						"checks": null
+					},
+					"required_pull_request_reviews":{
+						"dismissal_restrictions":{
+							"users":[{
+								"id":3,
+								"login":"uu"
+							}],
+							"teams":[{
+								"id":4,
+								"slug":"tt"
+							}],
+							"apps":[{
+								"id":5,
+								"slug":"aa"
+							}]
+						},
+						"dismiss_stale_reviews":true,
+						"require_code_owner_reviews":true,
+						"bypass_pull_request_allowances": {
+							"users":[{"id":10,"login":"uuu"}],
+							"teams":[{"id":20,"slug":"ttt"}],
+							"apps":[{"id":30,"slug":"aaa"}]
+						}
+					},
+					"restrictions":{
+						"users":[{"id":1,"login":"u"}],
+						"teams":[{"id":2,"slug":"t"}],
+						"apps":[{"id":3,"slug":"a"}]
+					},
+					"block_creations": {
+						"enabled": true
+					},
+					"lock_branch": {
+						"enabled": true
+					},
+					"allow_fork_syncing": {
+						"enabled": true
+					}
+				}`)
+			})
+
+			ctx := context.Background()
+			protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input)
+			if err != nil {
+				t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err)
+			}
+
+			want := &Protection{
+				RequiredStatusChecks: &RequiredStatusChecks{
+					Strict:   true,
+					Contexts: &[]string{},
+				},
+				RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
+					DismissStaleReviews: true,
+					DismissalRestrictions: &DismissalRestrictions{
+						Users: []*User{
+							{Login: String("uu"), ID: Int64(3)},
+						},
+						Teams: []*Team{
+							{Slug: String("tt"), ID: Int64(4)},
+						},
+						Apps: []*App{
+							{Slug: String("aa"), ID: Int64(5)},
+						},
+					},
+					RequireCodeOwnerReviews: true,
+					BypassPullRequestAllowances: &BypassPullRequestAllowances{
+						Users: []*User{
+							{Login: String("uuu"), ID: Int64(10)},
+						},
+						Teams: []*Team{
+							{Slug: String("ttt"), ID: Int64(20)},
+						},
+						Apps: []*App{
+							{Slug: String("aaa"), ID: Int64(30)},
+						},
+					},
+				},
+				Restrictions: &BranchRestrictions{
+					Users: []*User{
+						{Login: String("u"), ID: Int64(1)},
+					},
+					Teams: []*Team{
+						{Slug: String("t"), ID: Int64(2)},
+					},
+					Apps: []*App{
+						{Slug: String("a"), ID: Int64(3)},
+					},
+				},
+				BlockCreations: &BlockCreations{
+					Enabled: Bool(true),
+				},
+				LockBranch: &LockBranch{
+					Enabled: Bool(true),
+				},
+				AllowForkSyncing: &AllowForkSyncing{
+					Enabled: Bool(true),
+				},
+			}
+			if !cmp.Equal(protection, want) {
+				t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want)
+			}
+
+			const methodName = "UpdateBranchProtection"
+			testBadOptions(t, methodName, func() (err error) {
+				_, _, err = client.Repositories.UpdateBranchProtection(ctx, "\n", "\n", "\n", input)
+				return err
+			})
+
+			testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) {
+				got, resp, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input)
+				if got != nil {
+					t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got)
+				}
+				return resp, err
+			})
+		})
+	}
+}
+
 func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) {
 	tests := []struct {
 		branch  string
@@ -1609,7 +1787,7 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) {
 			input := &ProtectionRequest{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict: true,
-					Checks: []*RequiredStatusCheck{
+					Checks: &[]*RequiredStatusCheck{
 						{
 							Context: "continuous-integration",
 						},
@@ -1697,8 +1875,8 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) {
 			want := &Protection{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict:   true,
-					Contexts: []string{"continuous-integration"},
-					Checks: []*RequiredStatusCheck{
+					Contexts: &[]string{"continuous-integration"},
+					Checks: &[]*RequiredStatusCheck{
 						{
 							Context: "continuous-integration",
 						},
@@ -1749,6 +1927,149 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) {
 	}
 }
 
+func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) {
+	tests := []struct {
+		branch  string
+		urlPath string
+	}{
+		{branch: "b", urlPath: "/repos/o/r/branches/b/protection"},
+		{branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.branch, func(t *testing.T) {
+			client, mux, _, teardown := setup()
+			defer teardown()
+
+			input := &ProtectionRequest{
+				RequiredStatusChecks: &RequiredStatusChecks{
+					Strict: true,
+					Checks: &[]*RequiredStatusCheck{},
+				},
+				RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{
+					DismissStaleReviews: true,
+					DismissalRestrictionsRequest: &DismissalRestrictionsRequest{
+						Users: &[]string{"uu"},
+						Teams: &[]string{"tt"},
+						Apps:  &[]string{"aa"},
+					},
+					BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{
+						Users: []string{"uuu"},
+						Teams: []string{"ttt"},
+						Apps:  []string{"aaa"},
+					},
+				},
+				Restrictions: &BranchRestrictionsRequest{
+					Users: []string{"u"},
+					Teams: []string{"t"},
+					Apps:  []string{"a"},
+				},
+			}
+
+			mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) {
+				v := new(ProtectionRequest)
+				assertNilError(t, json.NewDecoder(r.Body).Decode(v))
+
+				testMethod(t, r, "PUT")
+				if !cmp.Equal(v, input) {
+					t.Errorf("Request body = %+v, want %+v", v, input)
+				}
+
+				// TODO: remove custom Accept header when this API fully launches
+				testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview)
+				fmt.Fprintf(w, `{
+					"required_status_checks":{
+						"strict":true,
+						"contexts":null,
+						"checks": []
+					},
+					"required_pull_request_reviews":{
+						"dismissal_restrictions":{
+							"users":[{
+								"id":3,
+								"login":"uu"
+							}],
+							"teams":[{
+								"id":4,
+								"slug":"tt"
+							}],
+							"apps":[{
+								"id":5,
+								"slug":"aa"
+							}]
+						},
+						"dismiss_stale_reviews":true,
+						"require_code_owner_reviews":true,
+						"bypass_pull_request_allowances": {
+							"users":[{"id":10,"login":"uuu"}],
+							"teams":[{"id":20,"slug":"ttt"}],
+							"apps":[{"id":30,"slug":"aaa"}]
+						}
+					},
+					"restrictions":{
+						"users":[{"id":1,"login":"u"}],
+						"teams":[{"id":2,"slug":"t"}],
+						"apps":[{"id":3,"slug":"a"}]
+					}
+				}`)
+			})
+
+			ctx := context.Background()
+			protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input)
+			if err != nil {
+				t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err)
+			}
+
+			want := &Protection{
+				RequiredStatusChecks: &RequiredStatusChecks{
+					Strict: true,
+					Checks: &[]*RequiredStatusCheck{},
+				},
+				RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
+					DismissStaleReviews: true,
+					DismissalRestrictions: &DismissalRestrictions{
+						Users: []*User{
+							{Login: String("uu"), ID: Int64(3)},
+						},
+						Teams: []*Team{
+							{Slug: String("tt"), ID: Int64(4)},
+						},
+						Apps: []*App{
+							{Slug: String("aa"), ID: Int64(5)},
+						},
+					},
+					RequireCodeOwnerReviews: true,
+					BypassPullRequestAllowances: &BypassPullRequestAllowances{
+						Users: []*User{
+							{Login: String("uuu"), ID: Int64(10)},
+						},
+						Teams: []*Team{
+							{Slug: String("ttt"), ID: Int64(20)},
+						},
+						Apps: []*App{
+							{Slug: String("aaa"), ID: Int64(30)},
+						},
+					},
+				},
+				Restrictions: &BranchRestrictions{
+					Users: []*User{
+						{Login: String("u"), ID: Int64(1)},
+					},
+					Teams: []*Team{
+						{Slug: String("t"), ID: Int64(2)},
+					},
+					Apps: []*App{
+						{Slug: String("a"), ID: Int64(3)},
+					},
+				},
+			}
+			if !cmp.Equal(protection, want) {
+				t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want)
+			}
+		})
+	}
+}
+
 func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) {
 	tests := []struct {
 		branch  string
@@ -1844,7 +2165,7 @@ func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T)
 			want := &Protection{
 				RequiredStatusChecks: &RequiredStatusChecks{
 					Strict:   true,
-					Contexts: []string{},
+					Contexts: &[]string{},
 				},
 				RequiredPullRequestReviews: &PullRequestReviewsEnforcement{
 					DismissStaleReviews: true,
@@ -2082,8 +2403,8 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) {
 
 			want := &RequiredStatusChecks{
 				Strict:   true,
-				Contexts: []string{"x", "y", "z"},
-				Checks: []*RequiredStatusCheck{
+				Contexts: &[]string{"x", "y", "z"},
+				Checks: &[]*RequiredStatusCheck{
 					{
 						Context: "x",
 					},
@@ -2202,8 +2523,8 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) {
 
 			want := &RequiredStatusChecks{
 				Strict:   true,
-				Contexts: []string{"continuous-integration"},
-				Checks: []*RequiredStatusCheck{
+				Contexts: &[]string{"continuous-integration"},
+				Checks: &[]*RequiredStatusCheck{
 					{
 						Context: "continuous-integration",
 					},
@@ -2300,8 +2621,8 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) {
 
 			want := &RequiredStatusChecks{
 				Strict:   true,
-				Contexts: []string{"continuous-integration"},
-				Checks: []*RequiredStatusCheck{
+				Contexts: &[]string{"continuous-integration"},
+				Checks: &[]*RequiredStatusCheck{
 					{
 						Context: "continuous-integration",
 					},
diff --git a/test/integration/repos_test.go b/test/integration/repos_test.go
index 79e8cdfe..9b32cc4a 100644
--- a/test/integration/repos_test.go
+++ b/test/integration/repos_test.go
@@ -103,7 +103,7 @@ func TestRepositories_EditBranches(t *testing.T) {
 	protectionRequest := &github.ProtectionRequest{
 		RequiredStatusChecks: &github.RequiredStatusChecks{
 			Strict:   true,
-			Contexts: []string{"continuous-integration"},
+			Contexts: &[]string{"continuous-integration"},
 		},
 		RequiredPullRequestReviews: &github.PullRequestReviewsEnforcementRequest{
 			DismissStaleReviews: true,
@@ -126,7 +126,7 @@ func TestRepositories_EditBranches(t *testing.T) {
 	want := &github.Protection{
 		RequiredStatusChecks: &github.RequiredStatusChecks{
 			Strict:   true,
-			Contexts: []string{"continuous-integration"},
+			Contexts: &[]string{"continuous-integration"},
 		},
 		RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{
 			DismissStaleReviews:          true,