Rewriting macro analyzer

[oreflow]

• 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)

[oreflow]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a significant and valuable rewrite of the macro analyzer. The new implementation is more robust, providing clearer diagnostics with call stacks, and the API is much cleaner. I've identified a critical bug in the graph traversal logic that could lead to incorrect analysis, along with a couple of medium-severity issues related to the formatting of warning messages which result in redundant or incomplete information being shown to the user. Overall, this is a great improvement, and with these fixes, it will be even better.

[gemini-code-assist]

You're taking the address of the loop variable call. In Go, the loop variable is reused for each iteration. This means that all traversalNode instances created in this loop will have their symbolCall field pointing to the same memory location, which will hold the value of the last element of the calls slice after the loop completes. This will lead to incorrect graph traversal.

To fix this, you should create a new variable within the loop for each iteration.

Suggested change

	for _, call := range calls {
	if _, isVisited := visited[*call.symbol]; isVisited {
	continue
	}
	ref := &traversalNode{parent: current, symbolCall: &call}
	for _, call := range calls {
	call := call // Make a copy of the loop variable to ensure the pointer is unique for each iteration.
	if _, isVisited := visited[*call.symbol]; isVisited {
	continue
	}
	ref := &traversalNode{parent: current, symbolCall: &call}

[gemini-code-assist]

The report.SelfDescription is included in the formatted string for the %q verb, and then it's passed again as a separate argument to be formatted by the first %s. This results in the macro's description being printed twice in the warning, which is redundant.

To provide a clear and non-repetitive message, you should construct the full call stack and print it once.

 						fullStack := report.SelfDescription
 						if pcs := report.PrintableCallStack(); pcs != "" {
 							fullStack += "\n" + pcs
 						}
 						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.
The function was considered a macro as it may produce targets via calls:
%s`,
 							report.SelfDescription, fullStack)))

[gemini-code-assist]

The generated warning message for an unnamed macro is missing the top-level function from the call stack. It only prints report.PrintableCallStack(), which contains the call stack below the function being analyzed. This provides an incomplete picture to the user.

To make the diagnostic message more helpful and consistent with other warnings, you should include the full call stack, starting with the function itself (report.SelfDescription).

 		fullStack := report.SelfDescription
 		if pcs := report.PrintableCallStack(); pcs != "" {
 			fullStack += "\n" + pcs
 		}
 		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.`,
 			def.Name,
 			fullStack,
 			def.Name)