Rewriting macro analyzer
- Including found call stack as motivation for decision (macro or not)
- Allowing traversal of cached files before loading non cached files
- Rewording to indicate whether function or call produces targets or not (by definition rule or macro)
- Cleans up API (separating external and internal structures and state)
diff --git a/WARNINGS.md b/WARNINGS.md
index 79b7531..690237c 100644
--- a/WARNINGS.md
+++ b/WARNINGS.md
@@ -1216,14 +1216,6 @@
+ my_macro(name = "foo", env = "bar")
```
-The linter allows the following functions to be called with positional arguments:
-
- * `load()`
- * `vardef()`
- * `export_files()`
- * `licenses()`
- * `print()`
-
--------------------------------------------------------------------------------
## <a name="print"></a>`print()` is a debug function and shouldn't be submitted
diff --git a/warn/BUILD.bazel b/warn/BUILD.bazel
index 225b5fc..6408244 100644
--- a/warn/BUILD.bazel
+++ b/warn/BUILD.bazel
@@ -3,6 +3,7 @@
go_library(
name = "warn",
srcs = [
+ "macro_analyzer.go",
"multifile.go",
"types.go",
"warn.go",
@@ -34,6 +35,7 @@
name = "warn_test",
size = "small",
srcs = [
+ "macro_analyzer_test.go",
"types_test.go",
"warn_bazel_api_test.go",
"warn_bazel_operation_test.go",
@@ -53,6 +55,7 @@
"//build",
"//tables",
"//testutils",
+ "@com_github_google_go_cmp//cmp",
],
)
diff --git a/warn/docs/warnings.textproto b/warn/docs/warnings.textproto
index 9d3eb8e..16c390a 100644
--- a/warn/docs/warnings.textproto
+++ b/warn/docs/warnings.textproto
@@ -884,13 +884,7 @@
"```diff\n"
"- my_macro(\"foo\", \"bar\")\n"
"+ my_macro(name = \"foo\", env = \"bar\")\n"
- "```\n\n"
- "The linter allows the following functions to be called with positional arguments:\n\n"
- " * `load()`\n"
- " * `vardef()`\n"
- " * `export_files()`\n"
- " * `licenses()`\n"
- " * `print()`"
+ "```"
}
warnings: {
diff --git a/warn/macro_analyzer.go b/warn/macro_analyzer.go
new file mode 100644
index 0000000..e95d07b
--- /dev/null
+++ b/warn/macro_analyzer.go
@@ -0,0 +1,302 @@
+/*
+Copyright 2025 Google LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// Analyzer to determine if a function is a macro (produces targets).
+
+package warn
+
+import (
+ "fmt"
+ "strings"
+
+ "github.com/bazelbuild/buildtools/build"
+ "github.com/bazelbuild/buildtools/labels"
+)
+
+// MacroAnalyzer is an object that analyzes the directed graph of functions calling each other,
+// determining whether a function produces targets or not.
+type MacroAnalyzer struct {
+ fileReader *FileReader
+ cache map[symbolRef]*symbolAnalysisResult
+}
+
+// NewMacroAnalyzer creates and initiates an instance of macroAnalyzer.
+func NewMacroAnalyzer(fileReader *FileReader) MacroAnalyzer {
+ if fileReader == nil {
+ // If no file reader is provided, a default one is provided which fails on all reads.
+ // This can still be used if functions are preloaded via cache.
+ fileReader = NewFileReader(func(_ string) ([]byte, error) {
+ return nil, fmt.Errorf("tried to read file without file reader")
+ })
+ }
+ return MacroAnalyzer{
+ fileReader: fileReader,
+ cache: make(map[symbolRef]*symbolAnalysisResult),
+ }
+}
+
+// MacroAnalyzerReport defines the results of analyzing a function using the MacroAnalyzer.
+type MacroAnalyzerReport struct {
+ SelfDescription string
+ symbolAnalysis *symbolAnalysisResult
+}
+
+// CanProduceTargets returns true if provided function has any call path which produces a target.
+// A function which produces targets is by definition either a rule or a macro.
+func (mar *MacroAnalyzerReport) CanProduceTargets() bool {
+ if mar.symbolAnalysis == nil {
+ return false
+ }
+ return mar.symbolAnalysis.canProduceTargets
+}
+
+// PrintableCallStack returns a user-readable call stack, providing a path for how a function may
+// produce targets.
+func (mar *MacroAnalyzerReport) PrintableCallStack() string {
+ if mar.symbolAnalysis == nil {
+ return ""
+ }
+ return strings.Join(mar.symbolAnalysis.callStackFrames, "\n")
+}
+
+// AnalyzeFn analyzes the provided def statement, and returns a report containing whether it produces a target (is a macro) or not.
+func (ma *MacroAnalyzer) AnalyzeFn(f *build.File, def *build.DefStmt) (*MacroAnalyzerReport, error) {
+ ma.fileReader.AddFileToCache(f)
+ call := symbolCall{symbol: &symbolRef{pkg: f.Pkg, label: f.Label, name: def.Name}, line: exprLine(def)}
+ report, err := ma.analyzeSymbol(call)
+ if err != nil {
+ return nil, err
+ }
+ return &MacroAnalyzerReport{
+ SelfDescription: call.asCallStackFrame(),
+ symbolAnalysis: report,
+ }, nil
+}
+
+// AnalyzeFnCall analyzes a function call to see if it can produce a targets or not.
+func (ma *MacroAnalyzer) AnalyzeFnCall(f *build.File, call *build.CallExpr) (*MacroAnalyzerReport, error) {
+ ma.fileReader.AddFileToCache(f)
+ if symbolName := callExprToString(call); symbolName != "" {
+ call := symbolCall{symbol: &symbolRef{pkg: f.Pkg, label: f.Label, name: symbolName}, line: exprLine(call)}
+ report, err := ma.analyzeSymbol(call)
+ if err != nil {
+ return nil, err
+ }
+ return &MacroAnalyzerReport{
+ SelfDescription: call.asCallStackFrame(),
+ symbolAnalysis: report,
+ }, nil
+ }
+ return nil, fmt.Errorf("error checking call for being a macro at %s:%d", f.Path, exprLine(call))
+}
+
+// symbolAnalysisResult stores the result of analyzing a symbolRef.
+type symbolAnalysisResult struct {
+ canProduceTargets bool
+ callStackFrames []string
+}
+
+// symbolRef represents a symbol in a specific file.
+type symbolRef struct {
+ pkg string
+ label string
+ name string
+}
+
+// symbolCall represents a call (by line number) to a symbolRef.
+type symbolCall struct {
+ line int
+ symbol *symbolRef
+}
+
+func (sc *symbolCall) asCallStackFrame() string {
+ return fmt.Sprintf("%s:%s:%d %s", sc.symbol.pkg, sc.symbol.label, sc.line, sc.symbol.name)
+}
+
+// traversalNode is an internal structure to keep track of symbol call hierarchies while traversing symbols.
+type traversalNode struct {
+ parent *traversalNode
+ symbolCall *symbolCall
+}
+
+// analyzeSymbol identifies a given symbol, and traverses its call stack to detect if any downstream calls can generate targets.
+func (ma *MacroAnalyzer) analyzeSymbol(sc symbolCall) (*symbolAnalysisResult, error) {
+ queue := []*traversalNode{{symbolCall: &sc}}
+ visited := make(map[symbolRef]bool)
+
+ var current *traversalNode
+ var nodeProducedTarget *traversalNode
+
+ for len(queue) > 0 && nodeProducedTarget == nil {
+ current, queue = queue[0], queue[1:]
+ visited[*current.symbolCall.symbol] = true
+
+ if producesTarget(current.symbolCall.symbol) {
+ nodeProducedTarget = current
+ }
+ calls, err := ma.expandSymbol(current.symbolCall.symbol)
+ if err != nil {
+ return nil, err
+ }
+ for _, call := range calls {
+ if _, isVisited := visited[*call.symbol]; isVisited {
+ continue
+ }
+ ref := &traversalNode{parent: current, symbolCall: &call}
+ // adding symbol to front/back of queue depending on whether the file is already loaded or not.
+ if ma.fileReader.IsCached(call.symbol.pkg, call.symbol.label) {
+ queue = append([]*traversalNode{ref}, queue...)
+ } else {
+ queue = append(queue, ref)
+ }
+ }
+ }
+ if nodeProducedTarget == nil {
+ // If no node produced a target, all visited nodes can be cached as non-macros.
+ for symbol := range visited {
+ ma.cache[symbol] = &symbolAnalysisResult{canProduceTargets: false}
+ }
+ } else {
+ // If a node produced a target, the call stack above the node can be cached as producing targets.
+ var callStackFrames []string
+ node := nodeProducedTarget
+ for node != nil {
+ ma.cache[*node.symbolCall.symbol] = &symbolAnalysisResult{canProduceTargets: true, callStackFrames: callStackFrames}
+ callStackFrames = append([]string{node.symbolCall.asCallStackFrame()}, callStackFrames...)
+ node = node.parent
+ }
+ }
+ return ma.cache[*sc.symbol], nil
+}
+
+// exprLine returns the start line of an expression
+func exprLine(expr build.Expr) int {
+ start, _ := expr.Span()
+ return start.Line
+}
+
+// expandSymbol expands the provided symbol, returning a list of other symbols that it references.
+// e.g. if the symbol is an alias, the aliased symbol is returned, or if the symbol is a function, the symbols it calls downstream are returned.
+func (ma *MacroAnalyzer) expandSymbol(symbol *symbolRef) ([]symbolCall, error) {
+ f := ma.fileReader.GetFile(symbol.pkg, symbol.label)
+ if f == nil {
+ return nil, fmt.Errorf("unable to find file %s:%s", symbol.pkg, symbol.label)
+ }
+
+ for _, stmt := range f.Stmt {
+ switch stmt := stmt.(type) {
+ case *build.AssignExpr:
+ if lhsIdent, ok := stmt.LHS.(*build.Ident); ok && lhsIdent.Name == symbol.name {
+ if rhsIdent, ok := stmt.RHS.(*build.Ident); ok {
+ return []symbolCall{{
+ symbol: &symbolRef{pkg: f.Pkg, label: f.Label, name: rhsIdent.Name},
+ line: exprLine(stmt),
+ }}, nil
+ }
+ if fnName := callExprToString(stmt.RHS); fnName != "" {
+ return []symbolCall{{
+ symbol: &symbolRef{pkg: f.Pkg, label: f.Label, name: fnName},
+ line: exprLine(stmt),
+ }}, nil
+ }
+ }
+ case *build.DefStmt:
+ if stmt.Name == symbol.name {
+ var calls []symbolCall
+ build.Walk(stmt, func(x build.Expr, _ []build.Expr) {
+ if fnName := callExprToString(x); fnName != "" {
+ calls = append(calls, symbolCall{
+ symbol: &symbolRef{pkg: f.Pkg, label: f.Label, name: fnName},
+ line: exprLine(x),
+ })
+ }
+ })
+ return calls, nil
+ }
+ case *build.LoadStmt:
+ label := labels.ParseRelative(stmt.Module.Value, f.Pkg)
+ if label.Repository != "" || label.Target == "" {
+ continue
+ }
+ for i, from := range stmt.From {
+ if stmt.To[i].Name == symbol.name {
+ return []symbolCall{{
+ symbol: &symbolRef{pkg: label.Package, label: label.Target, name: from.Name},
+ line: exprLine(stmt),
+ }}, nil
+ }
+ }
+ }
+ }
+ return nil, nil
+}
+
+// callExprToString converts a callExpr to its "symbol name"
+func callExprToString(expr build.Expr) string {
+ call, ok := expr.(*build.CallExpr)
+ if !ok {
+ return ""
+ }
+
+ if fnIdent, ok := call.X.(*build.Ident); ok {
+ return fnIdent.Name
+ }
+
+ // call of the format obj.fn(...), ignores call if anything other than ident.fn().
+ if fn, ok := call.X.(*build.DotExpr); ok {
+ if obj, ok := fn.X.(*build.Ident); ok {
+ return fmt.Sprintf("%s.%s", obj.Name, fn.Name)
+ }
+ }
+ return ""
+}
+
+// native functions which do not produce targets (https://bazel.build/rules/lib/toplevel/native).
+var nativeRuleExceptions = map[string]bool{
+ "native.existing_rule": true,
+ "native.existing_rules": true,
+ "native.exports_files": true,
+ "native.glob": true,
+ "native.module_name": true,
+ "native.module_version": true,
+ "native.package_default_visibility": true,
+ "native.package_group": true,
+ "native.package_name": true,
+ "native.package_relative_label": true,
+ "native.repo_name": true,
+ "native.repository_name": true,
+ "native.subpackages": true,
+}
+
+// producesTargets returns true if the symbol name is a known generator of a target.
+func producesTarget(s *symbolRef) bool {
+ // Calls to the macro() symbol produce a symbolic macro (https://bazel.build/extending/macros).
+ if s.name == "macro" {
+ return true
+ }
+ // Calls to the rule() symbol define a rule (https://bazel.build/extending/rules).
+ if s.name == "rule" {
+ return true
+ }
+ // Calls to native. invokes native rules (except defined list of native helper functions).
+ // https://bazel.build/rules/lib/toplevel/native
+ if strings.HasPrefix(s.name, "native.") {
+ if _, ok := nativeRuleExceptions[s.name]; !ok {
+ return true
+ }
+ }
+ return false
+}
diff --git a/warn/macro_analyzer_test.go b/warn/macro_analyzer_test.go
new file mode 100644
index 0000000..a6c5314
--- /dev/null
+++ b/warn/macro_analyzer_test.go
@@ -0,0 +1,280 @@
+/*
+Copyright 2025 Google LLC
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+ https://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package warn
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/bazelbuild/buildtools/build"
+ "github.com/google/go-cmp/cmp"
+)
+
+func mustFindDefStatement(f *build.File, name string) *build.DefStmt {
+ for _, stmt := range f.Stmt {
+ if def, ok := stmt.(*build.DefStmt); ok {
+ if def.Name == name {
+ return def
+ }
+ }
+ }
+ panic(fmt.Sprintf("unable to find def statement matching %q", name))
+}
+
+func mustFindCallExpression(f *build.File, name string) *build.CallExpr {
+ for _, stmt := range f.Stmt {
+ if call, ok := stmt.(*build.CallExpr); ok {
+ if fnIdent, ok := call.X.(*build.Ident); ok && fnIdent.Name == name {
+ return call
+ }
+ }
+ }
+ panic(fmt.Sprintf("unable to find call expression matching %q", name))
+}
+
+func fileReaderWithFiles(fileContents map[string]string) *FileReader {
+ return NewFileReader(func(filename string) ([]byte, error) {
+ return []byte(fileContents[filename]), nil
+ })
+}
+
+func TestAnalyzeFn(t *testing.T) {
+ defaultFilename := "BUILD"
+ defaultPackage := "//package/path"
+ defaultFilepath := fmt.Sprintf("%s/%s", defaultPackage, defaultFilename)
+ tests := []struct {
+ name string
+ fileContents map[string]string
+ wantCanProduceTargets bool
+ wantStackTrace string
+ }{
+ {
+ name: "non_macro",
+ fileContents: map[string]string{
+ defaultFilepath: `
+def other_function():
+ pass
+
+def test_symbol():
+ other_function()
+`,
+ },
+ wantCanProduceTargets: false,
+ wantStackTrace: "",
+ },
+ {
+ name: "with_infinite_recursion",
+ fileContents: map[string]string{
+ defaultFilepath: `
+def first_function():
+ test_symbol()
+
+def test_symbol():
+ first_function()
+`,
+ },
+ wantCanProduceTargets: false,
+ wantStackTrace: "",
+ },
+ {
+ name: "macro_within_single_file",
+ fileContents: map[string]string{
+ defaultFilepath: `
+macro_def = macro()
+
+def test_symbol():
+ macro_def()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:5 macro_def
+//package/path:BUILD:2 macro`,
+ },
+ {
+ name: "macro_through_load_statement",
+ fileContents: map[string]string{
+ "package/other_path/file.bzl": `
+imported_rule = rule()
+`,
+ defaultFilepath: `
+load("//package/other_path:file.bzl", "imported_rule")
+
+def test_symbol():
+ imported_rule()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:5 imported_rule
+package/other_path:file.bzl:2 imported_rule
+package/other_path:file.bzl:2 rule`,
+ },
+ {
+ name: "with_load_statements_prioritizes_local_statements",
+ fileContents: map[string]string{
+ "package/other_path/file.bzl": `
+imported_rule = rule()
+`,
+ defaultFilepath: `
+load("//package/other_path:file.bzl", "imported_rule")
+
+def test_symbol():
+ imported_rule()
+ native.cc_library()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:6 native.cc_library`,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ ma := NewMacroAnalyzer(fileReaderWithFiles(tc.fileContents))
+ f := ma.fileReader.GetFile(defaultPackage, defaultFilename)
+ macroDef := mustFindDefStatement(f, "test_symbol")
+
+ report, err := ma.AnalyzeFn(f, macroDef)
+
+ if err != nil {
+ t.Errorf("Got unexpected error %s", err)
+ }
+ if diff := cmp.Diff(tc.wantCanProduceTargets, report.CanProduceTargets()); diff != "" {
+ t.Errorf("AnalyzeFn.CanProduceTargets returned unexpected diff %s", diff)
+ }
+ if diff := cmp.Diff(tc.wantStackTrace, report.PrintableCallStack()); diff != "" {
+ t.Errorf("AnalyzeFn.PrintableCallStack returned unexpected diff %s", diff)
+ }
+ })
+ }
+}
+
+func TestAnalyzeFnCall(t *testing.T) {
+ defaultFilename := "BUILD"
+ defaultPackage := "//package/path"
+ defaultFilepath := fmt.Sprintf("%s/%s", defaultPackage, defaultFilename)
+ tests := []struct {
+ name string
+ fileContents map[string]string
+ wantCanProduceTargets bool
+ wantStackTrace string
+ }{
+ {
+ name: "non_macro",
+ fileContents: map[string]string{
+ defaultFilepath: `
+def test_symbol():
+ pass
+
+test_symbol()
+`,
+ },
+ wantCanProduceTargets: false,
+ wantStackTrace: "",
+ },
+ {
+ name: "with_infinite_recursion",
+ fileContents: map[string]string{
+ defaultFilepath: `
+def test_symbol():
+ second_function()
+
+def second_function():
+ test_symbol()
+
+test_symbol()
+`,
+ },
+ wantCanProduceTargets: false,
+ wantStackTrace: "",
+ },
+ {
+ name: "macro_within_single_file",
+ fileContents: map[string]string{
+ defaultFilepath: `
+macro_def = macro()
+
+def test_symbol():
+ macro_def()
+
+test_symbol()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:5 macro_def
+//package/path:BUILD:2 macro`,
+ },
+ {
+ name: "macro_through_load_statement",
+ fileContents: map[string]string{
+ "package/other_path/file.bzl": `
+imported_rule = rule()
+`,
+ defaultFilepath: `
+load("//package/other_path:file.bzl", "imported_rule")
+
+def test_symbol():
+ imported_rule()
+
+test_symbol()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:5 imported_rule
+package/other_path:file.bzl:2 imported_rule
+package/other_path:file.bzl:2 rule`,
+ },
+ {
+ name: "with_load_statements_prioritizes_local_statements",
+ fileContents: map[string]string{
+ "package/other_path/file.bzl": `
+imported_rule = rule()
+`,
+ defaultFilepath: `
+load("//package/other_path:file.bzl", "imported_rule")
+
+def test_symbol():
+ imported_rule()
+ macro()
+
+test_symbol()
+`,
+ },
+ wantCanProduceTargets: true,
+ wantStackTrace: `//package/path:BUILD:6 macro`,
+ },
+ }
+
+ for _, tc := range tests {
+ t.Run(tc.name, func(t *testing.T) {
+ ma := NewMacroAnalyzer(fileReaderWithFiles(tc.fileContents))
+ f := ma.fileReader.GetFile(defaultPackage, defaultFilename)
+ call := mustFindCallExpression(f, "test_symbol")
+
+ report, err := ma.AnalyzeFnCall(f, call)
+
+ if err != nil {
+ t.Errorf("Got unexpected error %s", err)
+ }
+ if diff := cmp.Diff(tc.wantCanProduceTargets, report.CanProduceTargets()); diff != "" {
+ t.Errorf("AnalyzeFn.CanProduceTargets returned unexpected diff %s", diff)
+ }
+ if diff := cmp.Diff(tc.wantStackTrace, report.PrintableCallStack()); diff != "" {
+ t.Errorf("AnalyzeFn.PrintableCallStack returned unexpected diff %s", diff)
+ }
+ })
+ }
+}
diff --git a/warn/multifile.go b/warn/multifile.go
index 73f5af4..174da4e 100644
--- a/warn/multifile.go
+++ b/warn/multifile.go
@@ -38,6 +38,23 @@
}
}
+// AddFileToCache adds the provided file to the filereader cache.
+func (fr *FileReader) AddFileToCache(f *build.File) {
+ if f != nil {
+ fr.cache[f.Path] = f
+ }
+}
+
+// IsCached returns true if the file is present in the cache.
+func (fr *FileReader) IsCached(pkg, label string) bool {
+ filename := label
+ if pkg != "" {
+ filename = pkg + "/" + label
+ }
+ _, contains := fr.cache[filename]
+ return contains
+}
+
// retrieveFile reads a Starlark file using only the readFile method
// (without using the cache).
func (fr *FileReader) retrieveFile(filename string) *build.File {
@@ -72,6 +89,6 @@
file.Pkg = pkg
file.Label = label
}
- fr.cache[filename] = file
+ fr.AddFileToCache(file)
return file
}
diff --git a/warn/warn_bazel.go b/warn/warn_bazel.go
index be6000c..02f6fd8 100644
--- a/warn/warn_bazel.go
+++ b/warn/warn_bazel.go
@@ -175,26 +175,28 @@
if f.Type != build.TypeBuild {
return nil
}
- macroAnalyzer := newMacroAnalyzer(fileReader)
- macroAnalyzer.files[f.Pkg+":"+f.Label] = analyzeFile(f)
+ macroAnalyzer := NewMacroAnalyzer(fileReader)
for _, expr := range f.Stmt {
build.Walk(expr, func(x build.Expr, _ []build.Expr) {
if fnCall, ok := x.(*build.CallExpr); ok {
- fnIdent, ok := fnCall.X.(*build.Ident)
- if !ok {
+ report, err := macroAnalyzer.AnalyzeFnCall(f, fnCall)
+ if err != nil {
+ // TODO: Analysis errors are simply ignored as buildifier does not currently handle errors.
return
}
-
- if macroAnalyzer.IsRuleOrMacro(function{pkg: f.Pkg, filename: f.Label, name: fnIdent.Name}).isRuleOrMacro {
+ if report.CanProduceTargets() {
for _, arg := range fnCall.List {
if _, ok := arg.(*build.AssignExpr); ok || arg == nil {
continue
}
findings = append(findings, makeLinterFinding(fnCall, fmt.Sprintf(
`All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro %q with positional arguments.`,
- fnIdent.Name)))
+Found call to rule or macro %q with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+%s
+%s`,
+ report.SelfDescription, report.SelfDescription, report.PrintableCallStack())))
return
}
}
diff --git a/warn/warn_bazel_test.go b/warn/warn_bazel_test.go
index 9201cea..ec089a8 100644
--- a/warn/warn_bazel_test.go
+++ b/warn/warn_bazel_test.go
@@ -124,14 +124,43 @@
my_function("foo", "bar")
`,
[]string{
- `6: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_macro" with positional arguments.`,
- `7: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_rule" with positional arguments.`,
+ `:6: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:6 my_macro" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:6 my_macro
+test/package:BUILD:1 macro`,
+ `:7: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:7 my_rule" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:7 my_rule
+test/package:BUILD:2 rule`,
},
scopeBuild)
}
+func TestPositionalArgumentsWithNestedCalls(t *testing.T) {
+ checkFindings(t, "positional-args", `
+def macro3(foo, *args, **kwargs):
+ macro2()
+
+def macro2(foo, *, name):
+ macro1()
+
+def macro1(foo, name, bar):
+ native.java_library()
+
+macro3("foo")
+`,
+ []string{`:10: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:10 macro3" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:10 macro3
+test/package:BUILD:2 macro2
+test/package:BUILD:5 macro1
+test/package:BUILD:8 native.java_library`},
+ scopeBuild)
+}
+
func TestPositionalArgumentsWarnsWhenCalledInNestedContexts(t *testing.T) {
checkFindings(t, "positional-args", `
my_macro = macro()
@@ -149,13 +178,25 @@
`,
[]string{
`6: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_macro" with positional arguments.`,
- `7: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_rule" with positional arguments.`,
- `10: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_macro" with positional arguments.`,
- `11: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
-Found call to rule or macro "my_rule" with positional arguments.`,
+Found call to rule or macro "test/package:BUILD:6 my_macro" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:6 my_macro
+test/package:BUILD:1 macro`,
+ `:7: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:7 my_rule" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:7 my_rule
+test/package:BUILD:2 rule`,
+ `:10: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:10 my_macro" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:10 my_macro
+test/package:BUILD:1 macro`,
+ `:11: All calls to rules or macros should pass arguments by keyword (arg_name=value) syntax.
+Found call to rule or macro "test/package:BUILD:11 my_rule" with positional arguments.
+The function was considered a macro as it may produce targets via calls:
+test/package:BUILD:11 my_rule
+test/package:BUILD:2 rule`,
},
scopeBuild)
@@ -233,7 +274,7 @@
)
some_rule(
- arg1 = "normal/path/file.txt",
+ arg1 = "normal/path/file.txt",
arg2 = "/external/repo/file.py",
arg3 = ["file1.txt", "/external/another/file.cc"],
)`,
diff --git a/warn/warn_macro.go b/warn/warn_macro.go
index f77c2f3..46befa7 100644
--- a/warn/warn_macro.go
+++ b/warn/warn_macro.go
@@ -23,31 +23,8 @@
"strings"
"github.com/bazelbuild/buildtools/build"
- "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
- filename string // name of a .bzl file relative to the package
- name string // original name of the function
-}
-
-func (f function) label() string {
- 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
-}
-
// acceptsNameArgument checks whether a function can accept a named argument called "name",
// either directly or via **kwargs.
func acceptsNameArgument(def *build.DefStmt) bool {
@@ -59,251 +36,12 @@
return false
}
-// fileData represents information about rules and functions extracted from a file
-type fileData struct {
- loadedSymbols map[string]function // Symbols loaded from other files.
- rulesOrMacros map[string]bool // all rules or macros 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{}
- }
-
- report := fileData{
- loadedSymbols: make(map[string]function),
- rulesOrMacros: make(map[string]bool),
- functions: make(map[string]map[string]funCall),
- aliases: make(map[string]function),
- }
-
- // Collect loaded symbols
- 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 {
- report.loadedSymbols[load.To[i].Name] = function{label.Package, label.Target, from.Name}
- }
- }
-
- 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}, report.loadedSymbols)
- continue
- }
-
- call, ok := stmt.RHS.(*build.CallExpr)
- if !ok {
- continue
- }
- if ident, ok := call.X.(*build.Ident); ok {
- if ident.Name == "rule" || ident.Name == "macro" {
- report.rulesOrMacros[lhsIdent.Name] = true
- continue
- }
- }
- case *build.DefStmt:
- report.functions[stmt.Name] = getFunCalls(stmt, f.Pkg, f.Label, report.loadedSymbols)
- default:
- continue
- }
- }
- return report
-}
-
-// functionReport represents the analysis result of a function
-type functionReport struct {
- isRuleOrMacro bool // whether the function is a macro (or a rule)
- fc *funCall // a call to the rule or another macro
-}
-
-// 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
-}
-
-// 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
- }
- if ma.fileReader == nil {
- fd := fileData{}
- ma.files[filename] = fd
- return fd
- }
- f := ma.fileReader.GetFile(pkg, label)
- fd := analyzeFile(f)
- ma.files[filename] = fd
- return fd
-}
-
-// IsMacro is a public function that checks whether the given function is a macro
-func (ma macroAnalyzer) IsRuleOrMacro(fn function) (report functionReport) {
- // Check the cache first
- if cached, ok := ma.cache[fn]; ok {
- return cached
- }
- // Write a negative result to the cache before analyzing. This will prevent stack overflow crashes
- // if the input data contains recursion.
- ma.cache[fn] = report
- defer func() {
- // Update the cache with the actual result
- 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
- default:
- report.isRuleOrMacro = 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.IsRuleOrMacro(alias).isRuleOrMacro {
- report.isRuleOrMacro = true
- }
- return
- }
-
- // Check whether fn.name is a rule or macro
- if fileData.rulesOrMacros[fn.name] {
- report.isRuleOrMacro = true
- return
- }
-
- // Check whether fn.name is a loaded symbol from another file
- if externalFn, ok := fileData.loadedSymbols[fn.name]; ok {
- if ma.IsRuleOrMacro(externalFn).isRuleOrMacro {
- report.isRuleOrMacro = 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)
- }
- }
-
- for _, fc := range append(knownFunCalls, newFunCalls...) {
- if ma.IsRuleOrMacro(fc.function).isRuleOrMacro {
- report.isRuleOrMacro = true
- report.fc = &fc
- return
- }
- }
-
- return
-}
-
-// 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),
- }
-}
-
func unnamedMacroWarning(f *build.File, fileReader *FileReader) []*LinterFinding {
if f.Type != build.TypeBzl {
return nil
}
- macroAnalyzer := newMacroAnalyzer(fileReader)
- macroAnalyzer.files[f.Pkg+":"+f.Label] = analyzeFile(f)
+ macroAnalyzer := NewMacroAnalyzer(fileReader)
findings := []*LinterFinding{}
for _, stmt := range f.Stmt {
@@ -316,24 +54,27 @@
continue
}
- report := macroAnalyzer.IsRuleOrMacro(function{f.Pkg, f.Label, def.Name})
- if !report.isRuleOrMacro {
+ report, err := macroAnalyzer.AnalyzeFn(f, def)
+ if err != nil {
+ // TODO: Analysis errors are simply ignored as buildifier does not currently handle errors.
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(`
-
-It is considered a macro because it calls a rule or another macro %q on line %d.
+ if !report.CanProduceTargets() {
+ continue
+ }
+ msg := fmt.Sprintf(`The macro %q should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+%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)
- }
+* 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.`,
+ def.Name,
+ report.PrintableCallStack(),
+ 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 13c6ce3..b2e8569 100644
--- a/warn/warn_macro_test.go
+++ b/warn/warn_macro_test.go
@@ -24,7 +24,7 @@
my_rule = rule()
-def macro(x):
+def a_macro(x):
foo()
my_rule(name = x)
@@ -43,12 +43,13 @@
[native.cc_library() for i in x]
`,
[]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 "native.cc_library" on line 21.`,
+ `:5: The macro "a_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:7 my_rule
+test/package:test_file.bzl:3 rule`,
+ `:19: The macro "another_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:21 native.cc_library`,
},
scopeBzl)
@@ -70,32 +71,22 @@
checkFindings(t, "unnamed-macro", `
my_rule = rule()
-def macro(name):
+def a_macro(name):
my_rule(name = name)
-alias = macro
+alias = a_macro
def bad_macro():
for x in y:
alias(x)
`,
[]string{
- `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.`,
- },
- scopeBzl)
-
- checkFindings(t, "unnamed-macro", `
-symbolic_macro = macro()
-
-def bad_macro():
- symbolic_macro(x)
-`,
- []string{
- `3: The macro "bad_macro" should have a keyword argument called "name".
-
-It is considered a macro because it calls a rule or another macro "symbolic_macro" on line 4.`,
+ `:8: The macro "bad_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:10 alias
+test/package:test_file.bzl:6 a_macro
+test/package:test_file.bzl:4 my_rule
+test/package:test_file.bzl:1 rule`,
},
scopeBzl)
@@ -115,18 +106,25 @@
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 "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 "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.`,
+ `:3: The macro "macro1" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+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 as it may produce targets via calls:
+test/package:test_file.bzl:7 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 as it may produce targets via calls:
+test/package:test_file.bzl:10 macro2
+test/package:test_file.bzl:7 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 as it may produce targets via calls:
+test/package:test_file.bzl:13 my_rule
+test/package:test_file.bzl:1 rule`,
},
scopeBzl)
}
@@ -137,15 +135,15 @@
checkFindings(t, "unnamed-macro", `
my_rule = rule()
-def macro():
- macro()
+def a_macro():
+ a_macro()
`, []string{}, scopeBzl)
checkFindings(t, "unnamed-macro", `
my_rule = rule()
-def macro():
- macro()
+def a_macro():
+ a_macro()
`, []string{}, scopeBzl)
checkFindings(t, "unnamed-macro", `
@@ -172,12 +170,15 @@
my_rule()
`,
[]string{
- `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 "my_rule" on line 8.`,
+ `:3: The macro "foo" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:4 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 as it may produce targets via calls:
+test/package:test_file.bzl:8 my_rule
+test/package:test_file.bzl:1 rule`,
},
scopeBzl)
}
@@ -226,36 +227,48 @@
f()
`,
[]string{
- `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.
+ `:4: The macro "macro1" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:5 abc
+test/package:subdir1/foo.bzl:1 bar
+test/package:subdir1/foo.bzl:6 foo
+test/package:subdir1/foo.bzl:3 native.foo_binary
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 "_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".
-
-It is considered a macro because it calls a rule or another macro "baz" on line 8.
+* 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".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:8 baz
+test/package:subdir2/baz.bzl:2 baz
+test/package:subdir2/baz.bzl:7 bar
+test/package:subdir1/foo.bzl:2 bar
+test/package:subdir1/foo.bzl:6 foo
+test/package:subdir1/foo.bzl:3 native.foo_binary
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 "_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".
-
-It is considered a macro because it calls a rule or another macro "qux" on line 11.
+* 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".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:11 qux
+test/package:subdir2/baz.bzl:2 qux
+test/package:subdir2/baz.bzl:10 your_rule
+test/package:subdir1/foo.bzl:2 my_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.
- * 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 "_macro3").
- * Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
+* 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 "_macro3").
+* Otherwise, add a "name" argument. If possible, use that name when calling other macros/rules.`,
},
scopeBzl)
}
@@ -294,14 +307,18 @@
load(":foo.bzl", "foo", "baz")
load(":bar.bzl", quux = "qux")
-def macro():
+def a_macro():
foo()
baz()
quux()
`, []string{
- `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.`,
+ `:4: The macro "a_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:7 quux
+test/package:bar.bzl:2 qux
+test/package:bar.bzl:10 quuux
+test/package:foo.bzl:2 qux
+test/package:foo.bzl:11 native.cc_library`,
}, scopeBzl)
}
@@ -320,12 +337,16 @@
my_rule = rule()
-def macro():
+def a_macro():
bar()
`, []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 "bar" on line 6.`,
+ `:5: The macro "a_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:6 bar
+test/package:foo.bzl:1 foo
+test/package:foo.bzl:5 some_rule
+test/package:test_file.bzl:2 my_rule
+test/package:test_file.bzl:3 rule`,
}, scopeBzl)
}
@@ -362,18 +383,23 @@
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.`,
+ `:6: The macro "macro1" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:7 a
+:a.bzl:1 a
+:a.bzl:1 rule`,
+ `:9: The macro "macro2" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:11 native.cc_library`,
+ `:13: The macro "macro3" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:15 a
+:a.bzl:1 a
+:a.bzl:1 rule`,
+ `:17: The macro "macro4" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:19 r
+test/package:test_file.bzl:21 rule`,
}, scopeBzl)
if len(fileReaderRequests) == 1 && fileReaderRequests[0] == "a.bzl" {
@@ -405,9 +431,13 @@
def macro2(name):
baz()
`, []string{
- `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.`,
+ `:5: The macro "macro1" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:6 baz
+test/package:test_file.bzl:3 bar
+test/package:bar.bzl:1 bar
+test/package:bar.bzl:4 my_rule
+test/package:bar.bzl:2 rule`,
}, scopeBzl)
}
@@ -418,13 +448,15 @@
def _not_macro(x):
my_rule(name = x)
-def macro(x):
+def a_macro(x):
_not_macro(x)
`,
[]string{
- `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.`,
+ `:6: The macro "a_macro" should have a keyword argument called "name".
+It is considered a macro as it may produce targets via calls:
+test/package:test_file.bzl:7 _not_macro
+test/package:test_file.bzl:4 my_rule
+test/package:test_file.bzl:1 rule`,
},
scopeBzl)
}
@@ -433,7 +465,7 @@
checkFindings(t, "unnamed-macro", `
my_rule = rule()
-def macro(name: string):
+def a_macro(name: string):
my_rule(name)
`,
[]string{},
@@ -442,7 +474,7 @@
checkFindings(t, "unnamed-macro", `
my_rule = rule()
-def macro(name: string = "default"):
+def a_macro(name: string = "default"):
my_rule(name)
`,
[]string{},
