--incompatible_existing_rules_immutable_view breaks json encoding of existing_rule/s objects

[tetromino]

Originally posted by @dws in #13907 (comment)

If --incompatible_existing_rules_immutable_view is set, json.encode (or encode_indent) will encode an immutable native.existing_rule or existing_rules object as an array of field names, instead of a dict (because our DictLikeView object is a StarlarkIterable).

proto.encode_text is worse: it will fail with Error in encode_text: [...] got ExistingRulesView, want string, int, bool, or struct when trying to encode a struct one of whose fields has an existing_rule object as value.

Although there is a workaround (encode dict(native.existing_rule("foo").items())), this is obviously a regression compared to the situation before the flag flip.

[tetromino]

I think a solution would have the following shape:

• fix Avoid throwing NotRepresentableException in StarlarkNativeModule.StarlarkifyValue() #13829
• ... allowing us to have a non-throwing version of get() for DictLikeView, in turn allowing DictLikeView to extend Map (see Allow dict(), **, and | to operate on non-dict mappings #13605 (comment) for why we want to extend Map and not go with a custom map-like creature)
• ... which would immediately fix json; for proto, we'd have to allow it to support arbitrary mappings, not just Dict.

An alternative would be for DictLikeView to implement Json.Encodable (whose interface we'd need to change a bit), and the corresponding ProtoModule.Encodable (which we'd need to add) - but IMHO that would be quite ugly. Especially the duplication between the two encodable interfaces.

[tetromino]

@brandjon fyi

[tetromino]

After looking at this a bit, proto.encode_text cannot reasonably support the output of native.existing_rule(): the encoder cannot produce any reasonable schema for a dict (or dict-like entity) whose values may be lists/tuples, as opposed to scalars. Which is a reasonable limitation, considering the proto format.

We do still want to fix json encoding, however.

[meteorcloudy]

@comius Is this something you can look into? This is also blocking the 6.0 release.

[tetromino]

@meteorcloudy - I have a fix in review internally

[meteorcloudy]

Bazel 6.0 cut is scheduled on Oct 10th, do you think we can fix this in time?

[tetromino]

I hope to submit the fix this week