Repository: bazelbuild/buildtools
Pull request #810 (2020-03-30 09:34:38)

Add buildifier warning on deprecated use of the 'items' parameter in depset()
This adds a warning for depset()-usage that will be forbidden after --incompatible_disable_depset_items is flipped, see https://github.com/bazelbuild/bazel/issues/9017


Patch:
diff --git a/WARNINGS.md b/WARNINGS.md
index c428fff..fdea648 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -14,6 +14,7 @@ Warning categories supported by buildifier's linter:
   * [`ctx-actions`](#ctx-actions)
   * [`ctx-args`](#ctx-args)
   * [`deprecated-function`](#deprecated-function)
+  * [`depset-items`](#depset-items)
   * [`depset-iteration`](#depset-iteration)
   * [`depset-union`](#depset-union)
   * [`dict-concatenation`](#dict-concatenation)
@@ -253,6 +254,20 @@ the [`function-docstring`](function-docstring) warning.
 
 --------------------------------------------------------------------------------
 
+## <a name="depset-items"></a>Depset "items" parameter is deprecated
+
+  * Category name: `depset-items`
+  * Flag in Bazel: [`--incompatible_disable_depset_items`](https://github.com/bazelbuild/bazel/issues/9017)
+  * Automatic fix: no
+
+The `items` parameter for [`depset`](https://docs.bazel.build/versions/master/skylark/lib/globals.html#depset)
+is deprecated. In it's old form it's either a list of direct elements to be
+added (use the `direct` or unnamed first parameter instead) or a depset that
+becomes a tarnsitive element of the new depset (use the `transitive` parameter
+instead).
+
+--------------------------------------------------------------------------------
+
 ## <a name="depset-iteration"></a>Depset iteration is deprecated
 
   * Category name: `depset-iteration`
diff --git a/warn/warn.go b/warn/warn.go
index 4af5e0c..bd5f6fd 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -111,6 +111,7 @@ var FileWarningMap = map[string]func(f *build.File) []*LinterFinding{
 	"constant-glob":             constantGlobWarning,
 	"ctx-actions":               ctxActionsWarning,
 	"ctx-args":                  contextArgsAPIWarning,
+	"depset-items":              depsetItemsWarning,
 	"depset-iteration":          depsetIterationWarning,
 	"depset-union":              depsetUnionWarning,
 	"dict-concatenation":        dictionaryConcatenationWarning,
diff --git a/warn/warn_bazel_api.go b/warn/warn_bazel_api.go
index 2126543..801e734 100644
--- a/warn/warn_bazel_api.go
+++ b/warn/warn_bazel_api.go
@@ -345,6 +345,40 @@ func attrConfigurationWarning(f *build.File) []*LinterFinding {
 	return findings
 }
 
+func depsetItemsWarning(f *build.File) []*LinterFinding {
+	var findings []*LinterFinding
+
+	types := detectTypes(f)
+	build.WalkPointers(f, func(expr *build.Expr, stack []build.Expr) {
+		call, ok := (*expr).(*build.CallExpr)
+		if !ok {
+			return
+		}
+		base, ok := call.X.(*build.Ident)
+		if !ok || base.Name != "depset" {
+			return
+		}
+		if len(call.List) == 0 {
+			return
+		}
+		_, _, param := getParam(call.List, "items")
+		if param != nil {
+			findings = append(findings,
+				makeLinterFinding(param, `Parameter "items" is deprecated, use "direct" and/or "transitive" instead.`))
+			return
+		}
+		if _, ok := call.List[0].(*build.AssignExpr); ok {
+			return
+		}
+		// We have an unnamed first parameter. Check the type.
+		if types[call.List[0]] == Depset {
+			findings = append(findings,
+				makeLinterFinding(call.List[0], `Giving a depset as first unnamed parameter to depset() is deprecated, use the "transitive" parameter instead.`))
+		}
+	})
+	return findings
+}
+
 func attrNonEmptyWarning(f *build.File) []*LinterFinding {
 	if f.Type != build.TypeBzl {
 		return nil


Test patch:
diff --git a/warn/warn_bazel_api_test.go b/warn/warn_bazel_api_test.go
index 76c4cff..c4bad0c 100644
--- a/warn/warn_bazel_api_test.go
+++ b/warn/warn_bazel_api_test.go
@@ -26,6 +26,19 @@ attr.label_list(mandatory = True, cfg = "host")`,
 		scopeBzl)
 }
 
+func TestDepsetItemsWarning(t *testing.T) {
+	checkFindings(t, "depset-items", `
+def f():
+  depset(items=foo)
+  a = depset()
+  depset(a)
+`, []string{
+		`:2: Parameter "items" is deprecated, use "direct" and/or "transitive" instead.`,
+		`:4: Giving a depset as first unnamed parameter to depset() is deprecated, use the "transitive" parameter instead.`,
+	},
+		scopeEverywhere)
+}
+
 func TestAttrNonEmptyWarning(t *testing.T) {
 	checkFindingsAndFix(t, "attr-non-empty", `
 rule(