Repository: bazelbuild/buildtools
Pull request #402 (2018-10-26 17:51:40)

 Add a warning for the FileType function


Patch:
diff --git a/WARNINGS.md b/WARNINGS.md
index 6877009..9d7473d 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -275,3 +275,11 @@ instead.
 The global variable `REPOSITORY_NAME` is deprecated, please use
 [`native.repository_name()`](https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name)
 instead.
+
+--------------------------------------------------------------------------------
+
+## <a name="filetype"></a>The `FileType` function is deprecated
+
+The function `FileType` is [deprecated](https://docs.bazel.build/versions/master/skylark/backward-compatibility.html#filetype-is-deprecated).
+Instead of using it as an argument to the [`rule` function](https://docs.bazel.build/versions/master/skylark/lib/globals.html#rule)
+just use a list of strings.
diff --git a/build/walk.go b/build/walk.go
index b661432..5e931f2 100644
--- a/build/walk.go
+++ b/build/walk.go
@@ -65,8 +65,8 @@ func walk1(v *Expr, stack *[]Expr, f func(x Expr, stk []Expr) Expr) Expr {
 func WalkOnce(v Expr, f func(x *Expr)) {
 	switch v := v.(type) {
 	case *File:
-		for _, stmt := range v.Stmt {
-			f(&stmt)
+		for i := range v.Stmt {
+			f(&v.Stmt[i])
 		}
 	case *DotExpr:
 		f(&v.X)
@@ -137,25 +137,25 @@ func WalkOnce(v Expr, f func(x *Expr)) {
 		f(&v.Test)
 		f(&v.Else)
 	case *DefStmt:
-		for _, p := range v.Params {
-			f(&p)
+		for i := range v.Params {
+			f(&v.Params[i])
 		}
-		for _, s := range v.Body {
-			f(&s)
+		for i := range v.Body {
+			f(&v.Body[i])
 		}
 	case *IfStmt:
 		f(&v.Cond)
-		for _, s := range v.True {
-			f(&s)
+		for i := range v.True {
+			f(&v.True[i])
 		}
-		for _, s := range v.False {
-			f(&s)
+		for i := range v.False {
+			f(&v.False[i])
 		}
 	case *ForStmt:
 		f(&v.Vars)
 		f(&v.X)
-		for _, s := range v.Body {
-			f(&s)
+		for i := range v.Body {
+			f(&v.Body[i])
 		}
 	case *ReturnStmt:
 		if v.Result != nil {
diff --git a/warn/warn.go b/warn/warn.go
index af15a39..3d9f22d 100644
--- a/warn/warn.go
+++ b/warn/warn.go
@@ -445,6 +445,7 @@ var FileWarningMap = map[string]func(f *build.File, fix bool) []*Finding{
 	"constant-glob":      constantGlobWarning,
 	"ctx-actions":        ctxActionsWarning,
 	"duplicated-name":    duplicatedNameWarning,
+	"filetype":           fileTypeWarning,
 	"integer-division":   integerDivisionWarning,
 	"load":               unusedLoadWarning,
 	"load-on-top":        loadOnTopWarning,
diff --git a/warn/warn_bazel.go b/warn/warn_bazel.go
index c1fbb7c..68a5dc1 100644
--- a/warn/warn_bazel.go
+++ b/warn/warn_bazel.go
@@ -260,6 +260,34 @@ func ctxActionsWarning(f *build.File, fix bool) []*Finding {
 	return findings
 }
 
+func fileTypeWarning(f *build.File, fix bool) []*Finding {
+	findings := []*Finding{}
+
+	var walk func(e *build.Expr, env *bzlenv.Environment)
+	walk = func(e *build.Expr, env *bzlenv.Environment) {
+		defer bzlenv.WalkOnceWithEnvironment(*e, env, walk)
+
+		call, ok := (*e).(*build.CallExpr)
+		if !ok {
+			return
+		}
+		ident, ok := (call.X).(*build.Ident)
+		if !ok || ident.Name != "FileType" {
+			return
+		}
+		if binding := env.Get("FileType"); binding == nil {
+			start, end := call.Span()
+			findings = append(findings,
+				makeFinding(f, start, end, "filetype",
+					"The FileType function is deprecated.", true, nil))
+		}
+	}
+	var expr build.Expr = f
+	walk(&expr, bzlenv.NewEnvironment())
+
+	return findings
+}
+
 func packageNameWarning(f *build.File, fix bool) []*Finding {
 	return globalVariableUsageCheck(f, "package-name", "PACKAGE_NAME", "native.package_name()", fix)
 }


Test patch:
diff --git a/warn/warn_bazel_test.go b/warn/warn_bazel_test.go
index 41b7529..9cc83f4 100644
--- a/warn/warn_bazel_test.go
+++ b/warn/warn_bazel_test.go
@@ -191,3 +191,31 @@ load(":foo.bzl", "REPOSITORY_NAME")
 foo(a = REPOSITORY_NAME)
 `, []string{}, false)
 }
+
+func TestFilTypeNameWarning(t *testing.T) {
+	checkFindings(t, "filetype", `
+rule1(types=FileType([".cc", ".h"]))
+rule2(types=FileType(types=[".cc", ".h"]))
+
+FileType(foobar)
+
+def macro1():
+    a = FileType([".py"])
+
+def macro2():
+    FileType = foo
+    b = FileType([".java"])
+`, []string{
+		":1: The FileType function is deprecated.",
+		":2: The FileType function is deprecated.",
+		":4: The FileType function is deprecated.",
+		":7: The FileType function is deprecated.",
+	}, false)
+
+	checkFindings(t, "filetype", `
+FileType = foo
+
+rule1(types=FileType([".cc", ".h"]))
+rule2(types=FileType(types=[".cc", ".h"]))
+`, []string{}, false)
+}