Revising implementation for warn_macro
- Temp changes, not sure if this will be kept or not
diff --git a/warn/warn_macro.go b/warn/warn_macro.go
index 65b071c..e9898ee 100644
--- a/warn/warn_macro.go
+++ b/warn/warn_macro.go
@@ -26,9 +26,6 @@
"github.com/bazelbuild/buildtools/labels"
)
-// Internal constant that represents the native module
-const nativeModule = "<native>"
-
// function represents a function identifier, which is a pair (module name, function name).
type function struct {
pkg string // package where the function is defined
@@ -40,12 +37,9 @@
return f.pkg + ":" + f.filename
}
-// funCall represents a call to another function. It contains information of the function itself as well as some
-// information about the environment
-type funCall struct {
- function
- nameAlias string // function name alias (it could be loaded with a different name or assigned to a new variable).
- line int // line on which the function is being called
+// callStackFrame formats a function call to be printable in a call stack.
+func (f function) callStackFrame(ref build.Expr) string {
+ return fmt.Sprintf("%s:%d: %s", f.label(), exprLine(ref), f.name)
}
// acceptsNameArgument checks whether a function can accept a named argument called "name",
@@ -53,236 +47,185 @@
func acceptsNameArgument(def *build.DefStmt) bool {
for _, param := range def.Params {
if name, op := build.GetParamName(param); name == "name" || op == "**" {
- return true
+ return true
}
}
return false
}
-// fileData represents information about rules and functions extracted from a file
-type fileData struct {
- rules map[string]bool // all rules defined in the file
- functions map[string]map[string]funCall // outer map: all functions defined in the file, inner map: all distinct function calls from the given function
- aliases map[string]function // all top-level aliases (e.g. `foo = bar`).
-}
-
-// resolvesExternal takes a local function definition and replaces it with an external one if it's been defined
-// in another file and loaded
-func resolveExternal(fn function, externalSymbols map[string]function) function {
- if external, ok := externalSymbols[fn.name]; ok {
- return external
- }
- return fn
-}
-
// exprLine returns the start line of an expression
func exprLine(expr build.Expr) int {
start, _ := expr.Span()
return start.Line
}
-// getFunCalls extracts information about functions that are being called from the given function
-func getFunCalls(def *build.DefStmt, pkg, filename string, externalSymbols map[string]function) map[string]funCall {
- funCalls := make(map[string]funCall)
- build.Walk(def, func(expr build.Expr, stack []build.Expr) {
- call, ok := expr.(*build.CallExpr)
- if !ok {
- return
- }
- if ident, ok := call.X.(*build.Ident); ok {
- funCalls[ident.Name] = funCall{
- function: resolveExternal(function{pkg, filename, ident.Name}, externalSymbols),
- nameAlias: ident.Name,
- line: exprLine(call),
- }
- return
- }
- dot, ok := call.X.(*build.DotExpr)
- if !ok {
- return
- }
- if ident, ok := dot.X.(*build.Ident); !ok || ident.Name != "native" {
- return
- }
- name := "native." + dot.Name
- funCalls[name] = funCall{
- function: function{
- name: dot.Name,
- filename: nativeModule,
- },
- nameAlias: name,
- line: exprLine(dot),
- }
- })
- return funCalls
-}
-
-// analyzeFile extracts the information about rules and functions defined in the file
-func analyzeFile(f *build.File) fileData {
- if f == nil {
- return fileData{}
- }
-
- // Collect loaded symbols
- externalSymbols := make(map[string]function)
- for _, stmt := range f.Stmt {
- load, ok := stmt.(*build.LoadStmt)
- if !ok {
- continue
- }
- label := labels.ParseRelative(load.Module.Value, f.Pkg)
- if label.Repository != "" || label.Target == "" {
- continue
- }
- for i, from := range load.From {
- externalSymbols[load.To[i].Name] = function{label.Package, label.Target, from.Name}
- }
- }
-
- report := fileData{
- rules: make(map[string]bool),
- functions: make(map[string]map[string]funCall),
- aliases: make(map[string]function),
- }
- for _, stmt := range f.Stmt {
- switch stmt := stmt.(type) {
- case *build.AssignExpr:
- // Analyze aliases (`foo = bar`) or rule declarations (`foo = rule(...)`)
- lhsIdent, ok := stmt.LHS.(*build.Ident)
- if !ok {
- continue
- }
- if rhsIdent, ok := stmt.RHS.(*build.Ident); ok {
- report.aliases[lhsIdent.Name] = resolveExternal(function{f.Pkg, f.Label, rhsIdent.Name}, externalSymbols)
- continue
- }
-
- call, ok := stmt.RHS.(*build.CallExpr)
- if !ok {
- continue
- }
- ident, ok := call.X.(*build.Ident)
- if !ok || ident.Name != "rule" {
- continue
- }
- report.rules[lhsIdent.Name] = true
- case *build.DefStmt:
- report.functions[stmt.Name] = getFunCalls(stmt, f.Pkg, f.Label, externalSymbols)
- default:
- continue
- }
- }
- return report
-}
-
// functionReport represents the analysis result of a function
-type functionReport struct {
- isMacro bool // whether the function is a macro (or a rule)
- fc *funCall // a call to the rule or another macro
+type macroReport struct {
+ callStack []string
+}
+
+// isMacroOrRule returns true if the report contained a rule or a macro.
+func (mr macroReport) isMacroOrRule() bool {
+ return mr.callStack != nil
+}
+
+// wrapReport is a helper to wrap a marcoReport for a downstream call.
+func wrapReport(fn function, ref build.Expr, mr macroReport) macroReport {
+ return macroReport{callStack: append([]string{fn.callStackFrame(ref)}, mr.callStack...)}
}
// macroAnalyzer is an object that analyzes the directed graph of functions calling each other,
// loading other files lazily if necessary.
type macroAnalyzer struct {
fileReader *FileReader
- files map[string]fileData
- cache map[function]functionReport
+ // Local files if file is defined here, fileReader is not used.
+ localFiles map[string]*build.File
+ cache map[function]*macroReport
}
-// getFileData retrieves a file using the fileReader object and extracts information about functions and rules
-// defined in the file.
-func (ma macroAnalyzer) getFileData(pkg, label string) fileData {
- filename := pkg + ":" + label
- if fd, ok := ma.files[filename]; ok {
- return fd
+func (ma macroAnalyzer) getFile(fn function) *build.File {
+ f := ma.localFiles[fn.label()]
+ if f != nil {
+ return f
}
if ma.fileReader == nil {
- fd := fileData{}
- ma.files[filename] = fd
- return fd
+ return nil
}
- f := ma.fileReader.GetFile(pkg, label)
- fd := analyzeFile(f)
- ma.files[filename] = fd
- return fd
+ return ma.fileReader.GetFile(fn.pkg, fn.filename)
}
-// IsMacro is a public function that checks whether the given function is a macro
-func (ma macroAnalyzer) IsMacro(fn function) (report functionReport) {
+// AnalyzeFn is a public function that checks whether the given function is a macro or rule.
+func (ma macroAnalyzer) AnalyzeFn(fn function) (report macroReport) {
// Check the cache first
if cached, ok := ma.cache[fn]; ok {
- return cached
+ return *cached
}
- // Write a negative result to the cache before analyzing. This will prevent stack overflow crashes
+ // Write an empty result to the cache before analyzing. This will prevent stack overflow crashes
// if the input data contains recursion.
- ma.cache[fn] = report
+ ma.cache[fn] = ¯oReport{}
defer func() {
// Update the cache with the actual result
- ma.cache[fn] = report
+ ma.cache[fn] = &report
}()
- // Check for native rules
- if fn.filename == nativeModule {
- switch fn.name {
- case "glob", "existing_rule", "existing_rules", "package_name",
- "repository_name", "exports_files":
- // Not a rule
+ f := ma.getFile(fn)
+ if f == nil {
+ return macroReport{}
+ }
+
+ for _, stmt := range f.Stmt {
+ switch stmt := stmt.(type) {
+ // If function is loaded from another file, check the separate file for the function call.
+ case *build.LoadStmt:
+ for i, from := range stmt.From {
+ if stmt.To[i].Name == fn.name {
+ label := labels.ParseRelative(stmt.Module.Value, f.Pkg)
+ if r := ma.AnalyzeFn(function{label.Package, label.Target, from.Name}); r.isMacroOrRule() {
+ return r
+ }
+ }
+ }
+ case *build.AssignExpr:
+ // Analyze aliases (`foo = bar`) or rule declarations (`foo = rule(...)`)
+ if lhsIdent, ok := stmt.LHS.(*build.Ident); !ok || lhsIdent.Name != fn.name {
+ continue
+ }
+
+ // If the RHS is an identifier (LHS is an alias), check if RHS is a macro.
+ if rhsIdent, ok := stmt.RHS.(*build.Ident); ok {
+ if r := ma.AnalyzeFn(function{f.Pkg, f.Label, rhsIdent.Name}); r.isMacroOrRule() {
+ return wrapReport(fn, stmt, r)
+ }
+ continue
+ }
+
+ // If the RHS is a function call, check if the called function is a "rule" or a macro.
+ if call, ok := stmt.RHS.(*build.CallExpr); ok {
+ if ident, ok := call.X.(*build.Ident); ok {
+ if ident.Name == "macro" {
+ report = macroReport{callStack: []string{
+ function{f.Pkg, f.Label, "(MACRO)"}.callStackFrame(stmt),
+ }}
+ return
+ }
+ if ident.Name == "rule" {
+ report = macroReport{callStack: []string{
+ function{f.Pkg, f.Label, "(RULE)"}.callStackFrame(stmt),
+ }}
+ return
+ }
+ if r := ma.AnalyzeFn(function{f.Pkg, f.Label, ident.Name}); r.isMacroOrRule() {
+ return wrapReport(fn, stmt, r)
+ }
+ }
+ }
+
+ if dotExpr, ok := stmt.RHS.(*build.DotExpr); ok {
+ // Note: Currently only handles "native." dot-expressions, others are ignored.
+ if isNativeRule(dotExpr) {
+ return macroReport{callStack: []string{function{f.Pkg, f.Label, "native." + dotExpr.Name + " (NATIVE RULE)"}.callStackFrame(dotExpr)}}
+ }
+ }
+ case *build.DefStmt:
+ if stmt.Name != fn.name {
+ continue
+ }
+ // If the function is implemented here, check if it calls any rules or macros.
+ if r := ma.checkFunctionCalls(fn, stmt); r.isMacroOrRule() {
+ return wrapReport(fn, stmt, r)
+ }
default:
- report.isMacro = true
- }
- return
- }
-
- fileData := ma.getFileData(fn.pkg, fn.filename)
-
- // Check whether fn.name is an alias for another function
- if alias, ok := fileData.aliases[fn.name]; ok {
- if ma.IsMacro(alias).isMacro {
- report.isMacro = true
- }
- return
- }
-
- // Check whether fn.name is a rule
- if fileData.rules[fn.name] {
- report.isMacro = true
- return
- }
-
- // Check whether fn.name is an ordinary function
- funCalls, ok := fileData.functions[fn.name]
- if !ok {
- return
- }
-
- // Prioritize function calls from already loaded files. If some of the function calls are from the same file
- // (or another file that has been loaded already), check them first.
- var knownFunCalls, newFunCalls []funCall
- for _, fc := range funCalls {
- if _, ok := ma.files[fc.function.pkg+":"+fc.function.filename]; ok || fc.function.filename == nativeModule {
- knownFunCalls = append(knownFunCalls, fc)
- } else {
- newFunCalls = append(newFunCalls, fc)
+ continue
}
}
-
- for _, fc := range append(knownFunCalls, newFunCalls...) {
- if ma.IsMacro(fc.function).isMacro {
- report.isMacro = true
- report.fc = &fc
- return
- }
- }
-
return
}
+func (ma macroAnalyzer) checkFunctionCalls(fn function, def *build.DefStmt) (report macroReport) {
+ report = macroReport{}
+ build.Walk(def, func(expr build.Expr, stack []build.Expr) {
+ if call, ok := expr.(*build.CallExpr); ok {
+ if fnIdent, ok := call.X.(*build.Ident); ok {
+ calledFn := function{pkg: fn.pkg, filename: fn.filename, name: fnIdent.Name}
+ if r := ma.AnalyzeFn(calledFn); r.isMacroOrRule() {
+ report = wrapReport(calledFn, call, r)
+ return
+ }
+ }
+ }
+ if dotExpr, ok := expr.(*build.DotExpr); ok {
+ // Note: Currently only handles "native." dot-expressions, others are ignored.
+ if isNativeRule(dotExpr) {
+ dotFn := function{pkg: fn.pkg, filename: fn.filename, name: "native." + dotExpr.Name + " (NATIVE RULE)"}
+ report = macroReport{callStack: []string{dotFn.callStackFrame(dotExpr)}}
+ }
+ }
+ })
+ return report
+}
+
+func isNativeRule(expr *build.DotExpr) bool {
+ if ident, ok := expr.X.(*build.Ident); !ok || ident.Name != "native" {
+ return false
+ }
+
+ switch expr.Name {
+ case "glob", "existing_rule", "existing_rules", "package_name",
+ "repository_name", "exports_files":
+
+ // Not a rule
+ return false
+ default:
+ return true
+ }
+}
+
// newMacroAnalyzer creates and initiates an instance of macroAnalyzer.
func newMacroAnalyzer(fileReader *FileReader) macroAnalyzer {
return macroAnalyzer{
fileReader: fileReader,
- files: make(map[string]fileData),
- cache: make(map[function]functionReport),
+ localFiles: make(map[string]*build.File),
+ cache: make(map[function]*macroReport),
}
}
@@ -292,9 +235,9 @@
}
macroAnalyzer := newMacroAnalyzer(fileReader)
- macroAnalyzer.files[f.Pkg+":"+f.Label] = analyzeFile(f)
+ macroAnalyzer.localFiles[f.Pkg+":"+f.Label] = f
- findings := []*LinterFinding{}
+ var findings []*LinterFinding
for _, stmt := range f.Stmt {
def, ok := stmt.(*build.DefStmt)
if !ok {
@@ -305,24 +248,26 @@
continue
}
- report := macroAnalyzer.IsMacro(function{f.Pkg, f.Label, def.Name})
- if !report.isMacro {
+ fn := function{f.Pkg, f.Label, def.Name}
+ report := macroAnalyzer.AnalyzeFn(fn)
+ if !report.isMacroOrRule() {
continue
}
- msg := fmt.Sprintf(`The macro %q should have a keyword argument called "name".`, def.Name)
- if report.fc != nil {
- // fc shouldn't be nil because that's the only node that can be found inside a function.
- msg += fmt.Sprintf(`
+ msg := fmt.Sprintf(`The macro %q should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro %q on line %d.
+It is considered a macro because it calls a rule or another macro, call stack:
+
+%s
By convention, every public macro needs a "name" argument (even if it doesn't use it).
This is important for tooling and automation.
* If this function is a helper function that's not supposed to be used outside of this file,
please make it private (e.g. rename it to "_%s").
- * Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`, report.fc.nameAlias, report.fc.line, def.Name)
- }
+ * Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
+ def.Name,
+ strings.Join(report.callStack, "\n"),
+ def.Name)
finding := makeLinterFinding(def, msg)
finding.End = def.ColonPos
findings = append(findings, finding)
diff --git a/warn/warn_macro_test.go b/warn/warn_macro_test.go
index d7dcf0a..19ebe5f 100644
--- a/warn/warn_macro_test.go
+++ b/warn/warn_macro_test.go
@@ -45,76 +45,154 @@
[]string{
`5: The macro "macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "my_rule" on line 7.`,
- `19: The macro "another_macro" should have a keyword argument called "name".
+It is considered a macro because it calls a rule or another macro, call stack:
-It is considered a macro because it calls a rule or another macro "native.cc_library" on line 21.`,
+test/package:test_file.bzl:5: macro
+test/package:test_file.bzl:7: my_rule
+test/package:test_file.bzl:3: (RULE)
+
+By convention, every public macro needs a "name" argument (even if it doesn't use it).
+This is important for tooling and automation.
+
+ * If this function is a helper function that's not supposed to be used outside of this file,
+ please make it private (e.g. rename it to "_macro").
+ * Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
+ `:19: The macro "another_macro" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:19: another_macro
+test/package:test_file.bzl:21: native.cc_library (NATIVE RULE)
+
+By convention, every public macro needs a "name" argument (even if it doesn't use it).
+This is important for tooling and automation.
+
+ * If this function is a helper function that's not supposed to be used outside of this file,
+ please make it private (e.g. rename it to "_another_macro").
+ * Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
},
scopeBzl)
checkFindings(t, "unnamed-macro", `
-my_rule = rule()
+ my_rule = rule()
-def macro1(foo, name, bar):
- my_rule()
+ def macro1(foo, name, bar):
+ my_rule()
-def macro2(foo, *, name):
- my_rule()
+ def macro2(foo, *, name):
+ my_rule()
-def macro3(foo, *args, **kwargs):
- my_rule()
-`,
+ def macro3(foo, *args, **kwargs):
+ my_rule()
+ `,
[]string{},
scopeBzl)
checkFindings(t, "unnamed-macro", `
-my_rule = rule()
+ my_rule = rule()
-def macro(name):
- my_rule(name = name)
+ def macro(name):
+ my_rule(name = name)
-alias = macro
+ alias = macro
-def bad_macro():
- for x in y:
- alias(x)
-`,
+ def bad_macro():
+ for x in y:
+ alias(x)
+ `,
[]string{
- `8: The macro "bad_macro" should have a keyword argument called "name".
+ `:8: The macro "bad_macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "alias" on line 10.`,
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:8: bad_macro
+test/package:test_file.bzl:10: alias
+test/package:test_file.bzl:6: alias
+test/package:test_file.bzl:3: macro
+test/package:test_file.bzl:4: my_rule
+test/package:test_file.bzl:1: (RULE)`,
},
scopeBzl)
checkFindings(t, "unnamed-macro", `
+ my_rule = rule()
+
+ def macro1():
+ my_rule(name = x)
+
+ def macro2():
+ macro1()
+
+ def macro3():
+ macro2()
+
+ def macro4():
+ my_rule()
+ `,
+ []string{
+ `:3: The macro "macro1" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:3: macro1
+test/package:test_file.bzl:4: my_rule
+test/package:test_file.bzl:1: (RULE)`,
+ `:6: The macro "macro2" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:6: macro2
+test/package:test_file.bzl:7: macro1
+test/package:test_file.bzl:3: macro1
+test/package:test_file.bzl:4: my_rule
+test/package:test_file.bzl:1: (RULE)
+`,
+ `:9: The macro "macro3" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:9: macro3
+test/package:test_file.bzl:10: macro2
+test/package:test_file.bzl:6: macro2
+test/package:test_file.bzl:7: macro1
+test/package:test_file.bzl:3: macro1
+test/package:test_file.bzl:4: my_rule
+test/package:test_file.bzl:1: (RULE)
+`,
+ `:12: The macro "macro4" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:12: macro4
+test/package:test_file.bzl:13: my_rule
+test/package:test_file.bzl:1: (RULE)`,
+ },
+ scopeBzl)
+}
+
+func TestSymbolicMacro(t *testing.T) {
+ checkFindings(t, "unnamed-macro", `
+load(":foo.bzl", "foo")
+
my_rule = rule()
-def macro1():
- my_rule(name = x)
-
-def macro2():
- macro1()
-
-def macro3():
- macro2()
-
-def macro4():
+def _my_macro_implementation(name):
my_rule()
+
+my_symbolic_macro = macro(implementation = _my_macro_impl)
+
+def legacy_macro():
+ my_symbolic_macro()
`,
[]string{
- `3: The macro "macro1" should have a keyword argument called "name".
+ `:10: The macro "legacy_macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "my_rule" on line 4.`,
- `6: The macro "macro2" should have a keyword argument called "name".
+It is considered a macro because it calls a rule or another macro, call stack:
-It is considered a macro because it calls a rule or another macro "macro1" on line 7`,
- `9: The macro "macro3" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "macro2" on line 10.`,
- `12: The macro "macro4" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "my_rule" on line 13.`,
- },
+test/package:test_file.bzl:10: legacy_macro
+test/package:test_file.bzl:11: my_symbolic_macro
+test/package:test_file.bzl:8: (MACRO)
+`},
scopeBzl)
}
@@ -159,12 +237,22 @@
my_rule()
`,
[]string{
- `3: The macro "foo" should have a keyword argument called "name".
+ `:3: The macro "foo" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "bar" on line 4.`,
- `6: The macro "bar" should have a keyword argument called "name".
+It is considered a macro because it calls a rule or another macro, call stack:
-It is considered a macro because it calls a rule or another macro "my_rule" on line 8.`,
+test/package:test_file.bzl:3: foo
+test/package:test_file.bzl:4: bar
+test/package:test_file.bzl:6: bar
+test/package:test_file.bzl:8: my_rule
+test/package:test_file.bzl:1: (RULE)`,
+ `:6: The macro "bar" should have a keyword argument called "name".
+
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:6: bar
+test/package:test_file.bzl:8: my_rule
+test/package:test_file.bzl:1: (RULE)`,
},
scopeBzl)
}
@@ -213,9 +301,16 @@
f()
`,
[]string{
- `4: The macro "macro1" should have a keyword argument called "name".
+ `:4: The macro "macro1" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "abc" on line 5.
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:4: macro1
+test/package:test_file.bzl:5: abc
+test/package:subdir1/foo.bzl:5: bar
+test/package:subdir1/foo.bzl:6: foo
+test/package:subdir1/foo.bzl:2: foo
+test/package:subdir1/foo.bzl:3: native.foo_binary (NATIVE RULE)
By convention, every public macro needs a "name" argument (even if it doesn't use it).
This is important for tooling and automation.
@@ -223,9 +318,18 @@
* If this function is a helper function that's not supposed to be used outside of this file,
please make it private (e.g. rename it to "_macro1").
* Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
- `7: The macro "macro2" should have a keyword argument called "name".
+ `:7: The macro "macro2" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "baz" on line 8.
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:7: macro2
+test/package:test_file.bzl:8: baz
+test/package:subdir2/baz.bzl:5: baz
+test/package:subdir2/baz.bzl:7: bar
+test/package:subdir1/foo.bzl:5: bar
+test/package:subdir1/foo.bzl:6: foo
+test/package:subdir1/foo.bzl:2: foo
+test/package:subdir1/foo.bzl:3: native.foo_binary (NATIVE RULE)
By convention, every public macro needs a "name" argument (even if it doesn't use it).
This is important for tooling and automation.
@@ -233,9 +337,15 @@
* If this function is a helper function that's not supposed to be used outside of this file,
please make it private (e.g. rename it to "_macro2").
* Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
- `10: The macro "macro3" should have a keyword argument called "name".
+ `:10: The macro "macro3" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "qux" on line 11.
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:10: macro3
+test/package:test_file.bzl:11: qux
+test/package:subdir2/baz.bzl:9: qux
+test/package:subdir2/baz.bzl:10: your_rule
+test/package:subdir1/foo.bzl:8: (RULE)
By convention, every public macro needs a "name" argument (even if it doesn't use it).
This is important for tooling and automation.
@@ -286,9 +396,16 @@
baz()
quux()
`, []string{
- `4: The macro "macro" should have a keyword argument called "name".
+ `:4: The macro "macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "quux" on line 7.`,
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:4: macro
+test/package:test_file.bzl:7: quux
+test/package:bar.bzl:7: qux
+test/package:bar.bzl:10: quuux
+test/package:foo.bzl:10: qux
+test/package:foo.bzl:11: native.cc_library (NATIVE RULE)`,
}, scopeBzl)
}
@@ -310,65 +427,18 @@
def macro():
bar()
`, []string{
- `5: The macro "macro" should have a keyword argument called "name".
+ `:5: The macro "macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "bar" on line 6.`,
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:5: macro
+test/package:test_file.bzl:6: bar
+test/package:foo.bzl:4: foo
+test/package:foo.bzl:5: some_rule
+test/package:test_file.bzl:3: (RULE)`,
}, scopeBzl)
}
-func TestUnnamedMacroLoadedFiles(t *testing.T) {
- // Test that not necessary files are not loaded
-
- defer setUpFileReader(map[string]string{
- "a.bzl": "a = rule()",
- "b.bzl": "b = rule()",
- "c.bzl": "c = rule()",
- "d.bzl": "d = rule()",
- })()
-
- checkFindings(t, "unnamed-macro", `
-load("//:a.bzl", "a")
-load("//:b.bzl", "b")
-load("//:c.bzl", "c")
-load("//:d.bzl", "d")
-
-def macro1():
- a() # has to load a.bzl to analyze
-
-def macro2():
- b() # can skip b.bzl because there's a native rule
- native.cc_library()
-
-def macro3():
- c() # can skip c.bzl because a.bzl has already been loaded
- a()
-
-def macro4():
- d() # can skip d.bzl because there's a rule or another macro defined in the same file
- r()
-
-r = rule()
-`, []string{
- `6: The macro "macro1" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "a" on line 7.`,
- `9: The macro "macro2" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "native.cc_library" on line 11.`,
- `13: The macro "macro3" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "a" on line 15.`,
- `17: The macro "macro4" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "r" on line 19.`,
- }, scopeBzl)
-
- if len(fileReaderRequests) == 1 && fileReaderRequests[0] == "a.bzl" {
- return
- }
- t.Errorf("expected to load only a.bzl, instead loaded %d files: %v", len(fileReaderRequests), fileReaderRequests)
-}
-
func TestUnnamedMacroAliases(t *testing.T) {
defer setUpFileReader(map[string]string{
"test/package/foo.bzl": `
@@ -392,9 +462,15 @@
def macro2(name):
baz()
`, []string{
- `5: The macro "macro1" should have a keyword argument called "name".
+ `:5: The macro "macro1" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "baz" on line 6.`,
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:5: macro1
+test/package:test_file.bzl:6: baz
+test/package:test_file.bzl:3: baz
+test/package:bar.bzl:4: bar
+test/package:bar.bzl:2: (RULE)`,
}, scopeBzl)
}
@@ -409,9 +485,15 @@
_not_macro(x)
`,
[]string{
- `6: The macro "macro" should have a keyword argument called "name".
+ `:6: The macro "macro" should have a keyword argument called "name".
-It is considered a macro because it calls a rule or another macro "_not_macro" on line 7.`,
+It is considered a macro because it calls a rule or another macro, call stack:
+
+test/package:test_file.bzl:6: macro
+test/package:test_file.bzl:7: _not_macro
+test/package:test_file.bzl:3: _not_macro
+test/package:test_file.bzl:4: my_rule
+test/package:test_file.bzl:1: (RULE)`,
},
scopeBzl)
}
