From c3dc38960bab385c172ab902f79f1c79ef7099a9 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 8 Apr 2024 07:55:37 -0700 Subject: [PATCH] Remove --incompatible_struct_has_no_methods and associated struct methods. Removes the disabled struct.to_json() and struct.to_proto() and the flag that could re-enable them. https://github.com/bazelbuild/bazel/issues/19465 RELNOTES[inc]: Removed --incompatible_struct_has_no_methods PiperOrigin-RevId: 622844911 Change-Id: Ie6a9b42a0ea6f45bfca40bd59a4ea6163214bebc --- .../build/lib/packages/StructImpl.java | 85 ------------------- .../semantics/BuildLanguageOptions.java | 13 --- .../build/lib/rules/java/JavaPluginInfo.java | 10 --- .../lib/starlarkbuildapi/core/StructApi.java | 57 ------------- .../starlarkbuildapi/java/JavaOutputApi.java | 12 --- .../skydoc/fakebuildapi/FakeStructApi.java | 10 --- .../packages/semantics/ConsistencyTest.java | 2 - .../StarlarkRuleClassFunctionsTest.java | 18 ---- 8 files changed, 207 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java b/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java index 2916911b8e4d1c..626c24ab28575e 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StructImpl.java @@ -17,17 +17,13 @@ import com.google.common.base.Objects; import com.google.common.collect.Ordering; import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi; -import com.google.protobuf.TextFormat; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.Map; import javax.annotation.Nullable; -import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; import net.starlark.java.eval.Starlark; -import net.starlark.java.eval.StarlarkInt; import net.starlark.java.eval.Structure; /** @@ -147,87 +143,6 @@ private Object getValueOrNull(String name) { } } - @Override - public String toProto() throws EvalException { - return Proto.INSTANCE.encodeText(this); - } - - /** - * Escapes the given string for use in proto/JSON string. - * - *

This escapes double quotes, backslashes, and newlines. - */ - private static String escapeDoubleQuotesAndBackslashesAndNewlines(String string) { - return TextFormat.escapeDoubleQuotesAndBackslashes(string).replace("\n", "\\n"); - } - - @Override - public String toJson() throws EvalException { - StringBuilder sb = new StringBuilder(); - printJson(this, sb, "struct field", null); - return sb.toString(); - } - - private static void printJson(Object value, StringBuilder sb, String container, String key) - throws EvalException { - if (value == Starlark.NONE) { - sb.append("null"); - } else if (value instanceof Structure) { - sb.append("{"); - - String join = ""; - for (String field : ((Structure) value).getFieldNames()) { - sb.append(join); - join = ","; - appendJSONStringLiteral(sb, field); - sb.append(':'); - printJson(((Structure) value).getValue(field), sb, "struct field", field); - } - sb.append("}"); - } else if (value instanceof Dict) { - sb.append("{"); - String join = ""; - for (Map.Entry entry : ((Dict) value).entrySet()) { - sb.append(join); - join = ","; - if (!(entry.getKey() instanceof String)) { - throw Starlark.errorf( - "Keys must be a string but got a %s for %s%s", - Starlark.type(entry.getKey()), container, key != null ? " '" + key + "'" : ""); - } - appendJSONStringLiteral(sb, (String) entry.getKey()); - sb.append(':'); - printJson(entry.getValue(), sb, "dict value", String.valueOf(entry.getKey())); - } - sb.append("}"); - } else if (value instanceof List) { - sb.append("["); - String join = ""; - for (Object item : ((List) value)) { - sb.append(join); - join = ","; - printJson(item, sb, "list element in struct field", key); - } - sb.append("]"); - } else if (value instanceof String) { - appendJSONStringLiteral(sb, (String) value); - } else if (value instanceof StarlarkInt || value instanceof Boolean) { - sb.append(value); - } else { - throw Starlark.errorf( - "Invalid text format, expected a struct, a string, a bool, or an int but got a %s for" - + " %s%s", - Starlark.type(value), container, key != null ? " '" + key + "'" : ""); - } - } - - private static void appendJSONStringLiteral(StringBuilder out, String s) { - out.append('"'); - out.append( - escapeDoubleQuotesAndBackslashesAndNewlines(s).replace("\r", "\\r").replace("\t", "\\t")); - out.append('"'); - } - @Override public String toString() { return Starlark.repr(this); diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 9269e0a22c97d2..73cf13a2f02fec 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -378,18 +378,6 @@ public final class BuildLanguageOptions extends OptionsBase { + " and 1 cpu.") public boolean experimentalActionResourceSet; - @Option( - name = "incompatible_struct_has_no_methods", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, - effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = - "Disables the to_json and to_proto methods of struct, which pollute the struct field" - + " namespace. Instead, use json.encode or json.encode_indent for JSON, or" - + " proto.encode_text for textproto.") - public boolean incompatibleStructHasNoMethods; - @Option( name = "incompatible_always_check_depset_elements", defaultValue = "true", @@ -798,7 +786,6 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool(INCOMPATIBLE_NO_PACKAGE_DISTRIBS, incompatibleNoPackageDistribs) .setBool(INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM, incompatibleNoRuleOutputsParam) .setBool(INCOMPATIBLE_RUN_SHELL_COMMAND_STRING, incompatibleRunShellCommandString) - .setBool(INCOMPATIBLE_STRUCT_HAS_NO_METHODS, incompatibleStructHasNoMethods) .setBool(StarlarkSemantics.PRINT_TEST_MARKER, internalStarlarkFlagTestCanary) .setBool( INCOMPATIBLE_DO_NOT_SPLIT_LINKING_CMDLINE, incompatibleDoNotSplitLinkingCmdline) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java index 8be5b5b8e51dc1..4979bd9bcaf730 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaPluginInfo.java @@ -205,16 +205,6 @@ private JavaPluginData disableAnnotationProcessing() { // Preserve data, which may be used by Error Prone plugins. data()); } - - @Override - public String toProto() throws EvalException { - throw Starlark.errorf("unsupported method"); - } - - @Override - public String toJson() throws EvalException { - throw Starlark.errorf("unsupported method"); - } } public static JavaPluginInfo mergeWithoutJavaOutputs(JavaPluginInfo a, JavaPluginInfo b) { diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/StructApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/StructApi.java index a5828dbf393b3f..4ebbdf93510908 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/StructApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/core/StructApi.java @@ -16,7 +16,6 @@ import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.docgen.annot.StarlarkConstructor; -import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import net.starlark.java.annot.Param; import net.starlark.java.annot.StarlarkBuiltin; import net.starlark.java.annot.StarlarkMethod; @@ -35,62 +34,6 @@ + "equal if they have the same fields and if corresponding field values are equal.") public interface StructApi extends StarlarkValue { - @StarlarkMethod( - name = "to_proto", - doc = - "Creates a text message from the struct parameter. This method only works if all " - + "struct elements (recursively) are strings, ints, booleans, " - + "other structs or dicts or lists of these types. " - + "Quotes and new lines in strings are escaped. " - + "Struct keys are iterated in the sorted order. " - + "Examples:

"
-              + "struct(key=123).to_proto()\n# key: 123\n\n"
-              + "struct(key=True).to_proto()\n# key: true\n\n"
-              + "struct(key=[1, 2, 3]).to_proto()\n# key: 1\n# key: 2\n# key: 3\n\n"
-              + "struct(key='text').to_proto()\n# key: \"text\"\n\n"
-              + "struct(key=struct(inner_key='text')).to_proto()\n"
-              + "# key {\n#   inner_key: \"text\"\n# }\n\n"
-              + "struct(key=[struct(inner_key=1), struct(inner_key=2)]).to_proto()\n"
-              + "# key {\n#   inner_key: 1\n# }\n# key {\n#   inner_key: 2\n# }\n\n"
-              + "struct(key=struct(inner_key=struct(inner_inner_key='text'))).to_proto()\n"
-              + "# key {\n#    inner_key {\n#     inner_inner_key: \"text\"\n#   }\n# }\n\n"
-              + "struct(foo={4: 3, 2: 1}).to_proto()\n"
-              + "# foo: {\n"
-              + "#   key: 4\n"
-              + "#   value: 3\n"
-              + "# }\n"
-              + "# foo: {\n"
-              + "#   key: 2\n"
-              + "#   value: 1\n"
-              + "# }\n"
-              + "
" - + "

Deprecated: use proto.encode_text(x) instead.", - disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_STRUCT_HAS_NO_METHODS) - String toProto() throws EvalException; - - @StarlarkMethod( - name = "to_json", - doc = - "Creates a JSON string from the struct parameter. This method only works if all " - + "struct elements (recursively) are strings, ints, booleans, other structs, a " - + "list of these types or a dictionary with string keys and values of these types. " - + "Quotes and new lines in strings are escaped. " - + "Examples:

"
-              + "struct(key=123).to_json()\n# {\"key\":123}\n\n"
-              + "struct(key=True).to_json()\n# {\"key\":true}\n\n"
-              + "struct(key=[1, 2, 3]).to_json()\n# {\"key\":[1,2,3]}\n\n"
-              + "struct(key='text').to_json()\n# {\"key\":\"text\"}\n\n"
-              + "struct(key=struct(inner_key='text')).to_json()\n"
-              + "# {\"key\":{\"inner_key\":\"text\"}}\n\n"
-              + "struct(key=[struct(inner_key=1), struct(inner_key=2)]).to_json()\n"
-              + "# {\"key\":[{\"inner_key\":1},{\"inner_key\":2}]}\n\n"
-              + "struct(key=struct(inner_key=struct(inner_inner_key='text'))).to_json()\n"
-              + "# {\"key\":{\"inner_key\":{\"inner_inner_key\":\"text\"}}}\n
." - + "

Deprecated: instead, use json.encode(x) or json.encode_indent(x), which work" - + " for values other than structs and do not pollute the struct field namespace. ", - disableWithFlag = BuildLanguageOptions.INCOMPATIBLE_STRUCT_HAS_NO_METHODS) - String toJson() throws EvalException; - /** Callable Provider for new struct objects. */ @StarlarkBuiltin(name = "Provider", documented = false, doc = "") interface StructProviderApi extends ProviderApi { diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaOutputApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaOutputApi.java index 0460367798eeb1..412dee25d3416d 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaOutputApi.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/java/JavaOutputApi.java @@ -19,8 +19,6 @@ import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi; import javax.annotation.Nullable; import net.starlark.java.annot.StarlarkMethod; -import net.starlark.java.eval.EvalException; -import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkSemantics; /** A tuple of a java classes jar and its associated source and interface archives. */ @@ -118,14 +116,4 @@ public interface JavaOutputApi extends StructApi { structField = true) @Nullable Depset getSrcJarsStarlark(StarlarkSemantics semantics); - - @Override - default String toProto() throws EvalException { - throw Starlark.errorf("unsupported method"); - } - - @Override - default String toJson() throws EvalException { - throw Starlark.errorf("unsupported method"); - } } diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStructApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStructApi.java index f4812e5dff8623..f41fc5536a3b78 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStructApi.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStructApi.java @@ -41,16 +41,6 @@ public FakeStructApi() { this(ImmutableMap.of()); } - @Override - public String toProto() throws EvalException { - return ""; - } - - @Override - public String toJson() throws EvalException { - return ""; - } - /** * Wraps {@link Structure#getValue(String)}, returning null in cases where {@link EvalException} * would have been thrown. diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index b380c0ba39e8fc..ce23adc1a87f6d 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -146,7 +146,6 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--incompatible_no_package_distribs=" + rand.nextBoolean(), "--incompatible_no_rule_outputs_param=" + rand.nextBoolean(), "--incompatible_run_shell_command_string=" + rand.nextBoolean(), - "--incompatible_struct_has_no_methods=" + rand.nextBoolean(), "--incompatible_require_linker_input_cc_api=" + rand.nextBoolean(), "--incompatible_use_cc_configure_from_rules_cc=" + rand.nextBoolean(), "--incompatible_unambiguous_label_stringification=" + rand.nextBoolean(), @@ -196,7 +195,6 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_PACKAGE_DISTRIBS, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_NO_RULE_OUTPUTS_PARAM, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_RUN_SHELL_COMMAND_STRING, rand.nextBoolean()) - .setBool(BuildLanguageOptions.INCOMPATIBLE_STRUCT_HAS_NO_METHODS, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_REQUIRE_LINKER_INPUT_CC_API, rand.nextBoolean()) .setBool( BuildLanguageOptions.INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC, rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java index bd27cacb351ecc..a5355b338050b3 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java @@ -1507,16 +1507,6 @@ private void checkTextMessage(String from, String... lines) throws Exception { assertThat(result).isEqualTo(expect); } - @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')"); - - ev.checkEvalErrorContains( - "cannot override built-in struct function 'to_proto'", "struct(to_proto='foo')"); - } - @Test public void testSimpleTextMessages() throws Exception { checkTextMessage("proto.encode_text(struct(name='value'))", "name: \"value\""); @@ -1664,16 +1654,8 @@ private void checkJson(String from, String expected) throws Exception { @Test public void testStarlarkJsonModule() throws Exception { - // struct.to_json is deprecated. - // java.starlark.net's json module is its replacement. - setBuildLanguageOptions("--incompatible_struct_has_no_methods=false"); checkJson("json.encode(struct(name=True))", "{\"name\":true}"); checkJson("json.encode([1, 2])", "[1,2]"); // works for non-structs too - checkJson("str(dir(struct()))", "[\"to_json\", \"to_proto\"]"); - - setBuildLanguageOptions("--incompatible_struct_has_no_methods=true"); - ev.checkEvalErrorContains("no field or method 'to_json'", "struct(name=True).to_json()"); - checkJson("str(dir(struct()))", "[]"); // no to_{json,proto} } @Test