bazel packages: minor StructProvider clean-ups
- Remove concept of "error format strings" from Provider interface.
Error messages are computed by a method. (Only the StarlarkInfo subclass
now talks about format strings).
- Eliminate StructProvider.create(Map, Location) by inlining 4 calls.
- Improve wording of StarlarkProvider unknown field error message.
- Add TODO notes on the challenge of moving the unknown field error
messages entirely into Providers.
This CL contains the uncomplicated parts a failed attempt to do those TODOs.
PiperOrigin-RevId: 341647758
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Info.java b/src/main/java/com/google/devtools/build/lib/packages/Info.java
index c009f85..61e1ada 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Info.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Info.java
@@ -27,16 +27,19 @@
// - Once to_{json,proto} are gone, StructApi can be deleted; structs should never again have
// methods.
// - StructImpl.location can be pushed down into subclasses that need it, much as we did for
-// StructImpl.provider in this CL.
-// - getErrorMessageFormatForUnknownField can become a method on provider.
-// It should compute a string from a parameter, not use higher-order formatting.
+// StructImpl.provider in CL 341102857.
// - StructImpl is then really just a collection of helper functions for subclasses
-// getValue(String, Class), repr, equals, hash. Move them, and merge it into Info interface.
+// getValue(String, Class), repr, equals, hash. Move them, and merge it into Info interface,
+// or rename it InfoStruct or StructuredInfo if we absolutely need inheritance.
// - Move StructProvider.STRUCT and make StructProvider private.
// The StructProvider.createStruct method could be a simple function like depset, select.
// StructProviderApi could be eliminated.
// - eliminate StarlarkInfo + StarlarkInfo.
-// - NativeInfo's two methods can (IIUC) be deleted immediately, and then NativeInfo itself.
+// - NativeInfo's get{FieldNames,Value} methods are not needed by the Starlark interpreter,
+// since all its fields are annotated. They exist for the hash/eq/str implementations
+// defined in StructImpl over all its subclasses, and for json.encode. More thought is
+// needed on how to bridge between annotated methods and user-defined Structures so that
+// they appear similar to clients like json.encode.
//
// Info (result of analysis)
// - StructImpl (structure with fields, to_{json,proto}). Implements Structure, StructApi.
