Skip to content

Commit

Permalink
Remove --incompatible_struct_has_no_methods and associated struct met…
Browse files Browse the repository at this point in the history
…hods.

Removes the disabled struct.to_json() and struct.to_proto() and the flag
that could re-enable them.

#19465

RELNOTES[inc]: Removed --incompatible_struct_has_no_methods

PiperOrigin-RevId: 622844911
Change-Id: Ie6a9b42a0ea6f45bfca40bd59a4ea6163214bebc
  • Loading branch information
c-mita authored and copybara-github committed Apr 8, 2024
1 parent a7c588c commit c3dc389
Show file tree
Hide file tree
Showing 8 changed files with 0 additions and 207 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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.
*
* <p>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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:<br><pre class=language-python>"
+ "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"
+ "</pre>"
+ "<p>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:<br><pre class=language-python>"
+ "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</pre>."
+ "<p>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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -118,14 +116,4 @@ public interface JavaOutputApi<FileT extends FileApi> 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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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\"");
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit c3dc389

Please sign in to comment.