Repository: bazelbuild/buildtools
Pull request #634 (2019-05-16 14:29:23)

Add a check for *args and **kwargs in BUILD files


Patch:
diff --git a/WARNINGS.md b/WARNINGS.md
index c9c110d..5c22236 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -7,6 +7,7 @@ Warning categories supported by buildifier's linter:
   * [attr-non-empty](#attr-non-empty)
   * [attr-output-default](#attr-output-default)
   * [attr-single-file](#attr-single-file)
+  * [build-args-kwargs](#build-args-kwargs)
   * [confusing-name](#confusing-name)
   * [constant-glob](#constant-glob)
   * [ctx-actions](#ctx-actions)
@@ -103,6 +104,17 @@ is deprecated, please use `allow_single_file` instead.
 
 --------------------------------------------------------------------------------
 
+## <a name="build-args-kwargs"></a>`*args` and `**kwargs` are not allowed in BUILD files
+
+  * Category name: `build-args-kwargs`
+  * Flag in Bazel: `--incompatible_no_kwargs_in_build_files`
+  * Automatic fix: no
+
+Having `*args` or `**kwargs` makes BUILD files hard to read and manipulate. The list of
+arguments should be explicit.
+
+--------------------------------------------------------------------------------
+
 ## <a name="confusing-name"></a>Never use `l`, `I`, or `O` as names
 
   * Category name: `confusing-name`
diff --git a/warn/warn.go b/warn/warn.go
index 4653bc5..ac76558 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -72,6 +72,7 @@ var FileWarningMap = map[string]func(f *build.File, fix bool) []*Finding{
 	"attr-non-empty":            attrNonEmptyWarning,
 	"attr-output-default":       attrOutputDefaultWarning,
 	"attr-single-file":          attrSingleFileWarning,
+	"build-args-kwargs":         argsKwargsInBuildFilesWarning,
 	"confusing-name":            confusingNameWarning,
 	"constant-glob":             constantGlobWarning,
 	"ctx-actions":               ctxActionsWarning,
diff --git a/warn/warn_bazel.go b/warn/warn_bazel.go
index baafce6..d3f8941 100644
--- a/warn/warn_bazel.go
+++ b/warn/warn_bazel.go
@@ -163,3 +163,37 @@ func positionalArgumentsWarning(f *build.File, pkg string, stmt build.Expr) *Fin
 	}
 	return nil
 }
+
+func argsKwargsInBuildFilesWarning(f *build.File, fix bool) []*Finding {
+	findings := []*Finding{}
+
+	if f.Type != build.TypeBuild {
+		return findings
+	}
+
+	build.Walk(f, func(expr build.Expr, stack []build.Expr) {
+		// Search for function call nodes
+		call, ok := expr.(*build.CallExpr)
+		if !ok {
+			return
+		}
+		for _, param := range call.List {
+			unary, ok := param.(*build.UnaryExpr)
+			if !ok {
+				continue
+			}
+			start, end := param.Span()
+			switch unary.Op {
+			case "*":
+				findings = append(findings,
+					makeFinding(f, start, end, "build-args-kwargs",
+						`*args are not allowed in BUILD files.`, true, nil))
+			case "**":
+				findings = append(findings,
+					makeFinding(f, start, end, "build-args-kwargs",
+						`**kwargs are not allowed in BUILD files.`, true, nil))
+			}
+		}
+	})
+	return findings
+}


Test patch:
diff --git a/warn/warn_bazel_test.go b/warn/warn_bazel_test.go
index e94969e..2d116e2 100644
--- a/warn/warn_bazel_test.go
+++ b/warn/warn_bazel_test.go
@@ -83,3 +83,31 @@ register_toolchains(
 		[]string{},
 		scopeBuild|scopeWorkspace)
 }
+
+func TestKwargsInBuildFilesWarning(t *testing.T) {
+	checkFindings(t, "build-args-kwargs", `
+cc_library(
+  name = "foo",
+  *args,
+  **kwargs,
+)
+
+foo(*bar(**kgs))`,
+		[]string{
+			":3: *args are not allowed in BUILD files.",
+			":4: **kwargs are not allowed in BUILD files.",
+			":7: *args are not allowed in BUILD files.",
+			":7: **kwargs are not allowed in BUILD files.",
+		},
+		scopeBuild)
+
+	checkFindings(t, "build-args-kwargs", `
+cc_library(
+  name = "foo",
+  -args,
+)
+
+foo(not bar(-kgs))`,
+		[]string{},
+		scopeBuild)
+}