From e506858c14d9ffceedbb22988f7c65fe70df5536 Mon Sep 17 00:00:00 2001
From: cparsons <cparsons@google.com>
Date: Mon, 16 Jul 2018 13:33:33 -0700
Subject: Add --incompatible_disable_deprecated_attr_params to disable some
 deprecated parameters of the skylark attr module.

RELNOTES: None.
PiperOrigin-RevId: 204797954
---
 .../build/lib/analysis/skylark/SkylarkAttr.java    | 20 ++++++++++++++---
 .../build/lib/packages/SkylarkSemanticsCodec.java  |  2 ++
 .../lib/packages/SkylarkSemanticsOptions.java      | 16 +++++++++++++
 .../build/lib/syntax/SkylarkSemantics.java         |  5 +++++
 .../packages/SkylarkSemanticsConsistencyTest.java  |  2 ++
 .../lib/skylark/SkylarkRuleClassFunctionsTest.java | 26 ++++++++++++++++++++++
 tools/build_defs/pkg/pkg.bzl                       | 18 +++++++--------
 7 files changed, 77 insertions(+), 12 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
index 0d1a1aa6c8..a0cf36dab9 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkAttr.java
@@ -147,8 +147,15 @@ public final class SkylarkAttr implements SkylarkAttrApi {
       builder.setPropertyFlag("MANDATORY");
     }
 
-    // TODO(laurentlb): Deprecated, remove in August 2016 (use allow_empty instead).
-    if (containsNonNoneKey(arguments, NON_EMPTY_ARG) && (Boolean) arguments.get(NON_EMPTY_ARG)) {
+    if (containsNonNoneKey(arguments, NON_EMPTY_ARG)
+        && (Boolean) arguments.get(NON_EMPTY_ARG)) {
+      if (env.getSemantics().incompatibleDisableDeprecatedAttrParams()) {
+        throw new EvalException(ast.getLocation(),
+            "'non_empty' is no longer supported. use allow_empty instead. You can use "
+                + "--incompatible_disable_deprecated_attr_params to temporarily disable this "
+                + "check.");
+      }
+
       builder.setPropertyFlag("NON_EMPTY");
     }
 
@@ -168,14 +175,21 @@ public final class SkylarkAttr implements SkylarkAttrApi {
       }
     }
 
-    // TODO(laurentlb): Deprecated, remove in August 2016 (use allow_single_file).
     if (containsNonNoneKey(arguments, SINGLE_FILE_ARG)
         && (Boolean) arguments.get(SINGLE_FILE_ARG)) {
+      if (env.getSemantics().incompatibleDisableDeprecatedAttrParams()) {
+        throw new EvalException(
+            ast.getLocation(),
+            "'single_file' is no longer supported. use allow_single_file instead. You can use "
+                + "--incompatible_disable_deprecated_attr_params to temporarily disable this "
+                + "check.");
+      }
       if (containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
         throw new EvalException(
             ast.getLocation(),
             "Cannot specify both single_file (deprecated) and allow_single_file");
       }
+
       builder.setPropertyFlag("SINGLE_ARTIFACT");
     }
 
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
index 7f147cb422..a8647a8d6e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsCodec.java
@@ -47,6 +47,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     codedOut.writeBoolNoTag(semantics.incompatibleBzlDisallowLoadAfterStatement());
     codedOut.writeBoolNoTag(semantics.incompatibleDepsetIsNotIterable());
     codedOut.writeBoolNoTag(semantics.incompatibleDepsetUnion());
+    codedOut.writeBoolNoTag(semantics.incompatibleDisableDeprecatedAttrParams());
     codedOut.writeBoolNoTag(semantics.incompatibleDisableObjcProviderResources());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDataTransition());
     codedOut.writeBoolNoTag(semantics.incompatibleDisallowDictPlus());
@@ -74,6 +75,7 @@ public final class SkylarkSemanticsCodec implements ObjectCodec<SkylarkSemantics
     builder.incompatibleBzlDisallowLoadAfterStatement(codedIn.readBool());
     builder.incompatibleDepsetIsNotIterable(codedIn.readBool());
     builder.incompatibleDepsetUnion(codedIn.readBool());
+    builder.incompatibleDisableDeprecatedAttrParams(codedIn.readBool());
     builder.incompatibleDisableObjcProviderResources(codedIn.readBool());
     builder.incompatibleDisallowDataTransition(codedIn.readBool());
     builder.incompatibleDisallowDictPlus(codedIn.readBool());
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 8781bb5b59..07989682a4 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -114,6 +114,21 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   )
   public boolean incompatibleDepsetIsNotIterable;
 
+  @Option(
+      name = "incompatible_disable_deprecated_attr_params",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      metadataTags = {
+          OptionMetadataTag.INCOMPATIBLE_CHANGE,
+          OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "If set to true, disable the deprecated parameters 'single_file' and 'non_empty' on "
+              + "skylark attribute definition methods, such as attr.label()."
+  )
+  public boolean incompatibleDisableDeprecatedAttrParams;
+
   @Option(
     name = "incompatible_disable_objc_provider_resources",
     defaultValue = "false",
@@ -327,6 +342,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
         .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
         .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
         .incompatibleDepsetUnion(incompatibleDepsetUnion)
+        .incompatibleDisableDeprecatedAttrParams(incompatibleDisableDeprecatedAttrParams)
         .incompatibleDisableObjcProviderResources(incompatibleDisableObjcProviderResources)
         .incompatibleDisallowDataTransition(incompatibleDisallowDataTransition)
         .incompatibleDisallowDictPlus(incompatibleDisallowDictPlus)
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 5bd711b75e..fc7afa45ca 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -47,6 +47,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean incompatibleDepsetUnion();
 
+  public abstract boolean incompatibleDisableDeprecatedAttrParams();
+
   public abstract boolean incompatibleDisableObjcProviderResources();
 
   public abstract boolean incompatibleDisallowDataTransition();
@@ -96,6 +98,7 @@ public abstract class SkylarkSemantics {
           .incompatibleBzlDisallowLoadAfterStatement(false)
           .incompatibleDepsetIsNotIterable(false)
           .incompatibleDepsetUnion(false)
+          .incompatibleDisableDeprecatedAttrParams(false)
           .incompatibleDisableObjcProviderResources(false)
           .incompatibleDisallowDataTransition(false)
           .incompatibleDisallowDictPlus(false)
@@ -126,6 +129,8 @@ public abstract class SkylarkSemantics {
 
     public abstract Builder incompatibleDepsetUnion(boolean value);
 
+    public abstract Builder incompatibleDisableDeprecatedAttrParams(boolean value);
+
     public abstract Builder incompatibleDisableObjcProviderResources(boolean value);
 
     public abstract Builder incompatibleDisallowDataTransition(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 4a8e303a50..a96d928283 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -123,6 +123,7 @@ public class SkylarkSemanticsConsistencyTest {
         "--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
         "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
         "--incompatible_depset_union=" + rand.nextBoolean(),
+        "--incompatible_disable_deprecated_attr_params=" + rand.nextBoolean(),
         "--incompatible_disable_objc_provider_resources=" + rand.nextBoolean(),
         "--incompatible_disallow_data_transition=" + rand.nextBoolean(),
         "--incompatible_disallow_dict_plus=" + rand.nextBoolean(),
@@ -151,6 +152,7 @@ public class SkylarkSemanticsConsistencyTest {
         .incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
         .incompatibleDepsetIsNotIterable(rand.nextBoolean())
         .incompatibleDepsetUnion(rand.nextBoolean())
+        .incompatibleDisableDeprecatedAttrParams(rand.nextBoolean())
         .incompatibleDisableObjcProviderResources(rand.nextBoolean())
         .incompatibleDisallowDataTransition(rand.nextBoolean())
         .incompatibleDisallowDictPlus(rand.nextBoolean())
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 14d4de5540..5378df232e 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -198,6 +198,32 @@ public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
         "attr.label_list(allow_files = 18)");
   }
 
+  @Test
+  public void testDisableDeprecatedParams() throws Exception {
+    ev = createEvaluationTestCase(
+        SkylarkSemantics.DEFAULT_SEMANTICS
+            .toBuilder()
+            .incompatibleDisableDeprecatedAttrParams(true)
+            .build());
+    ev.initialize();
+
+    // Verify 'single_file' deprecation.
+    EvalException expected =
+        assertThrows(EvalException.class, () -> eval("attr.label(single_file = True)"));
+    assertThat(expected).hasMessageThat().contains(
+        "'single_file' is no longer supported. use allow_single_file instead.");
+    Attribute attr = buildAttribute("a1", "attr.label(allow_single_file = ['.xml'])");
+    assertThat(attr.isSingleArtifact()).isTrue();
+
+    // Verify 'non_empty' deprecation.
+    expected =
+        assertThrows(EvalException.class, () -> eval("attr.string_list(non_empty=True)"));
+    assertThat(expected).hasMessageThat().contains(
+        "'non_empty' is no longer supported. use allow_empty instead.");
+    attr = buildAttribute("a2", "attr.string_list(allow_empty=False)");
+    assertThat(attr.isNonEmpty()).isTrue();
+  }
+
   @Test
   public void testAttrAllowedSingleFileTypesWrongType() throws Exception {
     checkErrorContains(
diff --git a/tools/build_defs/pkg/pkg.bzl b/tools/build_defs/pkg/pkg.bzl
index 459161785b..0e250b261e 100644
--- a/tools/build_defs/pkg/pkg.bzl
+++ b/tools/build_defs/pkg/pkg.bzl
@@ -231,23 +231,23 @@ def pkg_tar(**kwargs):
 pkg_deb = rule(
     implementation = _pkg_deb_impl,
     attrs = {
-        "data": attr.label(mandatory = True, allow_files = tar_filetype, single_file = True),
+        "data": attr.label(mandatory = True, allow_single_file = tar_filetype),
         "package": attr.string(mandatory = True),
         "architecture": attr.string(default = "all"),
         "distribution": attr.string(default = "unstable"),
         "urgency": attr.string(default = "medium"),
         "maintainer": attr.string(mandatory = True),
-        "preinst": attr.label(allow_files = True, single_file = True),
-        "postinst": attr.label(allow_files = True, single_file = True),
-        "prerm": attr.label(allow_files = True, single_file = True),
-        "postrm": attr.label(allow_files = True, single_file = True),
-        "conffiles_file": attr.label(allow_files = True, single_file = True),
+        "preinst": attr.label(allow_single_file = True),
+        "postinst": attr.label(allow_single_file = True),
+        "prerm": attr.label(allow_single_file = True),
+        "postrm": attr.label(allow_single_file = True),
+        "conffiles_file": attr.label(allow_single_file = True),
         "conffiles": attr.string_list(default = []),
-        "version_file": attr.label(allow_files = True, single_file = True),
+        "version_file": attr.label(allow_single_file = True),
         "version": attr.string(),
-        "description_file": attr.label(allow_files = True, single_file = True),
+        "description_file": attr.label(allow_single_file = True),
         "description": attr.string(),
-        "built_using_file": attr.label(allow_files = True, single_file = True),
+        "built_using_file": attr.label(allow_single_file = True),
         "built_using": attr.string(),
         "priority": attr.string(),
         "section": attr.string(),
-- 
cgit v1.2.3