diff options
author | 2017-12-27 12:01:27 -0800 | |
---|---|---|
committer | 2017-12-27 12:03:35 -0800 | |
commit | 11bcd246bc56f5f0dfc3f9b95af42b5cfc94c575 (patch) | |
tree | 2c9f2909914af8a722c0162c24433ac960a59e4d /src/main/java | |
parent | 4af309d4b24f65994481203211c6ea7bec388bed (diff) |
Refactor Info class
This simplifies the location field (now non-nullable), removes a couple redundant loc args, and clarifies the type's description and some javadoc.
RELNOTES: None
PiperOrigin-RevId: 180210943
Diffstat (limited to 'src/main/java')
6 files changed, 69 insertions, 41 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java index fcf06d8fdb..c30a9ed394 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredAspect.java @@ -205,7 +205,7 @@ public final class ConfiguredAspect { return this; } - public Builder addSkylarkDeclaredProvider(Info declaredProvider, Location loc) + public Builder addSkylarkDeclaredProvider(Info declaredProvider) throws EvalException { Provider constructor = declaredProvider.getProvider(); if (!constructor.isExported()) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java index 8f53a9b27b..93b2df3771 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java @@ -291,7 +291,7 @@ public final class RuleConfiguredTargetBuilder { * * <p>Use {@link #addNativeDeclaredProvider(Info)} in definitions of native rules. */ - public RuleConfiguredTargetBuilder addSkylarkDeclaredProvider(Info provider, Location loc) + public RuleConfiguredTargetBuilder addSkylarkDeclaredProvider(Info provider) throws EvalException { Provider constructor = provider.getProvider(); if (!constructor.isExported()) { @@ -313,7 +313,7 @@ public final class RuleConfiguredTargetBuilder { * Adds "declared providers" defined in native code to the rule. Use this method for declared * providers in definitions of native rules. * - * <p>Use {@link #addSkylarkDeclaredProvider(Info, Location)} for Skylark rule implementations. + * <p>Use {@link #addSkylarkDeclaredProvider(Info)} for Skylark rule implementations. */ public RuleConfiguredTargetBuilder addNativeDeclaredProviders(Iterable<Info> providers) { for (Info provider : providers) { @@ -326,7 +326,7 @@ public final class RuleConfiguredTargetBuilder { * Adds a "declared provider" defined in native code to the rule. Use this method for declared * providers in definitions of native rules. * - * <p>Use {@link #addSkylarkDeclaredProvider(Info, Location)} for Skylark rule implementations. + * <p>Use {@link #addSkylarkDeclaredProvider(Info)} for Skylark rule implementations. */ public RuleConfiguredTargetBuilder addNativeDeclaredProvider(Info provider) { Provider constructor = provider.getProvider(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java index deefbcb972..7f3a3b15b2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleConfiguredTargetUtil.java @@ -320,9 +320,7 @@ public final class SkylarkRuleConfiguredTargetUtil { parseDefaultProviderKeys(declaredProvider, context, builder); defaultProviderProvidedExplicitly = true; } else { - Location creationLoc = declaredProvider.getCreationLocOrNull(); - builder.addSkylarkDeclaredProvider( - declaredProvider, creationLoc == null ? loc : creationLoc); + builder.addSkylarkDeclaredProvider(declaredProvider); } } @@ -495,7 +493,7 @@ public final class SkylarkRuleConfiguredTargetUtil { if (ruleContext.getRule().getRuleClassObject().isSkylarkTestable()) { Info actions = ActionsProvider.create(ruleContext.getAnalysisEnvironment().getRegisteredActions()); - builder.addSkylarkDeclaredProvider(actions, loc); + builder.addSkylarkDeclaredProvider(actions); } } 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 2c6d3169d2..550ef181ea 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 @@ -35,62 +35,91 @@ import java.util.List; import java.util.Map; import javax.annotation.Nullable; -/** Represents information provided by a {@link Provider}. */ +/** An instance (in the Skylark sense, not Java) of a {@link Provider}. */ @SkylarkModule( name = "struct", category = SkylarkModuleCategory.BUILTIN, doc = - "A special language element to support structs (i.e. simple value objects). " - + "See the global <a href=\"globals.html#struct\">struct</a> function " - + "for more details." + "A generic object with fields. See the global <a href=\"globals.html#struct\"><code>struct" + + "</code></a> function for more details." + + "<p>Structs fields cannot be reassigned once the struct is created. Two structs are " + + "equal if they have the same fields and if corresponding field values are equal." ) public abstract class Info implements ClassObject, SkylarkValue, Serializable { + + /** The {@link Provider} that describes the type of this instance. */ private final Provider provider; + + /** + * The Skylark location where this provider instance was created. + * + * <p>Built-in provider instances may use {@link Location#BUILTIN}. + */ private final Location creationLoc; - private final String errorMessage; - /** Creates an empty struct with a given location. */ - public Info(Provider provider, Location location) { + /** + * 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; - this.errorMessage = provider.getErrorMessageFormatForInstances(); + this.creationLoc = location == null ? Location.BUILTIN : location; + this.errorMessageFormatForUnknownField = provider.getErrorMessageFormatForInstances(); } - /** Creates a built-in struct (i.e. without creation loc). */ + /** Creates a built-in struct (i.e. without a Skylark creation location). */ public Info(Provider provider) { this.provider = provider; - this.creationLoc = null; - this.errorMessage = provider.getErrorMessageFormatForInstances(); + this.creationLoc = Location.BUILTIN; + this.errorMessageFormatForUnknownField = provider.getErrorMessageFormatForInstances(); } /** - * Creates a built-in struct (i.e. without creation loc). + * Creates a built-in struct (i.e. without creation location) that uses a specific error message + * for missing fields. * - * <p>Allows to supply a specific error message. Only used in - * {@link com.google.devtools.build.lib.packages.NativeProvider.StructConstructor#create(Map, - * String)} If you need to override an error message, preferred way is to create a specific {@link - * NativeProvider}. + * <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. */ - Info(Provider provider, String errorMessage) { + Info(Provider provider, String errorMessageFormatForUnknownField) { this.provider = provider; - this.creationLoc = null; - this.errorMessage = Preconditions.checkNotNull(errorMessage); + this.creationLoc = Location.BUILTIN; + this.errorMessageFormatForUnknownField = + Preconditions.checkNotNull(errorMessageFormatForUnknownField); } - // Ensure that values are all acceptable to Skylark before to stuff them in a ClassObject + /** + * Preprocesses a map of field values to convert the field names and field values to + * Skylark-acceptable names and types. + */ protected static ImmutableMap<String, Object> copyValues(Map<String, Object> values) { ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder(); for (Map.Entry<String, Object> e : values.entrySet()) { builder.put( - Attribute.getSkylarkName(e.getKey()), SkylarkType.convertToSkylark(e.getValue(), null)); + Attribute.getSkylarkName(e.getKey()), + SkylarkType.convertToSkylark(e.getValue(), /*env=*/ null)); } return builder.build(); } + /** + * Returns whether the given field name exists. + * + * <p>The "key" nomenclature is historic and for consistency with {@link ClassObject}. + */ + // TODO(bazel-team): Rename to hasField(), and likewise in ClassObject. public abstract boolean hasKey(String name); /** Returns a value and try to cast it into specified type */ - public <TYPE> TYPE getValue(String key, Class<TYPE> type) throws EvalException { + public <T> T getValue(String key, Class<T> type) throws EvalException { Object obj = getValue(key); if (obj == null) { return null; @@ -99,19 +128,19 @@ public abstract class Info implements ClassObject, SkylarkValue, Serializable { return type.cast(obj); } + /** + * Returns the Skylark location where this provider instance was created. + * + * <p>Builtin provider instances may return {@link Location#BUILTIN}. + */ public Location getCreationLoc() { - return Preconditions.checkNotNull(creationLoc, "This struct was not created in a Skylark code"); + return creationLoc; } public Provider getProvider() { return provider; } - @Nullable - public Location getCreationLocOrNull() { - return creationLoc; - } - /** * Returns the fields of this struct. * @@ -132,11 +161,12 @@ public abstract class Info implements ClassObject, SkylarkValue, Serializable { @Override public abstract Object getValue(String name); + // TODO(bazel-team): Rename to getErrorMessageForUnknownField. @Override public String errorMessage(String name) { String suffix = "Available attributes: " + Joiner.on(", ").join(Ordering.natural().sortedCopy(getKeys())); - return String.format(errorMessage, name) + "\n" + suffix; + return String.format(errorMessageFormatForUnknownField, name) + "\n" + suffix; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/packages/Provider.java b/src/main/java/com/google/devtools/build/lib/packages/Provider.java index 2616a209e0..028a03d446 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/Provider.java +++ b/src/main/java/com/google/devtools/build/lib/packages/Provider.java @@ -95,8 +95,9 @@ public abstract class Provider extends BaseFunction { /** * Returns an error message format for instances. * - * <p>Must contain one '%s' placeholder for field name. + * <p>Must contain one {@code '%s'} placeholder for field name. */ + // TODO(bazel-team): Rename to getErrorMessageFormatForUnknownField(). public abstract String getErrorMessageFormatForInstances(); public SkylarkProviderIdentifier id() { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java index ca57cbc88e..24112a4fae 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java @@ -155,8 +155,7 @@ public class SkylarkAspectFactory implements ConfiguredAspectFactory { + "a sequence of declared providers, instead got a %s at index %d", o.getClass(), i); - Location creationLoc = declaredProvider.getCreationLocOrNull(); - builder.addSkylarkDeclaredProvider(declaredProvider, creationLoc != null ? creationLoc : loc); + builder.addSkylarkDeclaredProvider(declaredProvider); i++; } } |