Skip to content

Commit d9cc6bf

Browse files
authored
refactor(internal/serviceconfig): rename NoRESTNumericEnums to SkipRESTNumericEnums (#5084)
NoRESTNumericEnums used a `map[string]bool` to track languages that should not pass the rest-numeric-enums flag. The double negation in the name was confusing, and the bool value introduces ambiguity since the zero value (false) is indistinguishable from an explicit setting. Replace this with a `[]string` of languages that skip the rest-numeric-enums flag. This removes the double negation and avoids the zero-value ambiguity. Fixes #4086
1 parent afbf013 commit d9cc6bf

8 files changed

Lines changed: 244 additions & 256 deletions

File tree

doc/api-allowlist-schema.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ This document describes the schema for the API Allowlist.
1212
| `documentation_uri` | string | Overrides the product documentation URI from the service config's publishing section. |
1313
| `languages` | list of string | Restricts which languages can generate client libraries for this API. Use "all" to indicate all languages can use this API.<br><br>Restrictions exist for several reasons:<br>- Newer languages (Rust, Dart) skip older beta versions when stable versions exist<br>- Python has historical legacy APIs not available to other languages<br>- Some APIs (like DIREGAPIC protos) are only used by specific languages |
1414
| `new_issue_uri` | string | Overrides the new issue URI from the service config's publishing section. |
15-
| `no_rest_numeric_enums` | map[string]bool | Determines whether to use numeric enums in REST requests. The "No" prefix is used because the default behavior (when this field is `false` or omitted) is to generate numeric enums. Map key is the language name (e.g., "python", "rust"). Optional. If omitted, the generator default is used. |
15+
| `skip_rest_numeric_enums` | list of string | Lists languages that should not pass the rest-numeric-enums flag to the generator. The special value "all" skips it for every language. If empty, all languages use numeric enums. |
1616
| `open_api` | string | Is the file path to an OpenAPI spec, currently in internal/testdata. This is not an official spec yet and exists only for Rust to validate OpenAPI support. |
1717
| `release_level` | map[string]string | Is the release level per language. Map key is the language name (e.g., "python", "rust"). Optional. If omitted, the generator default is used.<br><br>TODO(https://github.com/googleapis/librarian/issues/4834): Go uses "alpha", "beta", and "ga" instead of "preview" and "stable". We should standardize release level vocabulary across languages. |
1818
| `short_name` | string | Overrides the API short name from the service config's publishing section. |

internal/librarian/java/generate_test.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,7 @@ func TestResolveGAPICOptions(t *testing.T) {
8989
Transports: map[string]serviceconfig.Transport{
9090
config.LanguageJava: serviceconfig.GRPCRest,
9191
},
92-
NoRESTNumericEnums: map[string]bool{
93-
config.LanguageJava: true,
94-
},
92+
SkipRESTNumericEnums: []string{config.LanguageJava},
9593
},
9694
want: []string{
9795
"metadata",

internal/serviceconfig/api.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,10 @@ type API struct {
7676
// publishing section.
7777
NewIssueURI string `yaml:"new_issue_uri,omitempty"`
7878

79-
// NoRESTNumericEnums determines whether to use numeric enums in REST requests.
80-
// The "No" prefix is used because the default behavior (when this field is `false` or omitted) is
81-
// to generate numeric enums.
82-
// Map key is the language name (e.g., "python", "rust").
83-
// Optional. If omitted, the generator default is used.
84-
NoRESTNumericEnums map[string]bool `yaml:"no_rest_numeric_enums,omitempty"`
79+
// SkipRESTNumericEnums lists languages that should not pass the
80+
// rest-numeric-enums flag to the generator. The special value "all"
81+
// skips it for every language. If empty, all languages use numeric enums.
82+
SkipRESTNumericEnums []string `yaml:"skip_rest_numeric_enums,omitempty"`
8583

8684
// OpenAPI is the file path to an OpenAPI spec, currently in internal/testdata.
8785
// This is not an official spec yet and exists only for Rust to validate OpenAPI support.
@@ -132,18 +130,15 @@ func (api *API) Transport(language string) Transport {
132130

133131
// HasRESTNumericEnums reports whether the generator should pass the
134132
// rest-numeric-enums option for the given language. The default (when
135-
// NoRESTNumericEnums is empty) is true.
133+
// SkipRESTNumericEnums is empty) is true.
136134
func (api *API) HasRESTNumericEnums(language string) bool {
137-
if len(api.NoRESTNumericEnums) == 0 {
135+
if len(api.SkipRESTNumericEnums) == 0 {
138136
return true
139137
}
140-
if _, ok := api.NoRESTNumericEnums[config.LanguageAll]; ok {
141-
return false
142-
}
143-
if _, ok := api.NoRESTNumericEnums[language]; ok {
138+
if slices.Contains(api.SkipRESTNumericEnums, config.LanguageAll) {
144139
return false
145140
}
146-
return true
141+
return !slices.Contains(api.SkipRESTNumericEnums, language)
147142
}
148143

149144
// ReleaseLevel gets the release level for a given language.

internal/serviceconfig/api_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -217,26 +217,26 @@ func TestHasRESTNumericEnums(t *testing.T) {
217217
want: true,
218218
},
219219
{
220-
name: "nil map enables numeric enums",
221-
sc: &API{NoRESTNumericEnums: nil},
220+
name: "nil list enables numeric enums",
221+
sc: &API{SkipRESTNumericEnums: nil},
222222
lang: config.LanguageNodejs,
223223
want: true,
224224
},
225225
{
226-
name: "disabled for all languages",
227-
sc: &API{NoRESTNumericEnums: map[string]bool{config.LanguageAll: true}},
226+
name: "skipped for all languages",
227+
sc: &API{SkipRESTNumericEnums: []string{config.LanguageAll}},
228228
lang: config.LanguageGo,
229229
want: false,
230230
},
231231
{
232-
name: "disabled for specific language",
233-
sc: &API{NoRESTNumericEnums: map[string]bool{config.LanguageNodejs: true}},
232+
name: "skipped for specific language",
233+
sc: &API{SkipRESTNumericEnums: []string{config.LanguageNodejs}},
234234
lang: config.LanguageNodejs,
235235
want: false,
236236
},
237237
{
238-
name: "disabled for other language only",
239-
sc: &API{NoRESTNumericEnums: map[string]bool{"python": true}},
238+
name: "skipped for other language only",
239+
sc: &API{SkipRESTNumericEnums: []string{"python"}},
240240
lang: config.LanguageGo,
241241
want: true,
242242
},

0 commit comments

Comments
 (0)