diff --git a/warn/warn_bazel.go b/warn/warn_bazel.go index 004739373..cd67c3572 100644 --- a/warn/warn_bazel.go +++ b/warn/warn_bazel.go @@ -33,6 +33,23 @@ var functionsWithPositionalArguments = map[string]bool{ "vardef": true, } +func constantGlobPatternWarning(patterns *build.ListExpr) []*LinterFinding { + findings := []*LinterFinding{} + for _, expr := range patterns.List { + str, ok := expr.(*build.StringExpr) + if !ok { + continue + } + if !strings.Contains(str.Value, "*") { + message := fmt.Sprintf( + `Glob pattern %q has no wildcard ('*'). Constant patterns can be error-prone, move the file outside the glob.`, str.Value) + findings = append(findings, makeLinterFinding(expr, message)) + return findings // at most one warning per glob + } + } + return findings +} + func constantGlobWarning(f *build.File) []*LinterFinding { switch f.Type { case build.TypeBuild, build.TypeWorkspace, build.TypeBzl: @@ -52,18 +69,25 @@ func constantGlobWarning(f *build.File) []*LinterFinding { return } patterns, ok := call.List[0].(*build.ListExpr) - if !ok { - return + if ok { + // first arg is unnamed and is a list + findings = append(findings, constantGlobPatternWarning(patterns)...) + return // at most one warning per glob } - for _, expr := range patterns.List { - str, ok := expr.(*build.StringExpr) + + // look for named args called include + for _, arg := range call.List { + assign_expr, ok := arg.(*build.AssignExpr) if !ok { continue } - if !strings.Contains(str.Value, "*") { - message := fmt.Sprintf( - `Glob pattern %q has no wildcard ('*'). Constant patterns can be error-prone, move the file outside the glob.`, str.Value) - findings = append(findings, makeLinterFinding(expr, message)) + str, ok := assign_expr.LHS.(*build.Ident) + if !ok || str.Name != "include" { + continue + } + patterns, ok := assign_expr.RHS.(*build.ListExpr) + if ok { + findings = append(findings, constantGlobPatternWarning(patterns)...) return // at most one warning per glob } } diff --git a/warn/warn_bazel_test.go b/warn/warn_bazel_test.go index 2ee85f0f1..0d0a13313 100644 --- a/warn/warn_bazel_test.go +++ b/warn/warn_bazel_test.go @@ -21,15 +21,24 @@ import "testing" func TestConstantGlob(t *testing.T) { checkFindings(t, "constant-glob", ` cc_library(srcs = glob(["foo.cc"])) -cc_library(srcs = glob(["*.cc"])) +cc_library(srcs = glob(include = ["foo.cc"])) +cc_library(srcs = glob(include = ["foo.cc"], exclude = ["bar.cc"])) +cc_library(srcs = glob(exclude = ["bar.cc"], include = ["foo.cc"])) cc_library(srcs = - ["constant"] + glob([ - "*.cc", - "test.cpp", - ]) -)`, + ["constant"] + glob([ + "*.cc", + "test.cpp", + ]) + ) +cc_library(srcs = glob(["*.cc"])) +cc_library(srcs = glob(["*.cc"], exclude = ["bar.cc"])) +cc_library(srcs = glob(include = ["*.cc"], exclude = ["bar.cc"])) +cc_library(srcs = glob(exclude = ["bar.cc"], include = ["*.cc"]))`, []string{`:1: Glob pattern "foo.cc" has no wildcard`, - `:6: Glob pattern "test.cpp" has no wildcard`}, + `:2: Glob pattern "foo.cc" has no wildcard`, + `:3: Glob pattern "foo.cc" has no wildcard`, + `:4: Glob pattern "foo.cc" has no wildcard`, + `:8: Glob pattern "test.cpp" has no wildcard`}, scopeBuild|scopeBzl|scopeWorkspace) }