Make it an error to attempt to expand an attribute that does not exist
PiperOrigin-RevId: 171684595
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Expander.java b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java
index be34e5d..c4eb867 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/Expander.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/Expander.java
@@ -172,14 +172,10 @@
/**
* Obtains the value of the attribute, expands all values, and returns the resulting list. If the
- * attribute does not exist or is not of type {@link Type#STRING_LIST}, then this method returns
- * an empty list.
+ * attribute does not exist or is not of type {@link Type#STRING_LIST}, then this method throws
+ * an error.
*/
public ImmutableList<String> list(String attrName) {
- if (!ruleContext.getRule().isAttrDefined(attrName, Type.STRING_LIST)) {
- // TODO(bazel-team): This should be an error.
- return ImmutableList.of();
- }
return list(attrName, ruleContext.attributes().get(attrName, Type.STRING_LIST));
}
@@ -192,13 +188,9 @@
/**
* Obtains the value of the attribute, expands, and tokenizes all values. If the attribute does
- * not exist or is not of type {@link Type#STRING_LIST}, then this method returns an empty list.
+ * not exist or is not of type {@link Type#STRING_LIST}, then this method throws an error.
*/
public ImmutableList<String> tokenized(String attrName) {
- if (!ruleContext.getRule().isAttrDefined(attrName, Type.STRING_LIST)) {
- // TODO(bazel-team): This should be an error.
- return ImmutableList.of();
- }
return tokenized(attrName, ruleContext.attributes().get(attrName, Type.STRING_LIST));
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
index 230b1fd..9a935ff 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java
@@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
@@ -427,6 +428,11 @@
private static CommandLine computeArgs(
RuleContext ruleContext,
CommandLine additionalArgs) {
+ if (!ruleContext.getRule().isAttrDefined("args", Type.STRING_LIST)) {
+ // Some non-_binary rules create RunfilesSupport instances; it is fine to not have an args
+ // attribute here.
+ return additionalArgs;
+ }
return CommandLine.concat(
ruleContext.getExpander().withDataLocations().tokenized("args"),
additionalArgs);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
index adfb310..2e58a19 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java
@@ -193,6 +193,8 @@
fromTemplates("%{name}_images/emulator-meta-data.pb");
static final FileType APK = FileType.of(".apk");
+ public static final String NOCOMPRESS_EXTENSIONS_ATTR = "nocompress_extensions";
+
/** The default label of android_sdk option */
public static LateBoundDefault<?, Label> getAndroidSdkLabel(Label androidSdk) {
return LateBoundDefault.fromTargetConfiguration(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
index 8d9d95c..3431162 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApkActionsBuilder.java
@@ -25,9 +25,11 @@
import com.google.devtools.build.lib.rules.java.JavaCommon;
import com.google.devtools.build.lib.rules.java.JavaHelper;
import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
+import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.List;
/**
* A class for coordinating APK building, signing and zipaligning.
@@ -337,8 +339,18 @@
singleJarCommandLine.addExecPath("--sources", inputZip);
}
- ImmutableList<String> noCompressExtensions =
- ruleContext.getExpander().withDataLocations().tokenized("nocompress_extensions");
+ List<String> noCompressExtensions;
+ if (ruleContext.getRule().isAttrDefined(
+ AndroidRuleClasses.NOCOMPRESS_EXTENSIONS_ATTR, Type.STRING_LIST)) {
+ noCompressExtensions =
+ ruleContext
+ .getExpander()
+ .withDataLocations()
+ .tokenized(AndroidRuleClasses.NOCOMPRESS_EXTENSIONS_ATTR);
+ } else {
+ // This code is also used by android_test, which doesn't have this attribute.
+ noCompressExtensions = ImmutableList.of();
+ }
if (!noCompressExtensions.isEmpty()) {
compressedApkCommandLine.addAll("--nocompress_suffixes", noCompressExtensions);
singleJarCommandLine.addAll("--nocompress_suffixes", noCompressExtensions);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
index bf89607..38e4d57 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/ApplicationManifest.java
@@ -804,8 +804,18 @@
ResourceFilter resourceFilter = ResourceFilter.fromRuleContext(ruleContext);
- List<String> uncompressedExtensions =
- ruleContext.getExpander().withDataLocations().tokenized("nocompress_extensions");
+ List<String> uncompressedExtensions;
+ if (ruleContext.getRule().isAttrDefined(
+ AndroidRuleClasses.NOCOMPRESS_EXTENSIONS_ATTR, Type.STRING_LIST)) {
+ uncompressedExtensions =
+ ruleContext
+ .getExpander()
+ .withDataLocations()
+ .tokenized(AndroidRuleClasses.NOCOMPRESS_EXTENSIONS_ATTR);
+ } else {
+ // This code is also used by android_test, which doesn't have this attribute.
+ uncompressedExtensions = ImmutableList.of();
+ }
ImmutableList.Builder<String> additionalAaptOpts = ImmutableList.builder();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
index 9e46ef9..5ec3bd3 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaSkylarkCommon.java
@@ -40,6 +40,7 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.SkylarkList;
import com.google.devtools.build.lib.syntax.SkylarkNestedSet;
+import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import java.util.List;
import javax.annotation.Nullable;
@@ -511,9 +512,17 @@
ConfiguredTarget javaToolchainConfigTarget =
(ConfiguredTarget) skylarkRuleContext.getAttr().getValue(javaToolchainAttr);
JavaToolchainProvider toolchain = getJavaToolchainProvider(javaToolchainConfigTarget);
+ ImmutableList<String> javacOptsFromAttr;
+ if (ruleContext.getRule().isAttrDefined("javacopts", Type.STRING_LIST)) {
+ javacOptsFromAttr = ruleContext.getExpander().withDataLocations().tokenized("javacopts");
+ } else {
+ // This can also be called from Skylark rules that may or may not have an appropriate
+ // javacopts attribute.
+ javacOptsFromAttr = ImmutableList.of();
+ }
return ImmutableList.copyOf(Iterables.concat(
toolchain.getJavacOptions(),
- ruleContext.getExpander().withDataLocations().tokenized("javacopts")));
+ javacOptsFromAttr));
}
@SkylarkCallable(
