Refactor SkylarkInfo constructors

- Info now has one protected constructor. (Would've preferred the builder pattern, but inheritance makes it much more verbose.)
- Direct SkylarkInfo subclass access is replaced by factory methods and an isCompact() accessor.
- Added/simplified tests

RELNOTES: None
PiperOrigin-RevId: 181616757

src/main/java/com/google/devtools/build/lib/analysis/ActionsProvider.java

@@ -46,6 +46,6 @@ public static Info create(Iterable<ActionAnalysisMetadata> actions) {
       }
     }
     ImmutableMap<String, Object> fields = ImmutableMap.<String, Object>of("by_file", map);
-    return SkylarkInfo.fromMap(SKYLARK_CONSTRUCTOR, fields, Location.BUILTIN);
+    return SkylarkInfo.createSchemaless(SKYLARK_CONSTRUCTOR, fields, Location.BUILTIN);
   }
 }

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

@@ -55,45 +55,35 @@ public abstract class Info implements ClassObject, SkylarkValue, Serializable {
    *
    * <p>Built-in provider instances may use {@link Location#BUILTIN}.
    */
-  private final Location creationLoc;
+  private final Location location;
 
-  /**
-   * Formattable string with one {@code '%s'} placeholder for the missing field name.
-   */
+  /** Formattable string with one {@code '%s'} placeholder for the missing field name. */
   private final String errorMessageFormatForUnknownField;
 
-  /**
-   * Creates an empty struct with a given location.
-   *
-   * <p>If {@code location} is null, it defaults to {@link Location#BUILTIN}.
-   */
-  public Info(Provider provider, @Nullable Location location) {
-    this.provider = provider;
-    this.creationLoc = location == null ? Location.BUILTIN : location;
-    this.errorMessageFormatForUnknownField = provider.getErrorMessageFormatForUnknownField();
-  }
-
-  /** Creates a built-in struct (i.e. without a Skylark creation location). */
-  public Info(Provider provider) {
-    this.provider = provider;
-    this.creationLoc = Location.BUILTIN;
-    this.errorMessageFormatForUnknownField = provider.getErrorMessageFormatForUnknownField();
-  }
+  // Ideally we'd use a builder pattern for Info, but that would be cumbersome since it doesn't mix
+  // well with inheritance. Instead, we just use nullable constructor args.
 
   /**
-   * Creates a built-in struct (i.e. without creation location) that uses a specific error message
-   * for missing fields.
+   * Constructs an {@link Info}.
    *
-   * <p>Only used in {@link
-   * com.google.devtools.build.lib.packages.NativeProvider.StructConstructor#create(Map, String)}.
-   * If you need to override an error message, the preferred way is to create a new {@link
-   * NativeProvider} subclass.
+   * @param provider the provider describing the type of this instance
+   * @param location the Skylark location where this instance is created. If null, defaults to
+   *     {@link Location#BUILTIN}.
+   * @param errorMessageFormatForUnknownField a string format, as for {@link
+   *     Provider#getErrorMessageFormatForUnknownField}. If null, defaults to the format specified
+   *     by the provider. It is preferred to not use this field; instead, create a new subclass of
+   *     {@link NativeProvider}.
    */
-  Info(Provider provider, String errorMessageFormatForUnknownField) {
-    this.provider = provider;
-    this.creationLoc = Location.BUILTIN;
+  protected Info(
+      Provider provider,
+      @Nullable Location location,
+      @Nullable String errorMessageFormatForUnknownField) {
+    this.provider = Preconditions.checkNotNull(provider);
+    this.location = location == null ? Location.BUILTIN : location;
     this.errorMessageFormatForUnknownField =
-        Preconditions.checkNotNull(errorMessageFormatForUnknownField);
+        errorMessageFormatForUnknownField == null
+            ? provider.getErrorMessageFormatForUnknownField()
+            : errorMessageFormatForUnknownField;
   }
 
   /**
@@ -103,6 +93,7 @@ public Info(Provider provider) {
    * <p>This preserves the order of the map entries.
    */
   protected static ImmutableMap<String, Object> copyValues(Map<String, Object> values) {
+    Preconditions.checkNotNull(values);
     ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder();
     for (Map.Entry<String, Object> e : values.entrySet()) {
       builder.put(
@@ -118,7 +109,7 @@ protected static ImmutableMap<String, Object> copyValues(Map<String, Object> val
    * <p>Builtin provider instances may return {@link Location#BUILTIN}.
    */
   public Location getCreationLoc() {
-    return creationLoc;
+    return location;
   }
 
   public Provider getProvider() {

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

@@ -39,12 +39,12 @@ public ImmutableCollection<String> getFieldNames() {
   }
 
   public NativeInfo(NativeProvider<?> provider) {
-    super(provider, Location.BUILTIN);
+    super(provider, Location.BUILTIN, /*errorMessageFormatForUnknownField=*/ null);
     this.values = ImmutableMap.of();
   }
 
   public NativeInfo(NativeProvider<?> provider, Map<String, Object> values, Location loc) {
-    super(provider, loc);
+    super(provider, loc, /*errorMessageFormatForUnknownField=*/ null);
     this.values = copyValues(values);
   }
 

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

@@ -79,7 +79,7 @@ private StructProvider() {
     protected Info createInstanceFromSkylark(Object[] args, Location loc) {
       @SuppressWarnings("unchecked")
       Map<String, Object> kwargs = (Map<String, Object>) args[0];
-      return SkylarkInfo.fromMap(this, kwargs, loc);
+      return SkylarkInfo.createSchemaless(this, kwargs, loc);
     }
 
     /**
@@ -90,12 +90,13 @@ protected Info createInstanceFromSkylark(Object[] args, Location loc) {
      * {@code ctx.attr}.
      * */
     public Info create(Map<String, Object> values, String errorMessageFormatForUnknownField) {
-      return new SkylarkInfo.MapBackedSkylarkInfo(this, values, errorMessageFormatForUnknownField);
+      return SkylarkInfo.createSchemalessWithCustomMessage(
+          this, values, errorMessageFormatForUnknownField);
     }
 
     /** Creates an empty struct with the given location. */
     public Info createEmpty(Location loc) {
-      return SkylarkInfo.fromMap(this, ImmutableMap.of(), loc);
+      return SkylarkInfo.createSchemaless(this, ImmutableMap.of(), loc);
     }
   }