diff options
author | 2018-01-11 13:35:54 -0800 | |
---|---|---|
committer | 2018-01-11 13:38:04 -0800 | |
commit | b549457d45c0759737785ef21ffcbb6bd9c285f7 (patch) | |
tree | 79a487fb0b5969de9fd512903ba8a7251f990b6b | |
parent | 852111be336776dceaab58ddbcb41ce99c19d812 (diff) |
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, |