Move custom err msg functionality from Info to SkylarkInfo

Info objects no longer store a string pointer for their error message format, which is almost always the same as the one specified by their provider type. Only map-based SkylarkInfo used this (for fancy built-in structs like ctx.attr), so the field is pushed down to there.

RELNOTES: None
PiperOrigin-RevId: 181654641

3 files changed, 43 insertions, 37 deletions

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 cc9708c351..2714379355 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
@@ -57,33 +57,16 @@ public abstract class Info implements ClassObject, SkylarkValue, Serializable {
    */
   private final Location location;
 
-  /** Formattable string with one {@code '%s'} placeholder for the missing field name. */
-  private final String errorMessageFormatForUnknownField;
-
-  // 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.
-
   /**
    * Constructs an {@link Info}.
    *
    * @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}.
    */
-  protected Info(
-      Provider provider,
-      @Nullable Location location,
-      @Nullable String errorMessageFormatForUnknownField) {
+  protected Info(Provider provider, @Nullable Location location) {
     this.provider = Preconditions.checkNotNull(provider);
     this.location = location == null ? Location.BUILTIN : location;
-    this.errorMessageFormatForUnknownField =
-        errorMessageFormatForUnknownField == null
-            ? provider.getErrorMessageFormatForUnknownField()
-            : errorMessageFormatForUnknownField;
   }
 
   /**
@@ -155,11 +138,20 @@ public abstract class Info implements ClassObject, SkylarkValue, Serializable {
   @Override
   public abstract ImmutableCollection<String> getFieldNames();
 
+  /**
+   * Returns the error message format to use for unknown fields.
+   *
+   * <p>By default, it is the one specified by the provider.
+   */
+  protected String getErrorMessageFormatForUnknownField() {
+    return provider.getErrorMessageFormatForUnknownField();
+  }
+
   @Override
   public String getErrorMessageForUnknownField(String name) {
     String suffix = "Available attributes: "
         + Joiner.on(", ").join(Ordering.natural().sortedCopy(getFieldNames()));
-    return String.format(errorMessageFormatForUnknownField, name) + "\n" + suffix;
+    return String.format(getErrorMessageFormatForUnknownField(), name) + "\n" + suffix;
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
index 16f8f9ed13..e1035c86e9 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeInfo.java
@@ -39,12 +39,12 @@ public class NativeInfo extends Info {
   }
 
   public NativeInfo(NativeProvider<?> provider) {
-    super(provider, Location.BUILTIN, /*errorMessageFormatForUnknownField=*/ null);
+    super(provider, Location.BUILTIN);
     this.values = ImmutableMap.of();
   }
 
   public NativeInfo(NativeProvider<?> provider, Map<String, Object> values, Location loc) {
-    super(provider, loc, /*errorMessageFormatForUnknownField=*/ null);
+    super(provider, loc);
     this.values = copyValues(values);
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
index e8e75674bc..7a6a458ecd 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkInfo.java
@@ -39,11 +39,8 @@ import javax.annotation.Nullable;
 public abstract class SkylarkInfo extends Info implements Concatable {
 
   // Private because this should not be subclassed outside this file.
-  private SkylarkInfo(
-      Provider provider,
-      @Nullable Location loc,
-      @Nullable String errorMessageFormatForUnknownField) {
-    super(provider, loc, errorMessageFormatForUnknownField);
+  private SkylarkInfo(Provider provider, @Nullable Location loc) {
+    super(provider, loc);
   }
 
   @Override
@@ -90,10 +87,15 @@ public abstract class SkylarkInfo extends Info implements Concatable {
    * Creates a schemaless (map-based) provider instance with the given provider type, field values,
    * and unknown-field error message.
    *
-   * <p>The creation location will be {@link Location#BUILTIN}.
-   *
    * <p>This is used to create structs for special purposes, such as {@code ctx.attr} and the
-   * {@code native} module.
+   * {@code native} module. The creation location will be {@link Location#BUILTIN}.
+   *
+   * <p>{@code errorMessageFormatForUnknownField} is a string format, as for {@link
+   * Provider#getErrorMessageFormatForUnknownField}.
+   *
+   * <p>It is preferred to not use this method. Instead, create a new subclass of {@link
+   * NativeProvider} with the desired error message format, and create a corresponding {@link
+   * NativeInfo} subclass.
    */
   // TODO(bazel-team): Make the special structs that need a custom error message use a different
   // provider (subclassing NativeProvider) and a different Info implementation. Then remove this
@@ -123,8 +125,7 @@ public abstract class SkylarkInfo extends Info implements Concatable {
   public static SkylarkInfo createSchemaful(
       SkylarkProvider provider, Object[] values, @Nullable Location loc) {
     Preconditions.checkArgument(provider.getLayout() != null, "provider cannot be schemaless");
-    return new CompactSkylarkInfo(
-        provider, provider.getLayout(), values, loc, /*errorMessageFormatForUnknownField=*/ null);
+    return new CompactSkylarkInfo(provider, provider.getLayout(), values, loc);
   }
 
   /**
@@ -142,8 +143,7 @@ public abstract class SkylarkInfo extends Info implements Concatable {
       ImmutableMap<String, Integer> layout,
       Object[] values,
       @Nullable Location loc) {
-    return new CompactSkylarkInfo(
-        provider, layout, values, loc, /*errorMessageFormatForUnknownField=*/ null);
+    return new CompactSkylarkInfo(provider, layout, values, loc);
   }
 
   /** Returns the layout for this provider if it is schemaful, null otherwise. */
@@ -159,13 +159,22 @@ public abstract class SkylarkInfo extends Info implements Concatable {
 
     private final ImmutableMap<String, Object> values;
 
+    /**
+     * Formattable string with one {@code '%s'} placeholder for the missing field name.
+     *
+     * <p>If null, uses the default format specified by the provider.
+     */
+    @Nullable
+    private final String errorMessageFormatForUnknownField;
+
     MapBackedSkylarkInfo(
         Provider provider,
         Map<String, Object> values,
         @Nullable Location loc,
         @Nullable String errorMessageFormatForUnknownField) {
-      super(provider, loc, errorMessageFormatForUnknownField);
+      super(provider, loc);
       this.values = copyValues(values);
+      this.errorMessageFormatForUnknownField = errorMessageFormatForUnknownField;
     }
 
     @Override
@@ -189,6 +198,12 @@ public abstract class SkylarkInfo extends Info implements Concatable {
     }
 
     @Override
+    protected String getErrorMessageFormatForUnknownField() {
+      return errorMessageFormatForUnknownField != null
+          ? errorMessageFormatForUnknownField : super.getErrorMessageFormatForUnknownField();
+    }
+
+    @Override
     public ImmutableMap<String, Integer> getLayout() {
       return null;
     }
@@ -205,9 +220,8 @@ public abstract class SkylarkInfo extends Info implements Concatable {
         Provider provider,
         ImmutableMap<String, Integer> layout,
         Object[] values,
-        @Nullable Location loc,
-        @Nullable String errorMessageFormatForUnknownField) {
-      super(provider, loc, errorMessageFormatForUnknownField);
+        @Nullable Location loc) {
+      super(provider, loc);
       this.layout = Preconditions.checkNotNull(layout);
       Preconditions.checkArgument(
           layout.size() == values.length,