21054809427

Committed 16 Jan 2026 03:40AM UTC coverage: 88.637%. First build

Build # 21054809427

Build Type

Pull #64

github

Committed by

kaidokert

Commit Message

Optimize define_property to avoid re-computing metadata

Inline add_raw_property logic in define_property to reuse pre-computed
metadata. This avoids calling compute_property_metadata twice for Global
and Parent fallback branches, improving efficiency.

Pull Request Pull Request #64: Add scope manipulation: ^ operator and scope attribute

Run Details

224 of 291 new or added lines in 4 files covered. (76.98%)

4501 of 5078 relevant lines covered (88.64%)

177.96 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

90.91

/src/parse/macro_def.rs

use crate::error::XacroError;
use std::collections::{HashMap, HashSet};
pub use xmltree::Element;

/// Default value specification for a macro parameter
#[derive(Debug, Clone, PartialEq)]
pub enum ParamDefault {
    /// No default: "param"
    None,
    /// Regular default: "param:=5" or "param:=${x*2}"
    Value(String),
    /// Forward required: "param:=^"
    /// Must exist in parent scope or error
    ForwardRequired(String),
    /// Forward with default: "param:=^|5" or "param:=^|"
    /// Try parent scope, fall back to default (or empty string if None)
    ForwardWithDefault(String, Option<String>),
}

// Type aliases to simplify complex return types
pub type ParamsMap = HashMap<String, ParamDefault>;
pub type ParamOrder = Vec<String>;
pub type BlockParamsSet = HashSet<String>;
pub type ParsedParams = (ParamsMap, ParamOrder, BlockParamsSet, BlockParamsSet);

pub type MacroArgs = HashMap<String, String>;
pub type MacroBlocks = HashMap<String, Element>;
pub type CollectedArgs = (MacroArgs, MacroBlocks);

#[derive(Debug, Clone)]
pub struct MacroDefinition {
    pub name: String,            // Macro name from 'name' attribute (for error messages)
    pub params: ParamsMap,       // Regular params with optional defaults
    pub param_order: ParamOrder, // Parameter declaration order (critical for block params!)
    pub block_params: BlockParamsSet, // Block params (names without * prefix)
    pub lazy_block_params: BlockParamsSet, // Lazy block params (**param - insert children only)
    pub content: Element,
}

/// Utility functions for parsing and validating macro definitions
pub struct MacroProcessor;

impl MacroProcessor {
    /// Helper to unquote a value (removes surrounding quotes if present)
    fn unquote_value(value: &str) -> &str {
        value
            .strip_prefix('\'')
            .and_then(|s| s.strip_suffix('\''))
            .or_else(|| value.strip_prefix('"').and_then(|s| s.strip_suffix('"')))
            .unwrap_or(value)
    }

    /// Split a parameter string on whitespace, respecting quoted sections.
    ///
    /// Returns an error if quotes are unbalanced (unclosed quote).
    ///
    /// Examples:
    /// - `"a b c"` → `["a", "b", "c"]`
    /// - `"a:='x y' b:=1"` → `["a:='x y'", "b:=1"]`
    /// - `"pos:='0 0 0' *block"` → `["pos:='0 0 0'", "*block"]`
    /// - `"rpy:='0 0 0"` → Error (unclosed quote)
    fn split_params_respecting_quotes(params_str: &str) -> Result<Vec<String>, XacroError> {
        let mut tokens = Vec::new();
        let mut current_token = String::new();
        let mut in_quotes = false;
        let mut quote_char = ' ';

        for ch in params_str.chars() {
            if in_quotes {
                current_token.push(ch);
                if ch == quote_char {
                    in_quotes = false;
                }
            } else if ch == '\'' || ch == '"' {
                // Start of quoted section
                in_quotes = true;
                quote_char = ch;
                current_token.push(ch);
            } else if ch.is_whitespace() {
                // End of token (if not empty)
                if !current_token.is_empty() {
                    // Use mem::take to avoid cloning
                    tokens.push(core::mem::take(&mut current_token));
                }
            } else {
                // Regular character
                current_token.push(ch);
            }
        }

        // Check for unbalanced quotes before returning
        if in_quotes {
            return Err(XacroError::UnbalancedQuote {
                quote_char,
                params_str: params_str.to_string(),
            });
        }

        // Don't forget the last token
        if !current_token.is_empty() {
            tokens.push(current_token);
        }

        Ok(tokens)
    }

    /// Parse macro parameters (strict mode - default)
    pub fn parse_params(params_str: &str) -> Result<ParsedParams, XacroError> {
        Self::parse_params_impl(params_str, false)
    }

    /// Parse macro parameters (compatibility mode - accept duplicates)
    pub fn parse_params_compat(params_str: &str) -> Result<ParsedParams, XacroError> {
        Self::parse_params_impl(params_str, true)
    }

    /// Internal implementation for parameter parsing
    fn parse_params_impl(
        params_str: &str,
        compat_mode: bool,
    ) -> Result<ParsedParams, XacroError> {
        let mut params = HashMap::new();
        let mut param_order = Vec::new();
        let mut block_params = HashSet::new();
        let mut lazy_block_params = HashSet::new();

        for token in Self::split_params_respecting_quotes(params_str)? {
            // Parse token to determine parameter type and components
            let (param_name_str, is_block, is_lazy, param_default) = if token.starts_with('*') {
                // Block parameter (**param or *param)
                // Block parameters CANNOT have defaults
                if token.contains(":=") || token.contains('=') {
                    return Err(XacroError::BlockParameterWithDefault {
                        param: token.clone(),
                    });
                }

                // Check for lazy block (**param) vs regular block (*param)
                let (stripped, is_lazy) = if let Some(s) = token.strip_prefix("**") {
                    // Lazy block parameter (**param - inserts children only)
                    (s, true)
                } else if let Some(s) = token.strip_prefix('*') {
                    // Regular block parameter (*param - inserts element itself)
                    (s, false)
                } else {
                    unreachable!("starts_with('*') check guarantees this branch is unreachable");
                };

                // Validate no extra asterisks (reject ***param, ****param, etc.)
                if stripped.starts_with('*') {
                    return Err(XacroError::InvalidParameterName {
                        param: token.clone(),
                    });
                }

                (stripped.to_string(), true, is_lazy, ParamDefault::None)
            } else if let Some((name, value)) =
                token.split_once(":=").or_else(|| token.split_once('='))
            {
                // Regular parameter with default value (supports := or =)
                // Python xacro supports both syntaxes:
                //   params="width:=5"  (preferred)
                //   params="width=5"   (also valid)

                // Check for ^ operator (parent scope forwarding)
                let param_default = if let Some(remainder) = value.strip_prefix('^') {
                    if let Some(default_str) = remainder.strip_prefix('|') {
                        // ^|default syntax - forward with default
                        let unquoted = Self::unquote_value(default_str).to_string();
                        if unquoted.is_empty() {
                            ParamDefault::ForwardWithDefault(name.to_string(), None)
                        } else {
                            ParamDefault::ForwardWithDefault(name.to_string(), Some(unquoted))
                        }
                    } else if remainder.is_empty() {
                        // ^ syntax - required forward
                        ParamDefault::ForwardRequired(name.to_string())
                    } else {
                        // Invalid: ^something (not ^| or plain ^)
                        return Err(XacroError::InvalidForwardSyntax {
                            param: token.clone(),
                            hint: "Use ^ for required forward or ^|default for optional"
                                .to_string(),
                        });
                    }
                } else {
                    // Regular default value
                    ParamDefault::Value(Self::unquote_value(value).to_string())
                };

                (name.to_string(), false, false, param_default)
            } else {
                // Regular parameter without default
                (token.clone(), false, false, ParamDefault::None)
            };

            // Validate parameter name is not empty
            if param_name_str.is_empty() {
                return Err(XacroError::InvalidParameterName { param: token });
            }

            let param_name = param_name_str;

            // Detect duplicate declarations (strict mode only)
            if params.contains_key(&param_name) && !compat_mode {
                return Err(XacroError::DuplicateParamDeclaration { param: param_name });
            }
            // In compat mode, silently overwrite (last declaration wins)

            // Insert into appropriate data structures
            // Only add to param_order if not already present (handles compat mode duplicates)
            if !params.contains_key(&param_name) {
                param_order.push(param_name.clone());
            }
            if is_block {
                block_params.insert(param_name.clone());
                if is_lazy {
                    lazy_block_params.insert(param_name.clone());
                } else {
                    // Regular block, remove from lazy set if previously there
                    lazy_block_params.remove(&param_name);
                }
                params.insert(param_name, ParamDefault::None);
            } else {
                // In compat mode, if changing from block to non-block, remove from block_params and lazy_block_params
                if compat_mode {
                    block_params.remove(&param_name);
                    lazy_block_params.remove(&param_name);
                }
                params.insert(param_name, param_default);
            }
        }

        Ok((params, param_order, block_params, lazy_block_params))
    }

    pub fn collect_macro_args(
        element: &Element,
        macro_def: &MacroDefinition,
    ) -> Result<CollectedArgs, XacroError> {
        let mut param_values = HashMap::new();
        let mut block_values = HashMap::new();

        // Extract regular parameters from attributes
        // Reject namespaced attributes - macro parameters must be local names only
        for (name, value) in &element.attributes {
            let local_name = &name.local_name;

            // Reject namespaced attributes on macro calls (Python xacro behavior)
            if let Some(prefix) = &name.prefix {
                return Err(XacroError::InvalidMacroParameter {
                    param: format!("{}:{}", prefix, name.local_name),
                    reason: "Macro parameters cannot have namespace prefixes".to_string(),
                });
            }

            if macro_def.block_params.contains(local_name) {
                // Block parameters cannot be specified as attributes
                return Err(XacroError::BlockParameterAttributeCollision {
                    param: local_name.clone(),
                });
            }
            param_values.insert(local_name.clone(), value.clone());
        }

        // Extract block parameters from child elements IN ORDER
        // Use iterator to avoid double-cloning (Vec allocation + insertion)
        let mut children_iter = element
            .children
            .iter()
            .filter_map(xmltree::XMLNode::as_element);

        log::debug!(
            "collect_macro_args: macro '{}' has {} block params, {} lazy",
            macro_def.name,
            macro_def.block_params.len(),
            macro_def.lazy_block_params.len()
        );
        log::debug!(
            "collect_macro_args: macro call has {} child elements",
            element
                .children
                .iter()
                .filter_map(|n| n.as_element())
                .count()
        );

        // Iterate through params in order they were declared
        // Block params consume child elements sequentially from the iterator
        for param_name in &macro_def.param_order {
            if macro_def.block_params.contains(param_name) {
                let child_element =
                    children_iter
                        .next()
                        .ok_or_else(|| XacroError::MissingBlockParameter {
                            macro_name: macro_def.name.clone(),
                            param: param_name.clone(),
                        })?;
                log::debug!(
                    "collect_macro_args: captured block param '{}' <- element '<{}>...'",
                    param_name,
                    child_element.name
                );
                block_values.insert(param_name.clone(), child_element.clone());
            }
        }

        // Error if extra children provided
        if children_iter.next().is_some() {
            let extra_count = 1 + children_iter.count();
            return Err(XacroError::UnusedBlock {
                macro_name: macro_def.name.clone(),
                extra_count,
            });
        }

        Ok((param_values, block_values))
    }
}

1	use crate::error::XacroError;
2	use std::collections::{HashMap, HashSet};
3	pub use xmltree::Element;
4
5	/// Default value specification for a macro parameter
6	#[derive(Debug, Clone, PartialEq)]
7	pub enum ParamDefault {
8	/// No default: "param"
9	None,
10	/// Regular default: "param:=5" or "param:=${x*2}"
11	Value(String),
12	/// Forward required: "param:=^"
13	/// Must exist in parent scope or error
14	ForwardRequired(String),
15	/// Forward with default: "param:=^\|5" or "param:=^\|"
16	/// Try parent scope, fall back to default (or empty string if None)
17	ForwardWithDefault(String, Option<String>),
18	}
19
20	// Type aliases to simplify complex return types
21	pub type ParamsMap = HashMap<String, ParamDefault>;
22	pub type ParamOrder = Vec<String>;
23	pub type BlockParamsSet = HashSet<String>;
24	pub type ParsedParams = (ParamsMap, ParamOrder, BlockParamsSet, BlockParamsSet);
25
26	pub type MacroArgs = HashMap<String, String>;
27	pub type MacroBlocks = HashMap<String, Element>;
28	pub type CollectedArgs = (MacroArgs, MacroBlocks);
29
30	#[derive(Debug, Clone)]
31	pub struct MacroDefinition {
32	pub name: String, // Macro name from 'name' attribute (for error messages)
33	pub params: ParamsMap, // Regular params with optional defaults
34	pub param_order: ParamOrder, // Parameter declaration order (critical for block params!)
35	pub block_params: BlockParamsSet, // Block params (names without * prefix)
36	pub lazy_block_params: BlockParamsSet, // Lazy block params (**param - insert children only)
37	pub content: Element,
38	}
39
40	/// Utility functions for parsing and validating macro definitions
41	pub struct MacroProcessor;
42
43	impl MacroProcessor {
44	/// Helper to unquote a value (removes surrounding quotes if present)
45	fn unquote_value(value: &str) -> &str {	88✔
46	value	88✔
47	.strip_prefix('\'')	88✔
48	.and_then(\|s\| s.strip_suffix('\''))	88✔
49	.or_else(\|\| value.strip_prefix('"').and_then(\|s\| s.strip_suffix('"')))	88✔
50	.unwrap_or(value)	88✔
51	}	88✔
52
53	/// Split a parameter string on whitespace, respecting quoted sections.
54	///
55	/// Returns an error if quotes are unbalanced (unclosed quote).
56	///
57	/// Examples:
58	/// - `"a b c"` → `["a", "b", "c"]`
59	/// - `"a:='x y' b:=1"` → `["a:='x y'", "b:=1"]`
60	/// - `"pos:='0 0 0' block"` → `["pos:='0 0 0'", "block"]`
61	/// - `"rpy:='0 0 0"` → Error (unclosed quote)
62	fn split_params_respecting_quotes(params_str: &str) -> Result<Vec<String>, XacroError> {	205✔
63	let mut tokens = Vec::new();	205✔
64	let mut current_token = String::new();	205✔
65	let mut in_quotes = false;	205✔
66	let mut quote_char = ' ';	205✔
67
68	for ch in params_str.chars() {	2,070✔
69	if in_quotes {	2,070✔
70	current_token.push(ch);	144✔
71	if ch == quote_char {	144✔
72	in_quotes = false;	23✔
73	}	121✔
74	} else if ch == '\'' \|\| ch == '"' {	1,926✔
75	// Start of quoted section	25✔
76	in_quotes = true;	25✔
77	quote_char = ch;	25✔
78	current_token.push(ch);	25✔
79	} else if ch.is_whitespace() {	1,901✔
80	// End of token (if not empty)
81	if !current_token.is_empty() {	130✔
82	// Use mem::take to avoid cloning	96✔
83	tokens.push(core::mem::take(&mut current_token));	96✔
84	}	96✔
85	} else {	1,771✔
86	// Regular character	1,771✔
87	current_token.push(ch);	1,771✔
88	}	1,771✔
89	}
90
91	// Check for unbalanced quotes before returning
92	if in_quotes {	205✔
93	return Err(XacroError::UnbalancedQuote {	2✔
94	quote_char,	2✔
95	params_str: params_str.to_string(),	2✔
96	});	2✔
97	}	203✔
98
99	// Don't forget the last token
100	if !current_token.is_empty() {	203✔
101	tokens.push(current_token);	163✔
102	}	163✔
103
104	Ok(tokens)	203✔
105	}	205✔
106
107	/// Parse macro parameters (strict mode - default)
108	pub fn parse_params(params_str: &str) -> Result<ParsedParams, XacroError> {	195✔
109	Self::parse_params_impl(params_str, false)	195✔
110	}	195✔
111
112	/// Parse macro parameters (compatibility mode - accept duplicates)
113	pub fn parse_params_compat(params_str: &str) -> Result<ParsedParams, XacroError> {	10✔
114	Self::parse_params_impl(params_str, true)	10✔
115	}	10✔
116
117	/// Internal implementation for parameter parsing
118	fn parse_params_impl(	205✔
119	params_str: &str,	205✔
120	compat_mode: bool,	205✔
121	) -> Result<ParsedParams, XacroError> {	205✔
122	let mut params = HashMap::new();	205✔
123	let mut param_order = Vec::new();	205✔
124	let mut block_params = HashSet::new();	205✔
125	let mut lazy_block_params = HashSet::new();	205✔
126
127	for token in Self::split_params_respecting_quotes(params_str)? {	256✔
128	// Parse token to determine parameter type and components
129	let (param_name_str, is_block, is_lazy, param_default) = if token.starts_with('*') {	256✔
130	// Block parameter (*param or param)
131	// Block parameters CANNOT have defaults
132	if token.contains(":=") \|\| token.contains('=') {	60✔
133	return Err(XacroError::BlockParameterWithDefault {	4✔
134	param: token.clone(),	4✔
135	});	4✔
136	}	56✔
137
138	// Check for lazy block (*param) vs regular block (param)
139	let (stripped, is_lazy) = if let Some(s) = token.strip_prefix("**") {	56✔
140	// Lazy block parameter (**param - inserts children only)
141	(s, true)	8✔
142	} else if let Some(s) = token.strip_prefix('*') {	48✔
143	// Regular block parameter (*param - inserts element itself)
144	(s, false)	48✔
145	} else {
146	unreachable!("starts_with('*') check guarantees this branch is unreachable");	×
147	};
148
149	// Validate no extra asterisks (reject *param, **param, etc.)
150	if stripped.starts_with('*') {	56✔
151	return Err(XacroError::InvalidParameterName {	1✔
152	param: token.clone(),	1✔
153	});	1✔
154	}	55✔
155
156	(stripped.to_string(), true, is_lazy, ParamDefault::None)	55✔
157	} else if let Some((name, value)) =	115✔
158	token.split_once(":=").or_else(\|\| token.split_once('='))	196✔
159	{
160	// Regular parameter with default value (supports := or =)
161	// Python xacro supports both syntaxes:
162	// params="width:=5" (preferred)
163	// params="width=5" (also valid)
164
165	// Check for ^ operator (parent scope forwarding)
166	let param_default = if let Some(remainder) = value.strip_prefix('^') {	115✔
167	if let Some(default_str) = remainder.strip_prefix('\|') {	37✔
168	// ^\|default syntax - forward with default
169	let unquoted = Self::unquote_value(default_str).to_string();	10✔
170	if unquoted.is_empty() {	10✔
171	ParamDefault::ForwardWithDefault(name.to_string(), None)	1✔
172	} else {
173	ParamDefault::ForwardWithDefault(name.to_string(), Some(unquoted))	9✔
174	}
175	} else if remainder.is_empty() {	27✔
176	// ^ syntax - required forward
177	ParamDefault::ForwardRequired(name.to_string())	27✔
178	} else {
179	// Invalid: ^something (not ^\| or plain ^)
NEW 180	return Err(XacroError::InvalidForwardSyntax {	×
NEW 181	param: token.clone(),	×
NEW 182	hint: "Use ^ for required forward or ^\|default for optional"	×
NEW 183	.to_string(),	×
NEW 184	});	×
185	}
186	} else {
187	// Regular default value
188	ParamDefault::Value(Self::unquote_value(value).to_string())	78✔
189	};
190
191	(name.to_string(), false, false, param_default)	115✔
192	} else {
193	// Regular parameter without default
194	(token.clone(), false, false, ParamDefault::None)	81✔
195	};
196
197	// Validate parameter name is not empty
198	if param_name_str.is_empty() {	251✔
199	return Err(XacroError::InvalidParameterName { param: token });	2✔
200	}	249✔
201
202	let param_name = param_name_str;	249✔
203
204	// Detect duplicate declarations (strict mode only)
205	if params.contains_key(&param_name) && !compat_mode {	249✔
206	return Err(XacroError::DuplicateParamDeclaration { param: param_name });	9✔
207	}	240✔
208	// In compat mode, silently overwrite (last declaration wins)
209
210	// Insert into appropriate data structures
211	// Only add to param_order if not already present (handles compat mode duplicates)
212	if !params.contains_key(&param_name) {	240✔
213	param_order.push(param_name.clone());	228✔
214	}	228✔
215	if is_block {	240✔
216	block_params.insert(param_name.clone());	52✔
217	if is_lazy {	52✔
218	lazy_block_params.insert(param_name.clone());	7✔
219	} else {	45✔
220	// Regular block, remove from lazy set if previously there	45✔
221	lazy_block_params.remove(&param_name);	45✔
222	}	45✔
223	params.insert(param_name, ParamDefault::None);	52✔
224	} else {
225	// In compat mode, if changing from block to non-block, remove from block_params and lazy_block_params
226	if compat_mode {	188✔
227	block_params.remove(&param_name);	21✔
228	lazy_block_params.remove(&param_name);	21✔
229	}	167✔
230	params.insert(param_name, param_default);	188✔
231	}
232	}
233
234	Ok((params, param_order, block_params, lazy_block_params))	187✔
235	}	205✔
236
237	pub fn collect_macro_args(	168✔
238	element: &Element,	168✔
239	macro_def: &MacroDefinition,	168✔
240	) -> Result<CollectedArgs, XacroError> {	168✔
241	let mut param_values = HashMap::new();	168✔
242	let mut block_values = HashMap::new();	168✔
243
244	// Extract regular parameters from attributes
245	// Reject namespaced attributes - macro parameters must be local names only
246	for (name, value) in &element.attributes {	252✔
247	let local_name = &name.local_name;	86✔
248
249	// Reject namespaced attributes on macro calls (Python xacro behavior)
250	if let Some(prefix) = &name.prefix {	86✔
251	return Err(XacroError::InvalidMacroParameter {	1✔
252	param: format!("{}:{}", prefix, name.local_name),	1✔
253	reason: "Macro parameters cannot have namespace prefixes".to_string(),	1✔
254	});	1✔
255	}	85✔
256
257	if macro_def.block_params.contains(local_name) {	85✔
258	// Block parameters cannot be specified as attributes
259	return Err(XacroError::BlockParameterAttributeCollision {	1✔
260	param: local_name.clone(),	1✔
261	});	1✔
262	}	84✔
263	param_values.insert(local_name.clone(), value.clone());	84✔
264	}
265
266	// Extract block parameters from child elements IN ORDER
267	// Use iterator to avoid double-cloning (Vec allocation + insertion)
268	let mut children_iter = element	166✔
269	.children	166✔
270	.iter()	166✔
271	.filter_map(xmltree::XMLNode::as_element);	166✔
272
273	log::debug!(	166✔
274	"collect_macro_args: macro '{}' has {} block params, {} lazy",	×
275	macro_def.name,
276	macro_def.block_params.len(),	×
277	macro_def.lazy_block_params.len()	×
278	);
279	log::debug!(	166✔
280	"collect_macro_args: macro call has {} child elements",	×
281	element	×
282	.children	×
283	.iter()	×
284	.filter_map(\|n\| n.as_element())	×
285	.count()	×
286	);
287
288	// Iterate through params in order they were declared
289	// Block params consume child elements sequentially from the iterator
290	for param_name in &macro_def.param_order {	345✔
291	if macro_def.block_params.contains(param_name) {	181✔
292	let child_element =	36✔
293	children_iter	38✔
294	.next()	38✔
295	.ok_or_else(\|\| XacroError::MissingBlockParameter {	38✔
296	macro_name: macro_def.name.clone(),	2✔
297	param: param_name.clone(),	2✔
298	})?;	2✔
299	log::debug!(	36✔
300	"collect_macro_args: captured block param '{}' <- element '<{}>...'",	×
301	param_name,
302	child_element.name
303	);
304	block_values.insert(param_name.clone(), child_element.clone());	36✔
305	}	143✔
306	}
307
308	// Error if extra children provided
309	if children_iter.next().is_some() {	164✔
310	let extra_count = 1 + children_iter.count();	2✔
311	return Err(XacroError::UnusedBlock {	2✔
312	macro_name: macro_def.name.clone(),	2✔
313	extra_count,	2✔
314	});	2✔
315	}	162✔
316
317	Ok((param_values, block_values))	162✔
318	}	168✔
319	}

kaidokert / xacro / 21054809427

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous