Use `omitzero` to support handling empty arrays

[stevehipwell]

With the release of Go 1.24 we now have the omitzero JSON struct tag. Using this tag we can differentiate between an unset array and an empty array, this would allow us to simplify the API surface for endpoints where this distinction is required.

For example the following single change would allow us to remove the 3 functions required to clear the BypassActors which in turn would make the calling code significantly simpler.

// RepositoryRuleset represents a GitHub ruleset object.
type RepositoryRuleset struct {
	ID                   *int64                       `json:"id,omitempty"`
	Name                 string                       `json:"name"`
	Target               *RulesetTarget               `json:"target,omitempty"`
	SourceType           *RulesetSourceType           `json:"source_type,omitempty"`
	Source               string                       `json:"source"`
	Enforcement          RulesetEnforcement           `json:"enforcement"`
	BypassActors         []*BypassActor               `json:"bypass_actors,omitzero"` // Was omitempty
	CurrentUserCanBypass *BypassMode                  `json:"current_user_can_bypass,omitempty"`
	NodeID               *string                      `json:"node_id,omitempty"`
	Links                *RepositoryRulesetLinks      `json:"_links,omitempty"`
	Conditions           *RepositoryRulesetConditions `json:"conditions,omitempty"`
	Rules                *RepositoryRulesetRules      `json:"rules,omitempty"`
	UpdatedAt            *Timestamp                   `json:"updated_at,omitempty"`
	CreatedAt            *Timestamp                   `json:"created_at,omitempty"`
}

We could also use omitzero instead of pointers in some cases, but this is explicitly not covered by this issue.

[maishivamhoo123]

@team @stevehipwell could you please assign this issue to me? I would love to work on it.

[maishivamhoo123]

@stevehipwell Sir and @team, I had a small doubt. If I replace omitempty with omitzero for:

BypassActors []*BypassActor json:"bypass_actors,omitzero"

I believe these workaround structs and functions may no longer be required:

https://github.com/google/go-github/blob/master/github/repos_rules.go#L14

https://github.com/google/go-github/blob/master/github/repos_rules.go#L32

https://github.com/google/go-github/blob/master/github/repos_rules.go#L174

https://github.com/google/go-github/blob/master/github/repos_rules.go#L203

https://github.com/google/go-github/blob/master/github/orgs_rules.go#L113

https://github.com/google/go-github/blob/master/github/enterprise_rules.go#L86

Sir, could you please confirm if removing these after switching to omitzero would still result in the correct expected behavior? Looking for your guidance.

[maishivamhoo123]

@team,

All tests are passing, but script/lint.sh and script/generate.sh are failing due to generated metadata.

The linter reports:

structfield: change field BypassActors to match tag json:"bypass_actors,omitzero"

I didn’t rename the struct field because it would break the public API.

Running script/generate.sh shows:

duplicate operation:
PUT /repos/{owner}/{repo}/rulesets/{ruleset_id}
PUT /orgs/{org}/rulesets/{ruleset_id}
PUT /enterprises/{enterprise}/rulesets/{ruleset_id}

It looks like the OpenAPI metadata or generator is producing an invalid JSON tag (omitzero) and duplicate operations.

Could you please guide me on the correct way to resolve this?

Thanks!

[gmlewis]

The first and most obvious problem is to make sure the custom linters work with omitzero.
We've actually had a couple contributors show interest lately in the custom linters...
@alexandear or @zyfy29 - would either of you like to take a stab at this?

[stevehipwell]

@maishivamhoo123 in answer to your question above, the implementation is meant to remove the requirement for custom functions and types to clear array fields that are optional.

[maishivamhoo123]

@stevehipwell Sir, I believe the main issue is resolved, but script/lint.sh is still reporting a warning due to the generated metadata. Could you please advise on the correct way to handle this?

[stevehipwell]

@maishivamhoo123 it looks like the custom linters need tuning to work with omitzero. You could take a look at them or you could wait and see if someone else picks it up.

[maishivamhoo123]

Thank you, sir. I’ll take a look at the custom linters and see what needs to be adjusted for omitzero.

[Not-Dhananjay-Mishra]

@gmlewis , can I try fixing that linters issue?

Approach -

I can add a check in the function, for whether the tag contains ,omitzero. If it does, we can strip it out of the tag value the same way we do with ,omitempty.

structfield.go

func processTag(structName string, goField *ast.Ident, field *ast.Field, structTag reflect.StructTag, tagType string, pass *analysis.Pass, allowedTagNames, allowedTagTypes map[string]bool) {
	tagName, ok := structTag.Lookup(tagType)
	if !ok || tagName == "-" {
		return
	}
    // adding omitzero logic here
	if strings.Contains(tagName, ",omitempty") {
		checkGoFieldType(structName, goField.Name, field, field.Type.Pos(), pass, allowedTagTypes)
		tagName = strings.ReplaceAll(tagName, ",omitempty", "")
	}

	if tagType == "url" {
		tagName = strings.ReplaceAll(tagName, ",comma", "")
	}

	checkGoFieldName(structName, goField.Name, tagName, goField.Pos(), pass, allowedTagNames)
}

can we go ahead with this approach?

[maishivamhoo123]

@Not-Dhananjay-Mishra could you please pick another issue? I’m currently working on this one.
By the way, thank you for pointing it out

[Not-Dhananjay-Mishra]

@Not-Dhananjay-Mishra could you please pick another issue? I’m currently working on this one.
By the way, thank you for pointing it out

Sure 👍

[maishivamhoo123]

@gmlewis sir, @team — this is still failing. I’ve tried all the possible approaches I can think of so far, but it’s still not working. Could you please help me or point me in the right direction?

[gmlewis]

@gmlewis sir, @team — this is still failing. I’ve tried all the possible approaches I can think of so far, but it’s still not working. Could you please help me or point me in the right direction?

@maishivamhoo123 - to my knowledge, #3859 (comment) has not happened yet and you will be blocked until it has.

Reading through this issue, I'm now a bit confused. It seems to me:

• I said the linters need updating before this issue can be addressed.
• @Not-Dhananjay-Mishra offered to fix the linters
• @maishivamhoo123 said "no" to @Not-Dhananjay-Mishra because you wanted to fix them
• @maishivamhoo123 are now saying you are blocked on this issue without having actually fixed the linters yet.

Am I mistaken?

If I'm correct, then I think we should let @Not-Dhananjay-Mishra fix the linters first, and then you will be unblocked on this issue.