Skip to content

Commit b11068b

Browse files
tetrominocopybara-github
authored andcommitted
Fix support for File objects in proto.encode_text
Follow-up to bazelbuild@5a99b45, which had the effect of adding proto.encode_text support for File objects - but encoding them very inconveniently (so that they could not be used as elements of a map or list in the overall object to be encoded). Instead, let's use a structure as the encoding proxy object. Note that this doesn't affect json encoding in any way. PiperOrigin-RevId: 866110711 Change-Id: Ib4cb1a3df10767a38c5231c5410e5ba816f24151
1 parent b5f2b34 commit b11068b

File tree

5 files changed

+101
-68
lines changed

5 files changed

+101
-68
lines changed

src/main/java/com/google/devtools/build/lib/actions/Artifact.java

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@
6060
import net.starlark.java.eval.Printer;
6161
import net.starlark.java.eval.Starlark;
6262
import net.starlark.java.eval.StarlarkSemantics;
63+
import net.starlark.java.eval.Structure;
64+
import net.starlark.java.lib.MapWrapperStructure;
6365
import net.starlark.java.lib.StarlarkEncodable;
6466

6567
/**
@@ -717,14 +719,15 @@ public final String toString() {
717719
}
718720

719721
@Override
720-
public Object objectForEncoding(StarlarkSemantics semantics) {
721-
return ImmutableMap.of(
722-
"path",
723-
getExecPathStringForStarlark(semantics),
724-
"root",
725-
getRootForStarlark(semantics).getExecPathString(),
726-
"short_path",
727-
getRunfilesPathString());
722+
public Structure objectForEncoding(StarlarkSemantics semantics) {
723+
return new MapWrapperStructure(
724+
ImmutableMap.of(
725+
"path",
726+
getExecPathStringForStarlark(semantics),
727+
"root",
728+
getRootForStarlark(semantics).getExecPathString(),
729+
"short_path",
730+
getRunfilesPathString()));
728731
}
729732

730733
/** Returns a string representing the complete artifact path information. */

src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java

Lines changed: 4 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import com.google.common.base.Objects;
1919
import com.google.common.collect.ImmutableMap;
20-
import com.google.common.collect.ImmutableSet;
2120
import com.google.common.collect.Ordering;
2221
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
2322
import java.util.List;
@@ -29,6 +28,7 @@
2928
import net.starlark.java.eval.Starlark;
3029
import net.starlark.java.eval.StarlarkSemantics;
3130
import net.starlark.java.eval.Structure;
31+
import net.starlark.java.lib.MapWrapperStructure;
3232
import net.starlark.java.lib.StarlarkEncodable;
3333
import net.starlark.java.syntax.Location;
3434

@@ -126,60 +126,12 @@ public int hashCode() {
126126
*/
127127
@Override
128128
public void repr(Printer printer, StarlarkSemantics semantics) {
129-
new EncodableStructure(this).repr(printer, semantics);
129+
new MapWrapperStructure(getLegacyFields()).repr(printer, semantics);
130130
}
131131

132132
@Override
133-
public Object objectForEncoding(StarlarkSemantics semantics) {
134-
return new EncodableStructure(this);
135-
}
136-
137-
/**
138-
* A {@link Structure} wrapper for a {@link NativeInfo}'s non-method fields; used for text
139-
* encoding.
140-
*/
141-
private static final class EncodableStructure implements Structure {
142-
private final NativeInfo info;
143-
private final ImmutableMap<String, Object> fields;
144-
145-
private EncodableStructure(NativeInfo info) {
146-
this.info = info;
147-
this.fields = info.getLegacyFields();
148-
}
149-
150-
@Override
151-
@Nullable
152-
public Object getValue(String name) {
153-
return fields.get(name);
154-
}
155-
156-
@Override
157-
@Nullable
158-
public ImmutableSet<String> getFieldNames() {
159-
return fields.keySet();
160-
}
161-
162-
@Override
163-
@Nullable
164-
public String getErrorMessageForUnknownField(String field) {
165-
return info.getProvider().getErrorMessageForUnknownField(field);
166-
}
167-
168-
@Override
169-
public void repr(Printer printer, StarlarkSemantics semantics) {
170-
boolean first = true;
171-
printer.append("struct(");
172-
for (var field : fields.entrySet()) {
173-
if (!first) {
174-
printer.append(", ");
175-
}
176-
first = false;
177-
printer.append(field.getKey());
178-
printer.append(" = ");
179-
printer.repr(field.getValue(), semantics);
180-
}
181-
printer.append(")");
182-
}
133+
public Structure objectForEncoding(StarlarkSemantics semantics) {
134+
return new MapWrapperStructure(getLegacyFields());
183135
}
184136

185137
@Override

src/main/java/net/starlark/java/lib/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,7 @@ java_library(
1919
visibility = ["//src/main/java/net/starlark/java:clients"],
2020
deps = [
2121
"//src/main/java/net/starlark/java/eval",
22+
"//third_party:guava",
23+
"//third_party:jsr305",
2224
],
2325
)
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
// Copyright 2026 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package net.starlark.java.lib;
16+
17+
import com.google.common.collect.ImmutableMap;
18+
import com.google.common.collect.ImmutableSet;
19+
import java.util.Map;
20+
import javax.annotation.Nullable;
21+
import net.starlark.java.eval.Printer;
22+
import net.starlark.java.eval.StarlarkSemantics;
23+
import net.starlark.java.eval.Structure;
24+
25+
/** A simple {@link Structure} implementation that wraps a map of fields. */
26+
public class MapWrapperStructure implements Structure {
27+
protected final ImmutableMap<String, Object> fields;
28+
29+
public MapWrapperStructure(Map<String, Object> fields) {
30+
this.fields = ImmutableMap.copyOf(fields);
31+
}
32+
33+
@Override
34+
@Nullable
35+
public Object getValue(String name) {
36+
return fields.get(name);
37+
}
38+
39+
@Override
40+
@Nullable
41+
public ImmutableSet<String> getFieldNames() {
42+
return fields.keySet();
43+
}
44+
45+
@Override
46+
@Nullable
47+
public String getErrorMessageForUnknownField(String field) {
48+
return null;
49+
}
50+
51+
@Override
52+
public void repr(Printer printer, StarlarkSemantics semantics) {
53+
boolean first = true;
54+
printer.append("struct(");
55+
for (var field : fields.entrySet()) {
56+
if (!first) {
57+
printer.append(", ");
58+
}
59+
first = false;
60+
printer.append(field.getKey());
61+
printer.append(" = ");
62+
printer.repr(field.getValue(), semantics);
63+
}
64+
printer.append(")");
65+
}
66+
}

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2150,8 +2150,8 @@ public void testJsonInvalidStructure() throws Exception {
21502150
}
21512151

21522152
@Test
2153-
public void testJsonFileEncoding() throws Exception {
2154-
// Test that File objects can be encoded as JSON
2153+
public void testJsonAndProtoFileEncoding() throws Exception {
2154+
// Test that File objects can be encoded as JSON and proto.
21552155
scratch.file(
21562156
"test/BUILD",
21572157
"""
@@ -2166,20 +2166,30 @@ public void testJsonFileEncoding() throws Exception {
21662166
"test/rule.bzl",
21672167
"""
21682168
def _impl(ctx):
2169-
input = {"file": ctx.file.src, "other": True}
2170-
expected_output = {
2169+
json_input = {"file": ctx.file.src, "other": True}
2170+
json_expected_output = {
21712171
"file": {
21722172
"path": ctx.file.src.path,
21732173
"short_path": ctx.file.src.short_path,
21742174
"root": ctx.file.src.root.path,
21752175
},
21762176
"other": True
21772177
}
2178-
encoded = json.encode(input)
2179-
decoded = json.decode(encoded)
2178+
json_encoded = json.encode(json_input)
2179+
json_decoded = json.decode(json_encoded)
21802180
2181-
if decoded != expected_output:
2182-
fail("JSON encode/decode of File did not round-trip. Expected: {}, actual: {}".format(expected_output, decoded))
2181+
if json_decoded != json_expected_output:
2182+
fail("JSON encode/decode of File did not round-trip. Expected: {}, actual: {}".format(repr(json_expected_output), repr(json_decoded)))
2183+
2184+
proto_input = struct(input = json_input) # proto input must be wrapped in a struct
2185+
proto_expected_encoded = 'input {\\n key: "file"\\n value {\\n path: "%s"\\n root: "%s"\\n short_path: "%s"\\n }\\n}\\ninput {\\n key: "other"\\n value: true\\n}\\n' % (
2186+
ctx.file.src.path,
2187+
ctx.file.src.root.path,
2188+
ctx.file.src.short_path,
2189+
)
2190+
proto_encoded = proto.encode_text(proto_input)
2191+
if proto_encoded != proto_expected_encoded:
2192+
fail("Proto encoding of File failed. Expected: {}, actual: {}".format(repr(proto_expected_encoded), repr(proto_encoded)))
21832193
21842194
return []
21852195

0 commit comments

Comments
 (0)