Skip to content

Commit

Permalink
Flip --incompatible_struct_has_no_methods
Browse files Browse the repository at this point in the history
Disables the ability to use struct.to_proto() and struct.to_json() by
default.

Instances of this can be safely replaced with either:
 * proto.encode_text(struct)
 * json.encode(struct)

RELNOTES[INC]: Flipped --incompatible_struct_has_no_methods

Fixes #19465

PiperOrigin-RevId: 617511177
Change-Id: Ie626da396f52fbed24bf2be8c04c89dbcc47e63f
  • Loading branch information
c-mita authored and Copybara-Service committed Mar 20, 2024
1 parent 3b6aac0 commit 8f18d36
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ public final class BuildLanguageOptions extends OptionsBase {

@Option(
name = "incompatible_struct_has_no_methods",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
Expand Down Expand Up @@ -898,7 +898,7 @@ public StarlarkSemantics toStarlarkSemantics() {
public static final String INCOMPATIBLE_RUN_SHELL_COMMAND_STRING =
"+incompatible_run_shell_command_string";
public static final String INCOMPATIBLE_STRUCT_HAS_NO_METHODS =
"-incompatible_struct_has_no_methods";
"+incompatible_struct_has_no_methods";
public static final String INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC =
"-incompatible_use_cc_configure_from_rules";
public static final String INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1431,12 +1431,14 @@ private void checkTextMessage(String from, String... lines) throws Exception {

@Test
public void testSimpleTextMessagesBooleanFields() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkTextMessage("struct(name=True).to_proto()", "name: true");
checkTextMessage("struct(name=False).to_proto()", "name: false");
}

@Test
public void testStructRestrictedOverrides() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
ev.checkEvalErrorContains(
"cannot override built-in struct function 'to_json'", "struct(to_json='foo')");

Expand All @@ -1446,6 +1448,7 @@ public void testStructRestrictedOverrides() throws Exception {

@Test
public void testSimpleTextMessages() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkTextMessage("struct(name='value').to_proto()", "name: \"value\"");
checkTextMessage("struct(name=[]).to_proto()"); // empty lines
checkTextMessage("struct(name=['a', 'b']).to_proto()", "name: \"a\"", "name: \"b\"");
Expand Down Expand Up @@ -1531,11 +1534,13 @@ public void testNoneStructValue() throws Exception {

@Test
public void testProtoFieldsOrder() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkTextMessage("struct(d=4, b=2, c=3, a=1).to_proto()", "a: 1", "b: 2", "c: 3", "d: 4");
}

@Test
public void testTextMessageEscapes() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkTextMessage("struct(name='a\"b').to_proto()", "name: \"a\\\"b\"");
checkTextMessage("struct(name='a\\'b').to_proto()", "name: \"a'b\"");
checkTextMessage("struct(name='a\\nb').to_proto()", "name: \"a\\nb\"");
Expand All @@ -1546,6 +1551,7 @@ public void testTextMessageEscapes() throws Exception {

@Test
public void testTextMessageInvalidStructure() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
// list in list
ev.checkEvalErrorContains(
"in struct field .a: at list index 0: got list, want string, int, float, bool, or struct",
Expand Down Expand Up @@ -1595,12 +1601,14 @@ public void testStarlarkJsonModule() throws Exception {

@Test
public void testJsonBooleanFields() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkJson("struct(name=True).to_json()", "{\"name\":true}");
checkJson("struct(name=False).to_json()", "{\"name\":false}");
}

@Test
public void testJsonDictFields() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkJson("struct(config={}).to_json()", "{\"config\":{}}");
checkJson("struct(config={'key': 'value'}).to_json()", "{\"config\":{\"key\":\"value\"}}");
ev.checkEvalErrorContains(
Expand All @@ -1616,6 +1624,7 @@ public void testJsonDictFields() throws Exception {

@Test
public void testJsonEncoding() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkJson("struct(name='value').to_json()", "{\"name\":\"value\"}");
checkJson("struct(name=['a', 'b']).to_json()", "{\"name\":[\"a\",\"b\"]}");
checkJson("struct(name=123).to_json()", "{\"name\":123}");
Expand All @@ -1629,6 +1638,7 @@ public void testJsonEncoding() throws Exception {

@Test
public void testJsonEscapes() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkJson("struct(name='a\"b').to_json()", "{\"name\":\"a\\\"b\"}");
checkJson("struct(name='a\\'b').to_json()", "{\"name\":\"a'b\"}");
checkJson("struct(name='a\\\\b').to_json()", "{\"name\":\"a\\\\b\"}");
Expand All @@ -1639,11 +1649,13 @@ public void testJsonEscapes() throws Exception {

@Test
public void testJsonNestedListStructure() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
checkJson("struct(a=[['b']]).to_json()", "{\"a\":[[\"b\"]]}");
}

@Test
public void testJsonInvalidStructure() throws Exception {
setBuildLanguageOptions("--incompatible_struct_has_no_methods=false");
ev.checkEvalErrorContains(
"Invalid text format, expected a struct, a string, a bool, or an int but got a "
+ "builtin_function_or_method for struct field 'a'",
Expand Down Expand Up @@ -2835,7 +2847,7 @@ public void ruleDefinitionEnvironmentDigest_accountsForAttrsWhenCreatingRuleWith
throws Exception {
scratch.file(
"r/create.bzl",
"def f(ctx): return struct(value=ctx.attr.to_json())",
"def f(ctx): return struct(value=json.encode(ctx.attr))",
"def create(attrs): return rule(implementation=f, attrs=attrs)");
scratch.file("r/def.bzl", "load(':create.bzl', 'create')", "r = create({})");
scratch.file("r/BUILD", "load(':def.bzl', 'r')", "r(name='r')");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1120,9 +1120,7 @@ public void testDefaultProvider() throws Exception {
.isEqualTo(DefaultInfo.PROVIDER.getKey());

assertThat(myInfo.getValue("dir"))
.isEqualTo(
"[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\", \"to_json\", "
+ "\"to_proto\"]");
.isEqualTo("[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\"]");

assertThat(myInfo.getValue("rule_data_runfiles")).isInstanceOf(Runfiles.class);
assertThat(
Expand Down Expand Up @@ -1200,9 +1198,7 @@ public void testDefaultProviderInStruct() throws Exception {
.isEqualTo(DefaultInfo.PROVIDER.getKey());

assertThat(myInfo.getValue("dir"))
.isEqualTo(
"[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\", \"to_json\", "
+ "\"to_proto\"]");
.isEqualTo("[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\"]");

assertThat(myInfo.getValue("rule_data_runfiles")).isInstanceOf(Runfiles.class);
assertThat(
Expand Down Expand Up @@ -1293,9 +1289,7 @@ public void testDefaultProviderOnFileTarget() throws Exception {
.isEqualTo(DefaultInfo.PROVIDER.getKey());

assertThat(myInfo.getValue("dir"))
.isEqualTo(
"[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\", \"to_json\", "
+ "\"to_proto\"]");
.isEqualTo("[\"data_runfiles\", \"default_runfiles\", \"files\", \"files_to_run\"]");

assertThat(myInfo.getValue("file_data_runfiles")).isInstanceOf(Runfiles.class);
assertThat(
Expand Down

0 comments on commit 8f18d36

Please sign in to comment.