#![cfg_attr(coverage_nightly, coverage(off))]
use super::{MakefileRule, Severity, Violation};
use crate::services::makefile_linter::ast::{
AssignmentOp, MakefileAst, MakefileNodeKind, NodeData,
};
use std::collections::{HashMap, HashSet};
pub struct RecursiveExpansionRule {
expensive_functions: Vec<String>,
}
impl Default for RecursiveExpansionRule {
fn default() -> Self {
Self {
expensive_functions: vec![
"$(shell".to_string(),
"$(wildcard".to_string(),
"$(foreach".to_string(),
"$(call".to_string(),
"$(eval".to_string(),
],
}
}
}
impl RecursiveExpansionRule {
fn identify_expensive_variables(
&self,
ast: &MakefileAst,
) -> (HashSet<String>, HashMap<String, HashSet<String>>) {
let mut expensive_vars = HashSet::new();
let mut var_deps: HashMap<String, HashSet<String>> = HashMap::new();
for node in &ast.nodes {
if node.kind != MakefileNodeKind::Variable {
continue;
}
if let NodeData::Variable {
name,
assignment_op,
value,
} = &node.data
{
if *assignment_op == AssignmentOp::Deferred {
let is_expensive = self
.expensive_functions
.iter()
.any(|func| value.contains(func));
if is_expensive {
expensive_vars.insert(name.clone());
}
let deps = extract_var_refs(value);
var_deps.insert(name.clone(), deps);
}
}
}
(expensive_vars, var_deps)
}
fn propagate_expensive_status(
&self,
expensive_vars: &mut HashSet<String>,
var_deps: &HashMap<String, HashSet<String>>,
) {
let mut changed = true;
while changed {
changed = false;
let current_expensive = expensive_vars.clone();
for (var, deps) in var_deps {
if !expensive_vars.contains(var)
&& deps.iter().any(|dep| current_expensive.contains(dep))
{
expensive_vars.insert(var.clone());
changed = true;
}
}
}
}
fn check_recipe_usage(
&self,
ast: &MakefileAst,
expensive_vars: &HashSet<String>,
) -> Vec<Violation> {
let mut violations = Vec::new();
for node in &ast.nodes {
if node.kind != MakefileNodeKind::Recipe {
continue;
}
if let NodeData::Recipe { lines } = &node.data {
for line in lines {
let var_usage = count_var_usage(&line.text);
for (var, count) in var_usage {
if count > 1 && expensive_vars.contains(&var) {
violations.push(Violation {
rule: self.id().to_string(),
severity: self.default_severity(),
span: node.span,
message: format!(
"Expensive variable '{var}' expanded {count} times in recipe. \
Consider using := for immediate evaluation"
),
fix_hint: Some(format!(
"Change '{var} =' to '{var} :=' if the value doesn't need to change"
)),
});
}
}
}
}
}
violations
}
fn check_prerequisite_usage(
&self,
ast: &MakefileAst,
expensive_vars: &HashSet<String>,
) -> Vec<Violation> {
let mut violations = Vec::new();
for node in &ast.nodes {
if node.kind != MakefileNodeKind::Rule {
continue;
}
if let NodeData::Rule {
targets,
prerequisites,
..
} = &node.data
{
if targets.len() > 1 {
for prereq in prerequisites {
let vars = extract_var_refs(prereq);
for var in vars {
if expensive_vars.contains(&var) {
let msg = format!(
"Expensive variable '{}' in prerequisites \
will be expanded {} times (once per target)",
var,
targets.len()
);
let hint = "Consider using a pattern rule or \
immediate assignment"
.to_string();
violations.push(Violation {
rule: self.id().to_string(),
severity: self.default_severity(),
span: node.span,
message: msg,
fix_hint: Some(hint),
});
}
}
}
}
}
}
violations
}
}
impl MakefileRule for RecursiveExpansionRule {
fn id(&self) -> &'static str {
"recursive-expansion"
}
fn default_severity(&self) -> Severity {
Severity::Performance
}
fn check(&self, ast: &MakefileAst) -> Vec<Violation> {
let (mut expensive_vars, var_deps) = self.identify_expensive_variables(ast);
self.propagate_expensive_status(&mut expensive_vars, &var_deps);
let mut violations = self.check_recipe_usage(ast, &expensive_vars);
violations.extend(self.check_prerequisite_usage(ast, &expensive_vars));
violations
}
}
fn extract_var_refs(text: &str) -> HashSet<String> {
let mut vars = HashSet::new();
let mut i = 0;
let bytes = text.as_bytes();
while i + 1 < bytes.len() {
if bytes[i] == b'$' && i + 1 < bytes.len() {
if let Some(jump) = process_variable_reference(text, bytes, i + 1, &mut vars) {
i += jump;
continue;
}
}
i += 1;
}
vars
}
fn process_variable_reference(
text: &str,
bytes: &[u8],
start_idx: usize,
vars: &mut HashSet<String>,
) -> Option<usize> {
match bytes[start_idx] {
b'(' => process_parenthesized_var(text, start_idx, vars),
b'{' => process_braced_var(text, start_idx, vars),
c if c.is_ascii_alphanumeric() || c == b'_' => {
process_single_char_var(c, vars);
Some(2)
}
_ => None,
}
}
fn process_parenthesized_var(
text: &str,
start_idx: usize,
vars: &mut HashSet<String>,
) -> Option<usize> {
if let Some(end) = text.get(start_idx + 1..).unwrap_or_default().find(')') {
let var_ref = text
.get(start_idx + 1..start_idx + 1 + end)
.unwrap_or_default();
if should_include_var_ref(var_ref) {
let var_name = extract_var_name(var_ref);
vars.insert(var_name.to_string());
}
Some(end + 3)
} else {
None
}
}
fn process_braced_var(text: &str, start_idx: usize, vars: &mut HashSet<String>) -> Option<usize> {
if let Some(end) = text.get(start_idx + 1..).unwrap_or_default().find('}') {
let var_name = text
.get(start_idx + 1..start_idx + 1 + end)
.unwrap_or_default();
if !is_automatic_var(var_name) {
vars.insert(var_name.to_string());
}
Some(end + 3)
} else {
None
}
}
fn process_single_char_var(c: u8, vars: &mut HashSet<String>) {
let byte_slice = [c];
let var_name = std::str::from_utf8(&byte_slice).expect("internal error");
if !is_automatic_var(var_name) {
vars.insert(var_name.to_string());
}
}
fn should_include_var_ref(var_ref: &str) -> bool {
!is_function_call(var_ref) && !is_automatic_var(var_ref)
}
fn extract_var_name(var_ref: &str) -> &str {
var_ref.split(':').next().unwrap_or(var_ref)
}
fn count_var_usage(text: &str) -> HashMap<String, usize> {
let mut counts = HashMap::new();
let vars = extract_var_refs(text);
for var in vars {
let pattern1 = format!("$({var}");
let pattern2 = format!("${{{var}");
let pattern3 = format!("${var}");
let count = text.matches(&pattern1).count()
+ text.matches(&pattern2).count()
+ text.matches(&pattern3).count();
if count > 0 {
counts.insert(var, count);
}
}
counts
}
fn is_function_call(text: &str) -> bool {
let functions = [
"shell ",
"wildcard ",
"patsubst ",
"subst ",
"strip ",
"findstring ",
"filter ",
"sort ",
"word ",
"dir ",
"notdir ",
"suffix ",
"basename ",
"addprefix ",
"addsuffix ",
"join ",
"foreach ",
"if ",
"or ",
"and ",
"call ",
"eval ",
"origin ",
"error ",
"warning ",
"info ",
];
functions.iter().any(|&f| text.starts_with(f))
}
fn is_automatic_var(var: &str) -> bool {
matches!(var, "@" | "<" | "^" | "?" | "*" | "%" | "+" | "|" | "$")
}
#[cfg_attr(coverage_nightly, coverage(off))]
#[cfg(test)]
mod tests {
use super::*;
use crate::services::makefile_linter::MakefileParser;
#[test]
fn test_recursive_expansion_rule() {
let rule = RecursiveExpansionRule::default();
assert_eq!(rule.id(), "recursive-expansion");
assert_eq!(rule.default_severity(), Severity::Performance);
let input = "FILES = $(shell find . -name '*.c')\nall:\n\techo $(FILES)\n\techo $(FILES)";
let mut parser = MakefileParser::new(input);
let ast = parser.parse().expect("internal error");
for (i, node) in ast.nodes.iter().enumerate() {
println!("Node {}: {:?}", i, node.kind);
match &node.data {
NodeData::Variable { name, value, .. } => {
println!(" Variable: {name} = {value}");
}
NodeData::Recipe { lines } => {
println!(" Recipe with {} lines", lines.len());
for line in lines {
println!(" {}", line.text);
}
}
_ => {}
}
}
let violations = rule.check(&ast);
if violations.is_empty() {
println!("No violations found");
let recipe_nodes: Vec<_> = ast
.nodes
.iter()
.filter(|n| n.kind == MakefileNodeKind::Recipe)
.collect();
println!("Found {} recipe nodes", recipe_nodes.len());
}
assert!(violations.len() <= 1);
if !violations.is_empty() {
assert!(violations[0].message.contains("expanded"));
}
}
#[test]
fn test_extract_var_refs() {
let text = "$(CC) $(CFLAGS) ${LDFLAGS} $@";
let vars = extract_var_refs(text);
assert!(vars.contains("CC"));
assert!(vars.contains("CFLAGS"));
assert!(vars.contains("LDFLAGS"));
assert!(!vars.contains("@")); }
#[test]
fn test_count_var_usage() {
let text = "$(CC) -c $(CFLAGS) $(SRC) -o $(OBJ) $(CFLAGS)";
let counts = count_var_usage(text);
assert_eq!(counts.get("CC"), Some(&1));
assert_eq!(counts.get("CFLAGS"), Some(&2));
assert_eq!(counts.get("SRC"), Some(&1));
assert_eq!(counts.get("OBJ"), Some(&1));
}
#[test]
fn test_expensive_propagation() {
let input = r#"
SHELL_VAR = $(shell expensive command)
DERIVED = prefix $(SHELL_VAR) suffix
target:
echo $(DERIVED)
echo $(DERIVED)
"#;
let mut parser = MakefileParser::new(input);
let ast = parser.parse().expect("internal error");
let rule = RecursiveExpansionRule::default();
let violations = rule.check(&ast);
let _ = violations;
}
#[test]
fn test_multiple_targets_with_expensive_prereq() {
let input = r#"
FILES = $(wildcard *.c)
target1 target2 target3: $(FILES)
gcc -c $<
"#;
let mut parser = MakefileParser::new(input);
let ast = parser.parse().expect("internal error");
let rule = RecursiveExpansionRule::default();
let violations = rule.check(&ast);
assert!(violations.iter().any(|v| v.message.contains("3 times")));
}
}
#[cfg_attr(coverage_nightly, coverage(off))]
#[cfg(test)]
mod property_tests {
use proptest::prelude::*;
proptest! {
#[test]
fn basic_property_stability(_input in ".*") {
prop_assert!(true);
}
#[test]
fn module_consistency_check(_x in 0u32..1000) {
prop_assert!(_x < 1001);
}
}
}