Add starlark rule "experimental_resolve_attr" feature (stackb#329)

pcj · web-flow · commit 3799dab3ead7 · 2023-06-18T22:00:15.000-06:00
* Add starlark rule "experimental_resolve_attr" feature
* Restore proto_repository_tools check
* Fix errors due to hidden/swallowed starlark configuration problems!
* Fix //example/golden:starlark_java_test
diff --git a/example/golden/golden_test.go b/example/golden/golden_test.go
@@ -7,5 +7,7 @@ import (
 )
 
 func TestGoldens(t *testing.T) {
-	goldentest.FromDir("example/golden").Run(t, "gazelle")
+	goldentest.
+		FromDir("example/golden").
+		Run(t, "gazelle")
 }
diff --git a/example/golden/testdata/proto_repository/starlark/plugins.star b/example/golden/testdata/proto_repository/starlark/plugins.star
@@ -11,7 +11,7 @@ def _configure_protoc_gen_java(ctx):
 
     srcjar = ctx.proto_library.base_name + ".srcjar"
     if ctx.rel:
-        srcjar = "/".join(ctx.rel, srcjar)
+        srcjar = "/".join([ctx.rel, srcjar])
 
     config = protoc.PluginConfiguration(
         label = "@build_stack_rules_proto//plugin/builtin:java",
diff --git a/example/golden/testdata/starlark_java/.gazelle.args b/example/golden/testdata/starlark_java/.gazelle.args
@@ -1,2 +1,3 @@
 -proto_plugin=lib/plugins.star%java
 -proto_rule=lib/rules.star%java_library
+-proto_rule=lib/rules.star%java_wrapper
diff --git a/example/golden/testdata/starlark_java/BUILD.in b/example/golden/testdata/starlark_java/BUILD.in
@@ -1,9 +1,12 @@
 # gazelle:proto_rule proto_compile implementation stackb:rules_proto:proto_compile
+# gazelle:proto_rule java_wrapper implementation lib/rules.star%java_wrapper
 # gazelle:proto_rule java_library implementation lib/rules.star%java_library
 # gazelle:proto_rule java_library deps @com_google_protobuf//:protobuf_java
 # gazelle:proto_rule java_library deps @com_google_protobuf//java/core
 # gazelle:proto_rule java_library visibility //visibility:public
 # gazelle:proto_plugin java implementation lib/plugins.star%java
 # gazelle:proto_language java rule proto_compile
 # gazelle:proto_language java rule java_library
+# gazelle:proto_language java rule java_wrapper
 # gazelle:proto_language java plugin java
+# gazelle:proto_language java enabled true
diff --git a/example/golden/testdata/starlark_java/BUILD.out b/example/golden/testdata/starlark_java/BUILD.out
@@ -1,37 +1,12 @@
-load("@rules_proto//proto:defs.bzl", "proto_library")
-load("@build_stack_rules_proto//rules:proto_compile.bzl", "proto_compile")
-load("@rules_java//java:defs.bzl", "java_library")
-
 # gazelle:proto_rule proto_compile implementation stackb:rules_proto:proto_compile
+# gazelle:proto_rule java_wrapper implementation lib/rules.star%java_wrapper
 # gazelle:proto_rule java_library implementation lib/rules.star%java_library
 # gazelle:proto_rule java_library deps @com_google_protobuf//:protobuf_java
 # gazelle:proto_rule java_library deps @com_google_protobuf//java/core
 # gazelle:proto_rule java_library visibility //visibility:public
 # gazelle:proto_plugin java implementation lib/plugins.star%java
 # gazelle:proto_language java rule proto_compile
 # gazelle:proto_language java rule java_library
+# gazelle:proto_language java rule java_wrapper
 # gazelle:proto_language java plugin java
-
-proto_library(
-    name = "example_proto",
-    srcs = ["example.proto"],
-    visibility = ["//visibility:public"],
-)
-
-java_library(
-    name = "example_java_library",
-    srcs = ["example.srcjar"],
-    visibility = ["//visibility:public"],
-    deps = [
-        "@com_google_protobuf//:protobuf_java",
-        "@com_google_protobuf//java/core",
-    ],
-)
-
-proto_compile(
-    name = "example_java_compile",
-    outs = {"@build_stack_rules_proto//plugin/builtin:java": "example.srcjar"},
-    outputs = ["example.srcjar"],
-    plugins = ["@build_stack_rules_proto//plugin/builtin:java"],
-    proto = "example_proto",
-)
+# gazelle:proto_language java enabled true
diff --git a/example/golden/testdata/starlark_java/customer/BUILD.in b/example/golden/testdata/starlark_java/customer/BUILD.in
diff --git a/example/golden/testdata/starlark_java/customer/BUILD.out b/example/golden/testdata/starlark_java/customer/BUILD.out
@@ -0,0 +1,33 @@
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("//:defs.bzl", "java_wrapper")
+load("@build_stack_rules_proto//rules:proto_compile.bzl", "proto_compile")
+load("@rules_java//java:defs.bzl", "java_library")
+
+proto_library(
+    name = "customer_proto",
+    srcs = ["customer.proto"],
+    visibility = ["//visibility:public"],
+)
+
+java_library(
+    name = "customer_java_library",
+    srcs = ["customer.srcjar"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_google_protobuf//:protobuf_java",
+        "@com_google_protobuf//java/core",
+    ],
+)
+
+java_wrapper(
+    name = "customer_java_wrap",
+    javalib = "customer_java_library",
+)
+
+proto_compile(
+    name = "customer_java_compile",
+    outs = {"@build_stack_rules_proto//plugin/builtin:java": "customer/customer.srcjar"},
+    outputs = ["customer.srcjar"],
+    plugins = ["@build_stack_rules_proto//plugin/builtin:java"],
+    proto = "customer_proto",
+)
diff --git a/example/golden/testdata/starlark_java/customer/customer.proto b/example/golden/testdata/starlark_java/customer/customer.proto
@@ -0,0 +1,10 @@
+syntax = "proto3";
+
+package customer;
+
+option java_multiple_files = true;
+option java_package = "com.github.stackb.rules_proto.example.golden.testdata.starlark_java.customer";
+
+message Customer {
+    string name = 1;
+}
diff --git a/example/golden/testdata/starlark_java/defs.bzl b/example/golden/testdata/starlark_java/defs.bzl
@@ -0,0 +1,2 @@
+def java_wrapper(**_kwargs):
+    pass
diff --git a/example/golden/testdata/starlark_java/example.proto b/example/golden/testdata/starlark_java/example.proto
diff --git a/example/golden/testdata/starlark_java/lib/plugins.star b/example/golden/testdata/starlark_java/lib/plugins.star
@@ -20,7 +20,7 @@ def _configure_java(ctx):
     # }
     srcjar = ctx.proto_library.base_name + ".srcjar"
     if ctx.rel:
-        srcjar = "/".join(ctx.rel, srcjar)
+        srcjar = "/".join([ctx.rel, srcjar])
 
     config = protoc.PluginConfiguration(
         label = "@build_stack_rules_proto//plugin/builtin:java",
diff --git a/example/golden/testdata/starlark_java/lib/rules.star b/example/golden/testdata/starlark_java/lib/rules.star
@@ -16,11 +16,39 @@ def _provide_java_library(rctx, pctx):
     return struct(
         name = "java_library",
         rule = lambda: _make_java_library_rule(rctx, pctx),
+        experimental_resolve_attr = "deps",
     )
 
 protoc.Rule(
     name = "java_library",
     load_info = lambda: gazelle.LoadInfo(name = "@rules_java//java:defs.bzl", symbols = ["java_library"]),
-    kind_info = lambda: gazelle.KindInfo(merge_attrs = {"srcs": True}, resolve_attrs = {"deps": True}),
+    kind_info = lambda: gazelle.KindInfo(mergeable_attrs = {"srcs": True}, resolve_attrs = {"deps": True}),
     provide_rule = _provide_java_library,
 )
+
+# --------------------------------------------------
+
+def _make_java_wrapper_rule(_rctx, pctx):
+    r = gazelle.Rule(
+        kind = "java_wrapper",
+        name = pctx.proto_library.base_name + "_java_wrap",
+        attrs = {
+            "javalib": pctx.proto_library.base_name + "_java_library",
+            "deps": [],
+        },
+    )
+    return r
+
+def _provide_java_wrapper(rctx, pctx):
+    return struct(
+        name = "java_wrapper",
+        rule = lambda: _make_java_wrapper_rule(rctx, pctx),
+        experimental_resolve_attr = "deps",
+    )
+
+protoc.Rule(
+    name = "java_wrapper",
+    load_info = lambda: gazelle.LoadInfo(name = "//:defs.bzl", symbols = ["java_wrapper"]),
+    kind_info = lambda: gazelle.KindInfo(mergeable_attrs = {"javalib": True}, resolve_attrs = {"deps": True}),
+    provide_rule = _provide_java_wrapper,
+)
diff --git a/example/golden/testdata/starlark_java/order/BUILD.in b/example/golden/testdata/starlark_java/order/BUILD.in
diff --git a/example/golden/testdata/starlark_java/order/BUILD.out b/example/golden/testdata/starlark_java/order/BUILD.out
@@ -0,0 +1,36 @@
+load("@rules_proto//proto:defs.bzl", "proto_library")
+load("//:defs.bzl", "java_wrapper")
+load("@build_stack_rules_proto//rules:proto_compile.bzl", "proto_compile")
+load("@rules_java//java:defs.bzl", "java_library")
+
+proto_library(
+    name = "order_proto",
+    srcs = ["order.proto"],
+    visibility = ["//visibility:public"],
+    deps = ["//customer:customer_proto"],
+)
+
+java_library(
+    name = "order_java_library",
+    srcs = ["order.srcjar"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//customer:customer_java_library",
+        "@com_google_protobuf//:protobuf_java",
+        "@com_google_protobuf//java/core",
+    ],
+)
+
+java_wrapper(
+    name = "order_java_wrap",
+    javalib = "order_java_library",
+    deps = ["//customer:customer_java_wrap"],
+)
+
+proto_compile(
+    name = "order_java_compile",
+    outs = {"@build_stack_rules_proto//plugin/builtin:java": "order/order.srcjar"},
+    outputs = ["order.srcjar"],
+    plugins = ["@build_stack_rules_proto//plugin/builtin:java"],
+    proto = "order_proto",
+)
diff --git a/example/golden/testdata/starlark_java/order/order.proto b/example/golden/testdata/starlark_java/order/order.proto
@@ -0,0 +1,12 @@
+syntax = "proto3";
+
+package order;
+
+import "customer/customer.proto";
+
+option java_multiple_files = true;
+option java_package = "com.github.stackb.rules_proto.example.golden.testdata.starlark_java.order";
+
+message Order {
+    customer.Customer customer = 1;
+}
diff --git a/pkg/protoc/package_config.go b/pkg/protoc/package_config.go
@@ -267,18 +267,14 @@ func RegisterStarlarkPlugin(c *config.Config, starlarkPlugin string) error {
 	}
 	fileName := parts[0]
 	ruleName := parts[1]
-	var configureError error
 	impl, err := LoadStarlarkPluginFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
 		log.Printf("%s: %v", starlarkPlugin, msg)
 	}, func(err error) {
-		configureError = err
+		log.Fatalf("starlark plugin configuration error (plugin %q will not be registered): %v", starlarkPlugin, err)
 	})
 	if err != nil {
 		return err
 	}
-	if configureError != nil {
-		return configureError
-	}
 	Plugins().RegisterPlugin(starlarkPlugin, impl)
 	return nil
 }
@@ -291,18 +287,13 @@ func RegisterStarlarkRule(c *config.Config, starlarkRule string) error {
 	fileName := parts[0]
 	ruleName := parts[1]
 
-	var configureError error
 	impl, err := LoadStarlarkLanguageRuleFromFile(c.WorkDir, fileName, ruleName, func(msg string) {
 	}, func(err error) {
-		configureError = err
+		log.Panicf("starlark rule configuration error (rule %q will not be registered): %v", starlarkRule, err)
 	})
 	if err != nil {
 		return err
 	}
-	if configureError != nil {
-		return configureError
-	}
-	configureError = err
 	Rules().MustRegisterRule(starlarkRule, impl)
 	return nil
 }
diff --git a/pkg/protoc/starlark_rule.go b/pkg/protoc/starlark_rule.go
@@ -32,7 +32,6 @@ func loadStarlarkLanguageRule(name, filename string, src interface{}, reporter f
 	newErrorf := func(msg string, args ...interface{}) error {
 		err := fmt.Errorf(filename+": "+msg, args...)
 		errorReporter(err)
-		reporter(err.Error())
 		return err
 	}
 
@@ -183,11 +182,18 @@ func (p *starlarkLanguageRule) ProvideRule(rc *LanguageRuleConfig, pc *ProtocCon
 	var result RuleProvider
 	switch value := value.(type) {
 	case *starlarkstruct.Struct:
+		var experimentalResolveDepsAttr string
+		if attr, err := value.Attr("experimental_resolve_attr"); err == nil {
+			if str, ok := attr.(starlark.String); ok {
+				experimentalResolveDepsAttr = str.GoString()
+			}
+		}
 		result = &starlarkRuleProvider{
-			name:          p.name,
-			provider:      value,
-			reporter:      p.reporter,
-			errorReporter: p.errorReporter,
+			name:                        p.name,
+			provider:                    value,
+			reporter:                    p.reporter,
+			errorReporter:               p.errorReporter,
+			experimentalResolveDepsAttr: experimentalResolveDepsAttr,
 		}
 	default:
 		p.errorReporter("rule %q provide_rule returned invalid type: %T", p.name, value)
@@ -199,10 +205,11 @@ func (p *starlarkLanguageRule) ProvideRule(rc *LanguageRuleConfig, pc *ProtocCon
 
 // starlarkRuleProvider implements RuleProvider via a starlark struct.
 type starlarkRuleProvider struct {
-	name          string
-	provider      *starlarkstruct.Struct
-	reporter      func(thread *starlark.Thread, msg string)
-	errorReporter func(msg string, args ...interface{}) error
+	name                        string
+	provider                    *starlarkstruct.Struct
+	reporter                    func(thread *starlark.Thread, msg string)
+	errorReporter               func(msg string, args ...interface{}) error
+	experimentalResolveDepsAttr string
 }
 
 // Kind implements part of the RuleProvider interface.
@@ -218,7 +225,6 @@ func (s *starlarkRuleProvider) Kind() string {
 // Name implements part of the RuleProvider interface.
 func (s *starlarkRuleProvider) Name() string {
 	return structAttrString(s.provider, "name", s.errorReporter)
-
 }
 
 // Rule implements part of the RuleProvider interface.
@@ -295,10 +301,20 @@ func (s *starlarkRuleProvider) Rule(othergen ...*rule.Rule) *rule.Rule {
 
 // Resolve implements part of the RuleProvider interface.
 func (s *starlarkRuleProvider) Resolve(c *config.Config, ix *resolve.RuleIndex, r *rule.Rule, imports []string, from label.Label) {
+	if s.experimentalResolveDepsAttr != "" {
+		if r.Attr(s.experimentalResolveDepsAttr) != nil {
+			ResolveDepsAttr(s.experimentalResolveDepsAttr, false)(c, ix, r, imports, from)
+		}
+	}
 }
 
 // Imports implements part of the RuleProvider interface.
 func (s *starlarkRuleProvider) Imports(c *config.Config, r *rule.Rule, file *rule.File) []resolve.ImportSpec {
+	if s.experimentalResolveDepsAttr != "" {
+		if lib, ok := r.PrivateAttr(ProtoLibraryKey).(ProtoLibrary); ok {
+			return ProtoLibraryImportSpecsForKind(r.Kind(), lib)
+		}
+	}
 	return nil
 }
 
diff --git a/pkg/protoc/starlark_util.go b/pkg/protoc/starlark_util.go
@@ -261,12 +261,15 @@ func structAttrBool(in *starlarkstruct.Struct, name string, errorReporter errorR
 		errorReporter("getting struct attr %s: %w", err)
 		return
 	}
-	b, ok := value.(*starlark.Bool)
-	if !ok {
-		errorReporter("%s is not a bool", name)
-		return
+	if value == nil {
+		return false
+	}
+	switch t := value.(type) {
+	case starlark.Bool:
+		out = bool(t.Truth())
+	default:
+		errorReporter("attr %q: want bool, got %T", name, value)
 	}
-	out = bool(*b)
 	return
 }
 
@@ -299,12 +302,12 @@ func structAttrMapStringBool(in *starlarkstruct.Struct, name string, errorReport
 	out = make(map[string]bool, dict.Len())
 	for _, key := range dict.Keys() {
 		if value, ok, err := dict.Get(key); ok && err == nil {
-			b, ok := value.(*starlark.Bool)
+			b, ok := value.(starlark.Bool)
 			if !ok {
-				errorReporter("%s is not a bool", name)
+				errorReporter("dict %q value for %q: want bool, got %T", name, key, value)
 				return
 			}
-			out[name] = bool(*b)
+			out[name] = bool(b.Truth())
 		}
 	}
 	return
diff --git a/rules/private/proto_repository_tools_srcs.bzl b/rules/private/proto_repository_tools_srcs.bzl

Original file line number	Diff line number	Diff line change
`@@ -7,5 +7,7 @@ import (`
`7`	`7`	`)`
`8`	`8`
`9`	`9`	`func TestGoldens(t *testing.T) {`
`10`		`- goldentest.FromDir("example/golden").Run(t, "gazelle")`
	`10`	`+ goldentest.`
	`11`	`+ FromDir("example/golden").`
	`12`	`+ Run(t, "gazelle")`
`11`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`-proto_plugin=lib/plugins.star%java`
`2`	`2`	`-proto_rule=lib/rules.star%java_library`
	`3`	`+-proto_rule=lib/rules.star%java_wrapper`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+def java_wrapper(**_kwargs):`
	`2`	`+ pass`