Skip to content

Commit 2ab03ac

Browse files
committed
Check struct.{to_json,to_proto} are built-in functions
`to_json`/`to_proto` function have been removed from `structs`. Check that they are built-in functions before eliminating the keys. Tests are gated behind `bazel_features.rules.no_struct_field_denylist` to prevent breakage on old Bazel when running the tests.
1 parent 00874b9 commit 2ab03ac

File tree

3 files changed

+38
-1
lines changed

3 files changed

+38
-1
lines changed

MODULE.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ bazel_dep(name = "rules_pkg", version = "1.0.1", dev_dependency = True)
2121
bazel_dep(name = "rules_testing", version = "0.6.0", dev_dependency = True)
2222
bazel_dep(name = "rules_cc", version = "0.0.17", dev_dependency = True)
2323
bazel_dep(name = "rules_shell", version = "0.3.0", dev_dependency = True)
24+
bazel_dep(name = "bazel_features", version = "1.32.0", dev_dependency = True)
2425

2526
# Needed for bazelci and for building distribution tarballs.
2627
# If using an unreleased version of bazel_skylib via git_override, apply

lib/structs.bzl

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,20 @@
1414

1515
"""Skylib module containing functions that operate on structs."""
1616

17+
_built_in_function = type(str)
18+
19+
def _is_built_in_function(value):
20+
"""Returns True if v is an instance of a built-in function.
21+
22+
Args:
23+
v: The value whose type should be checked.
24+
25+
Returns:
26+
True if v is an instance of a built-in function, False otherwise.
27+
"""
28+
29+
return type(value) == _built_in_function
30+
1731
def _to_dict(s):
1832
"""Converts a `struct` to a `dict`.
1933
@@ -31,7 +45,7 @@ def _to_dict(s):
3145
return {
3246
key: getattr(s, key)
3347
for key in dir(s)
34-
if key != "to_json" and key != "to_proto"
48+
if not ((key == "to_json" or key == "to_proto") and _is_built_in_function(getattr(s, key)))
3549
}
3650

3751
def _merge(first, *rest):

tests/structs_tests.bzl

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,13 @@
1414

1515
"""Unit tests for structs.bzl."""
1616

17+
load("@bazel_features//:features.bzl", "bazel_features")
1718
load("//lib:structs.bzl", "structs")
1819
load("//lib:unittest.bzl", "asserts", "unittest")
1920

21+
def _placeholder():
22+
pass
23+
2024
def _to_dict_test(ctx):
2125
"""Unit tests for structs.to_dict."""
2226
env = unittest.begin(ctx)
@@ -42,6 +46,24 @@ def _to_dict_test(ctx):
4246
structs.to_dict(struct(a = 1, b = struct(bb = 1))),
4347
)
4448

49+
# Older Bazel denied creating `struct` with `to_json`/`to_proto`
50+
if not bazel_features.rules.no_struct_field_denylist:
51+
return unittest.end(env)
52+
53+
# Test `to_json`/`to_proto` values are propagated
54+
asserts.equals(
55+
env,
56+
{"to_json": 1, "to_proto": 2},
57+
structs.to_dict(struct(to_json = 1, to_proto = 2)),
58+
)
59+
60+
# Test `to_json`/`to_proto` functions are propagated
61+
asserts.equals(
62+
env,
63+
{"to_json": _placeholder, "to_proto": _placeholder},
64+
structs.to_dict(struct(to_json = _placeholder, to_proto = _placeholder)),
65+
)
66+
4567
return unittest.end(env)
4668

4769
to_dict_test = unittest.make(_to_dict_test)

0 commit comments

Comments
 (0)