From 291cb1d090f4c1b112b9e4f5e924eb053fad774f Mon Sep 17 00:00:00 2001 From: Paul Johnston Date: Fri, 31 Jul 2026 08:54:04 -0600 Subject: [PATCH] fix(protoc): support boolean attrs from Starlark rules 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. --- pkg/protoc/starlark_rule.go | 4 +-- pkg/protoc/starlark_rule_test.go | 54 ++++++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/pkg/protoc/starlark_rule.go b/pkg/protoc/starlark_rule.go index 49e1ac6d1..9dde8703d 100644 --- a/pkg/protoc/starlark_rule.go +++ b/pkg/protoc/starlark_rule.go @@ -276,8 +276,8 @@ func (s *starlarkRuleProvider) Rule(othergen ...*rule.Rule) *rule.Rule { } if attrValue, ok, err := attrs.Get(attrName); ok && err == nil { switch t := attrValue.(type) { - case *starlark.Bool: - r.SetAttr(attrName.GoString(), bool(*t)) + case starlark.Bool: + r.SetAttr(attrName.GoString(), bool(t)) case *starlark.Int: intValue, _ := t.Int64() r.SetAttr(attrName.GoString(), intValue) diff --git a/pkg/protoc/starlark_rule_test.go b/pkg/protoc/starlark_rule_test.go index ddb7acc63..1ca0758ff 100644 --- a/pkg/protoc/starlark_rule_test.go +++ b/pkg/protoc/starlark_rule_test.go @@ -18,6 +18,11 @@ func TestLoadStarlarkRule(t *testing.T) { wantErr error wantPrinted string want *rule.Rule + panicOnErr bool + // wantRuleFormatted, if set, is compared against the build file + // rendering of the provided rule (used instead of tc.want, which + // cannot cmp.Diff a non-nil *rule.Rule having unexported fields). + wantRuleFormatted string }{ "degenerate": { wantErr: fmt.Errorf(`test.star: rule "test" was never declared`), @@ -91,6 +96,38 @@ protoc.Rule( `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 = "")` + "\n", }, + "boolean attr": { + code: ` +def make_ts_library_rule(): + return gazelle.Rule( + name = "foo_ts_proto", + kind = "trumid_proto_ts_library", + attrs = { + "has_services": False, + }, + ) + +def provide_rule(rctx, pctx): + return struct( + name = "foo_ts_proto", + kind = "trumid_proto_ts_library", + rule = make_ts_library_rule, + ) + +protoc.Rule( + name = "test", + load_info = lambda: None, + kind_info = lambda: None, + provide_rule = provide_rule, +) +`, + panicOnErr: true, + wantRuleFormatted: `trumid_proto_ts_library( + name = "foo_ts_proto", + has_services = False, +) +`, + }, "may-return-none": { code: ` def make_py_library_rule(self): @@ -129,11 +166,14 @@ protoc.Rule( t.Run(name, func(t *testing.T) { var err error var gotPrinted strings.Builder - var rule LanguageRule - rule, err = loadStarlarkLanguageRule("test", "test.star", strings.NewReader(tc.code), func(msg string) { + var languageRule LanguageRule + languageRule, err = loadStarlarkLanguageRule("test", "test.star", strings.NewReader(tc.code), func(msg string) { gotPrinted.WriteString(msg) gotPrinted.Write([]byte{'\n'}) }, func(loadErr error) { + if tc.panicOnErr { + panic(loadErr) + } err = loadErr }) if err != nil { @@ -147,7 +187,7 @@ protoc.Rule( } } - provider := rule.ProvideRule(tc.rc, tc.pc) + provider := languageRule.ProvideRule(tc.rc, tc.pc) if err != nil { if tc.wantErr != nil { if diff := cmp.Diff(tc.wantErr.Error(), err.Error()); diff != "" { @@ -168,6 +208,14 @@ protoc.Rule( } got := provider.Rule() + if tc.wantRuleFormatted != "" { + file := rule.EmptyFile("", "") + got.Insert(file) + if diff := cmp.Diff(tc.wantRuleFormatted, string(file.Format())); diff != "" { + t.Errorf("StarlarkRule.ProvideRule formatted rule (-want +got):\n%s", diff) + } + return + } if diff := cmp.Diff(tc.want, got); diff != "" { t.Errorf("StarlarkRule.ProvideRule (-want +got):\n%s", diff) }