Skip to content

Commit 4518136

Browse files
authored
fix(protoc): support boolean attrs from Starlark rules (#427)
Starlark booleans are value-typed starlark.Bool, so the existing *starlark.Bool type switch rejected them and triggered a panic through the rule error reporter. Handle the correct type and add regression coverage verifying that has_services = False is emitted in the generated BUILD rule.
1 parent 5c7b2ab commit 4518136

2 files changed

Lines changed: 53 additions & 5 deletions

File tree

pkg/protoc/starlark_rule.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,8 @@ func (s *starlarkRuleProvider) Rule(othergen ...*rule.Rule) *rule.Rule {
276276
}
277277
if attrValue, ok, err := attrs.Get(attrName); ok && err == nil {
278278
switch t := attrValue.(type) {
279-
case *starlark.Bool:
280-
r.SetAttr(attrName.GoString(), bool(*t))
279+
case starlark.Bool:
280+
r.SetAttr(attrName.GoString(), bool(t))
281281
case *starlark.Int:
282282
intValue, _ := t.Int64()
283283
r.SetAttr(attrName.GoString(), intValue)

pkg/protoc/starlark_rule_test.go

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ func TestLoadStarlarkRule(t *testing.T) {
1818
wantErr error
1919
wantPrinted string
2020
want *rule.Rule
21+
panicOnErr bool
22+
// wantRuleFormatted, if set, is compared against the build file
23+
// rendering of the provided rule (used instead of tc.want, which
24+
// cannot cmp.Diff a non-nil *rule.Rule having unexported fields).
25+
wantRuleFormatted string
2126
}{
2227
"degenerate": {
2328
wantErr: fmt.Errorf(`test.star: rule "test" was never declared`),
@@ -91,6 +96,38 @@ protoc.Rule(
9196
`ProtocConfiguration(imports = [], language_config = LanguageConfig(enabled = False, name = "", plugins = {}, protoc = "", rules = {}), mappings = {}, outputs = [], package_config = PackageConfig(config = Config(repo_name = "", repo_root = "", work_dir = "")), plugins = [], prefix = "", proto_library = ProtoLibrary(base_name = "", deps = [], files = [], imports = [], name = "", srcs = [], strip_import_prefix = ""), rel = "")` +
9297
"\n",
9398
},
99+
"boolean attr": {
100+
code: `
101+
def make_ts_library_rule():
102+
return gazelle.Rule(
103+
name = "foo_ts_proto",
104+
kind = "trumid_proto_ts_library",
105+
attrs = {
106+
"has_services": False,
107+
},
108+
)
109+
110+
def provide_rule(rctx, pctx):
111+
return struct(
112+
name = "foo_ts_proto",
113+
kind = "trumid_proto_ts_library",
114+
rule = make_ts_library_rule,
115+
)
116+
117+
protoc.Rule(
118+
name = "test",
119+
load_info = lambda: None,
120+
kind_info = lambda: None,
121+
provide_rule = provide_rule,
122+
)
123+
`,
124+
panicOnErr: true,
125+
wantRuleFormatted: `trumid_proto_ts_library(
126+
name = "foo_ts_proto",
127+
has_services = False,
128+
)
129+
`,
130+
},
94131
"may-return-none": {
95132
code: `
96133
def make_py_library_rule(self):
@@ -129,11 +166,14 @@ protoc.Rule(
129166
t.Run(name, func(t *testing.T) {
130167
var err error
131168
var gotPrinted strings.Builder
132-
var rule LanguageRule
133-
rule, err = loadStarlarkLanguageRule("test", "test.star", strings.NewReader(tc.code), func(msg string) {
169+
var languageRule LanguageRule
170+
languageRule, err = loadStarlarkLanguageRule("test", "test.star", strings.NewReader(tc.code), func(msg string) {
134171
gotPrinted.WriteString(msg)
135172
gotPrinted.Write([]byte{'\n'})
136173
}, func(loadErr error) {
174+
if tc.panicOnErr {
175+
panic(loadErr)
176+
}
137177
err = loadErr
138178
})
139179
if err != nil {
@@ -147,7 +187,7 @@ protoc.Rule(
147187
}
148188
}
149189

150-
provider := rule.ProvideRule(tc.rc, tc.pc)
190+
provider := languageRule.ProvideRule(tc.rc, tc.pc)
151191
if err != nil {
152192
if tc.wantErr != nil {
153193
if diff := cmp.Diff(tc.wantErr.Error(), err.Error()); diff != "" {
@@ -168,6 +208,14 @@ protoc.Rule(
168208
}
169209

170210
got := provider.Rule()
211+
if tc.wantRuleFormatted != "" {
212+
file := rule.EmptyFile("", "")
213+
got.Insert(file)
214+
if diff := cmp.Diff(tc.wantRuleFormatted, string(file.Format())); diff != "" {
215+
t.Errorf("StarlarkRule.ProvideRule formatted rule (-want +got):\n%s", diff)
216+
}
217+
return
218+
}
171219
if diff := cmp.Diff(tc.want, got); diff != "" {
172220
t.Errorf("StarlarkRule.ProvideRule (-want +got):\n%s", diff)
173221
}

0 commit comments

Comments
 (0)