Fix iterator macro parameter variable interpretation (#47)

* Fix: Function parameters with iterator macros are not CLI args

Co-authored-by: blakeinvictoria <blakeinvictoria@gmail.com>

* Bump version to 0.5.3 and fix CLI argument parsing

Co-authored-by: blakeinvictoria <blakeinvictoria@gmail.com>

* Fix: Remove unused CLI args from macro test

Co-authored-by: blakeinvictoria <blakeinvictoria@gmail.com>

* Refactor: Improve positional parameter analysis and macro handling

Add exclude_function_params to parse_positional_usages to prevent false positives.
Update analyzer orders and add variable type extraction for macro processing.

Co-authored-by: blakeinvictoria <blakeinvictoria@gmail.com>

* Refactor: Adjust analyzer order for environment and positional variables

Co-authored-by: blakeinvictoria <blakeinvictoria@gmail.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>

Author: BlakeASmith

CHANGELOG.md

@@ -13,7 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## [0.5.3] - 2025-01-28
+
 ### Fixed
+- Fixed function parameters being incorrectly treated as CLI arguments when used with iterator macros
+  - Function parameters (`$1`, `$2`, etc.) within functions that have iterator macros are now properly excluded from CLI argument generation
+  - The macro-generated loop provides these parameters instead of requiring them as CLI arguments
+  - This fixes the bug where scripts with iterator macros would incorrectly require `--1`, `--2`, etc. arguments
 - Fixed default value parsing in Google-style annotations
   - Empty default values (e.g., `Default:` or `Default: `) are now properly handled as no default
   - Descriptions ending with periods (e.g., `# VAR (str): Description.`) are now correctly parsed

pyproject.toml

@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "argorator"
-version = "0.5.2"
+version = "0.5.3"
 description = "CLI to wrap shell scripts and expose variables/positionals as argparse options"
 readme = "README.md"
 requires-python = ">=3.9"

src/argorator/analyzers.py

@@ -109,9 +109,13 @@ def parse_variable_usages(script_text: str) -> Set[str]:
     return {name for name in candidates if name and name not in SPECIAL_VARS}
 
 
-def parse_positional_usages(script_text: str) -> Tuple[Set[int], bool]:
+def parse_positional_usages(script_text: str, exclude_function_params: Optional[Set[str]] = None) -> Tuple[Set[int], bool]:
     """Extract positional parameter indices and varargs usage from script.
     
+    Args:
+        script_text: The script content
+        exclude_function_params: Set of function parameter indices to exclude (e.g., {'1', '2'})
+    
     Returns:
         Tuple of (positional_indices, varargs_present)
     """
@@ -121,9 +125,34 @@ def parse_positional_usages(script_text: str) -> Tuple[Set[int], bool]:
     indices = {int(m) for m in digit_pattern.findall(script_text)}
     varargs = bool(varargs_pattern.search(script_text))
 
+    # Exclude function parameters that are used with iterator macros
+    if exclude_function_params:
+        indices = {idx for idx in indices if str(idx) not in exclude_function_params}
+    
     return indices, varargs
 
 
+def _extract_function_parameters(script_text: str, function_target) -> Set[str]:
+    """Extract function parameter variables used within a function that has an iteration macro.
+    
+    Args:
+        script_text: The full script content
+        function_target: The MacroTarget representing the function
+        
+    Returns:
+        Set of function parameter variable names (e.g., {'1', '2', '3'})
+    """
+    lines = script_text.split('\n')
+    function_lines = lines[function_target.start_line:function_target.end_line + 1]
+    function_content = '\n'.join(function_lines)
+    
+    # Find all positional parameter usages within the function
+    digit_pattern = re.compile(r"\$([1-9][0-9]*)")
+    param_indices = {m for m in digit_pattern.findall(function_content)}
+    
+    return param_indices
+
+
 @analyzer(order=20)
 def analyze_variable_usages(context: AnalysisContext) -> None:
     """Find all variables referenced in the script."""
@@ -142,15 +171,28 @@ def analyze_loop_variables(context: AnalysisContext) -> None:
     context.loop_vars = parse_loop_variables(context.script_text)
 
 
-@analyzer(order=21.5)
+@analyzer(order=45)
 def identify_macro_iterator_variables(context: AnalysisContext) -> None:
     """Identify iterator variables from iteration macros to exclude from undefined variables."""
     try:
         from .macros.parser import macro_parser
+        from .macros.processor import macro_processor
+        
+        # Extract variable types from annotations and pass to macro processor
+        variable_types = {}
+        
+        # Get types from argument annotations (if available)
+        if context.annotations:
+            for var_name, annotation in context.annotations.items():
+                variable_types[var_name] = annotation.type
+        
+        # Set variable types in macro processor
+        macro_processor.set_variable_types(variable_types)
 
         # Find all iteration macro comments
         macro_comments = macro_parser.find_macro_comments(context.script_text)
         iterator_vars = set()
+        function_param_vars = set()
 
         for comment in macro_comments:
             if comment.macro_type == 'iteration':
@@ -161,30 +203,39 @@ def identify_macro_iterator_variables(context: AnalysisContext) -> None:
                     if target:
                         iteration_macro = macro_parser.parse_iteration_macro(comment, target)
                         iterator_vars.add(iteration_macro.iterator_var)
+                        
+                        # If this macro targets a function, identify function parameters used within that function
+                        if target.target_type == 'function':
+                            function_params = _extract_function_parameters(context.script_text, target)
+                            function_param_vars.update(function_params)
+                            
                 except Exception:
                     # If parsing fails, continue with other macros
                     pass
 
-        # Store iterator variables in temp_data for use in next analyzer
+        # Store iterator variables and function parameter variables in temp_data for use in next analyzer
         context.temp_data['macro_iterator_vars'] = iterator_vars
+        context.temp_data['macro_function_param_vars'] = function_param_vars
 
     except ImportError:
         # If macro modules aren't available, skip this step
         context.temp_data['macro_iterator_vars'] = set()
+        context.temp_data['macro_function_param_vars'] = set()
 
 
-@analyzer(order=22)
+@analyzer(order=46)
 def analyze_undefined_variables(context: AnalysisContext) -> None:
     """Identify variables that are used but not defined in the script."""
-    # Get iterator variables identified by macro analysis
+    # Get iterator variables and function parameter variables identified by macro analysis
     macro_iterator_vars = context.temp_data.get('macro_iterator_vars', set())
+    macro_function_param_vars = context.temp_data.get('macro_function_param_vars', set())
 
-    # Exclude iterator variables and loop variables from undefined variables
-    undefined_vars = context.all_used_vars - context.defined_vars - macro_iterator_vars - context.loop_vars
+    # Exclude iterator variables, function parameter variables, and loop variables from undefined variables
+    undefined_vars = context.all_used_vars - context.defined_vars - macro_iterator_vars - macro_function_param_vars - context.loop_vars
     context.undefined_vars = {name: None for name in sorted(undefined_vars)}
 
 
-@analyzer(order=23)
+@analyzer(order=47)
 def analyze_environment_variables(context: AnalysisContext) -> None:
     """Separate undefined variables into those with environment defaults and truly undefined."""
     env_vars: Dict[str, str] = {}
@@ -200,10 +251,13 @@ def analyze_environment_variables(context: AnalysisContext) -> None:
     context.undefined_vars = remaining_undefined
 
 
-@analyzer(order=30)
+@analyzer(order=49)
 def analyze_positional_parameters(context: AnalysisContext) -> None:
     """Detect positional parameter usage and varargs references in the script."""
-    indices, varargs = parse_positional_usages(context.script_text)
+    # Get function parameter variables that are used with iterator macros
+    macro_function_param_vars = context.temp_data.get('macro_function_param_vars', set())
+    
+    indices, varargs = parse_positional_usages(context.script_text, exclude_function_params=macro_function_param_vars)
     context.positional_indices = indices
     context.varargs = varargs
 

src/argorator/contexts.py

@@ -150,7 +150,8 @@ def create_transform_context(analysis: AnalysisContext) -> TransformContext:
         varargs=analysis.varargs,
         annotations=analysis.annotations,
         script_metadata=analysis.script_metadata,
-        script_path=analysis.script_path
+        script_path=analysis.script_path,
+        temp_data=analysis.temp_data.copy()
     )
 
 

src/argorator/transformers.py

@@ -98,9 +98,13 @@ def add_positional_arguments(context: TransformContext) -> None:
     if not context.argument_parser:
         raise ValueError("Base parser must be created first")
 
-    # Add numbered positional arguments
+    # Get function parameter variables that are used with iterator macros
+    macro_function_param_vars = context.temp_data.get('macro_function_param_vars', set())
+    
+    # Add numbered positional arguments, excluding those used with iterator macros
     for index in sorted(context.positional_indices):
-        context.argument_parser.add_argument(f"ARG{index}")
+        if str(index) not in macro_function_param_vars:
+            context.argument_parser.add_argument(f"ARG{index}")
 
     # Add varargs if needed
     if context.varargs:

tests/test_bug_fix_function_parameters.py

@@ -0,0 +1,125 @@
+"""Tests for the bug fix: function parameters with iterator macros should not require CLI arguments."""
+
+import pytest
+from argorator.macros.processor import macro_processor
+from argorator.macros.parser import macro_parser
+
+
+class TestFunctionParameterBugFix:
+    """Test that function parameters used with iterator macros don't require CLI arguments."""
+    
+    def test_function_parameters_with_iterator_macro_excluded_from_cli(self):
+        """Test that function parameters are excluded from CLI arguments when used with iterator macros."""
+        script = '''#!/usr/bin/env argorator
+# DATA (str): Input data for processing
+
+# for item in $DATA sep ,
+process_item() {
+    echo "Processing item: $1"
+    echo "Additional info: $2"
+}
+'''
+        
+        # This should work without requiring --1 or --2 arguments
+        # The macro will provide the parameters to the function
+        result = macro_processor.process_macros(script)
+        
+        # Verify that the macro generates the correct loop
+        assert 'for item in' in result
+        assert 'process_item "$item"' in result
+        assert 'done' in result
+    
+    def test_function_parameters_without_iterator_macro_require_cli(self):
+        """Test that function parameters still require CLI arguments when NOT used with iterator macros."""
+        script = '''#!/usr/bin/env argorator
+# DATA (str): Input data for processing
+
+process_item() {
+    echo "Processing item: $1"
+    echo "Additional info: $2"
+}
+
+# Call the function manually - this should require CLI arguments
+process_item "test" "info"
+'''
+        
+        # This should NOT work without providing --1 and --2 arguments
+        # because there's no iterator macro to provide the parameters
+        result = macro_processor.process_macros(script)
+        
+        # The script should remain unchanged (no macros to process)
+        assert result.strip() == script.strip()
+    
+    def test_mixed_scenario_function_with_and_without_macro(self):
+        """Test a scenario with both functions that have macros and functions that don't."""
+        script = '''#!/usr/bin/env argorator
+# DATA (str): Input data for processing
+# OUTPUT_DIR (str): Output directory
+
+# for item in $DATA sep ,
+process_item() {
+    echo "Processing item: $1"
+    echo "Additional info: $2"
+}
+
+# This function has no macro, so it should require CLI arguments
+process_other() {
+    echo "Processing other: $1"
+    echo "More info: $2"
+}
+
+# Call the function without macro manually
+process_other "test" "info"
+'''
+        
+        result = macro_processor.process_macros(script)
+        
+        # Verify that only the function with the macro gets processed
+        assert 'for item in' in result
+        assert 'process_item "$item"' in result
+        assert 'process_other "test" "info"' in result  # This should remain unchanged
+    
+    def test_function_with_additional_parameters_and_macro(self):
+        """Test function with additional parameters passed by the macro."""
+        script = '''#!/usr/bin/env argorator
+# DATA (str): Input data for processing
+# OUTPUT_DIR (str): Output directory
+
+# for item in $DATA sep , | with $OUTPUT_DIR
+process_item() {
+    local input="$1"
+    local output_dir="$2"
+    echo "Processing $input to $output_dir"
+}
+'''
+        
+        result = macro_processor.process_macros(script)
+        
+        # Verify that the macro generates the correct loop with additional parameters
+        assert 'for item in' in result
+        assert 'process_item "$item" "$OUTPUT_DIR"' in result
+        assert 'done' in result
+    
+    def test_nested_function_parameters_with_macro(self):
+        """Test that nested function calls with parameters work correctly with macros."""
+        script = '''#!/usr/bin/env argorator
+# DATA (str): Input data for processing
+
+# for item in $DATA sep ,
+process_item() {
+    echo "Processing item: $1"
+    helper_function "$1" "extra"
+}
+
+helper_function() {
+    echo "Helper processing: $1 with $2"
+}
+'''
+        
+        result = macro_processor.process_macros(script)
+        
+        # Verify that the macro generates the correct loop
+        assert 'for item in' in result
+        assert 'process_item "$item"' in result
+        assert 'helper_function "$1" "extra"' in result  # This should remain unchanged
+        assert 'done' in result

tests/test_integration_macros.py

@@ -188,7 +188,8 @@ def test_function_macros_with_separators(self):
 }
 '''
 
-        output = self._run_argorator(script, ['--data', "a'b'c", 'dummy1', 'dummy2'])
+        # Function parameters $1 and $2 are provided by the macro, not CLI arguments
+        output = self._run_argorator(script, ['--data', "a'b'c"])
         assert 'Processing: a with prefix' in output
         assert 'Processing: b with prefix' in output
         assert 'Processing: c with prefix' in output