diff --git a/CLAUDE.md b/CLAUDE.md index 224eea2..8e7040e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,6 +47,9 @@ parameters: - name: param1 description: Description of parameter example: "A1:B10" + - name: param2 + description: Another parameter + example: '""' formula: | LET( variable, expression, @@ -138,7 +141,9 @@ Formulas can call other named functions - the system automatically: **Key rules enforced:** - Formulas must not start with `=` (added during generation) +- All parameters must have non-empty example values - Warns about self-executing LAMBDA patterns (can often be simplified) +- Formulas must be syntactically valid per pyparsing grammar **Extensible**: Add new rules by subclassing `LintRule` and registering in the linter. @@ -148,7 +153,7 @@ Formulas can call other named functions - the system automatically: 1. Test in Google Sheets first before adding to repository 2. Use composition - call existing named functions instead of duplicating logic 3. Follow the 4-step workflow (lint → generate → test → commit both) -4. Provide realistic parameter examples in YAML +4. Provide non-empty example values for all parameters (linter enforces this; e.g., `"A1:B10"`, `'""'`, `BLANK()`, `0`) ### Parser Improvements 1. Run parser tests first: `uv run pytest tests/test_formula_parser.py -v` diff --git a/README.md b/README.md index 9196d34..66b3cb3 100644 --- a/README.md +++ b/README.md @@ -1544,6 +1544,12 @@ A1:B10 Value to use in place of non-errors ``` +**Example:** + +``` +"" +``` +
diff --git a/formulas/nonerrorsto.yaml b/formulas/nonerrorsto.yaml index 03184bf..0c461e4 100644 --- a/formulas/nonerrorsto.yaml +++ b/formulas/nonerrorsto.yaml @@ -14,7 +14,7 @@ parameters: - name: replacement description: Value to use in place of non-errors - example: "" + example: '""' formula: | MAP(input, LAMBDA(v, IF(ISERROR(v), v, replacement))) diff --git a/scripts/lint_formulas.py b/scripts/lint_formulas.py index 2d6a685..780ee0c 100644 --- a/scripts/lint_formulas.py +++ b/scripts/lint_formulas.py @@ -152,6 +152,62 @@ def check(self, file_path: Path, data: Dict[str, Any]) -> Tuple[List[str], List[ return errors, warnings +class RequireParameterExamplesRule(LintRule): + """Rule: All parameters must have non-empty example values.""" + + def __init__(self): + super().__init__( + name="require-parameter-examples", + description="All parameters must have non-empty example values" + ) + + def check(self, file_path: Path, data: Dict[str, Any]) -> Tuple[List[str], List[str]]: + """ + Check that all parameters have non-empty examples. + + Args: + file_path: Path to the YAML file + data: Parsed YAML data + + Returns: + Tuple of (errors, warnings) + """ + errors = [] + warnings = [] + + # Skip if parameters field is missing + if 'parameters' not in data: + return errors, warnings + + parameters = data['parameters'] + if not isinstance(parameters, list): + return errors, warnings + + # Check each parameter for example field + for i, param in enumerate(parameters): + if not isinstance(param, dict): + continue + + param_name = param.get('name', f'parameter-{i}') + + # Check if example field exists + if 'example' not in param: + errors.append( + f"{file_path}: Parameter '{param_name}' is missing 'example' field. " + f"Provide a concrete example value (e.g., '\"A1:B10\"', '0', 'BLANK()', etc.)" + ) + else: + # Check if example is empty string + example = param.get('example') + if isinstance(example, str) and example == '': + errors.append( + f"{file_path}: Parameter '{param_name}' has empty example. " + f"Provide a concrete example value (e.g., '\"A1:B10\"', '0', '\"\"', 'BLANK()', etc.)" + ) + + return errors, warnings + + class ValidFormulaSyntaxRule(LintRule): """Rule: Formula must be parseable by the pyparsing grammar.""" @@ -219,6 +275,7 @@ def __init__(self): self.rules: List[LintRule] = [ NoLeadingEqualsRule(), NoTopLevelLambdaRule(), + RequireParameterExamplesRule(), ValidFormulaSyntaxRule(), # Add more rules here as needed ] diff --git a/tests/CLAUDE.md b/tests/CLAUDE.md index ad8c81e..8a3c460 100644 --- a/tests/CLAUDE.md +++ b/tests/CLAUDE.md @@ -91,9 +91,10 @@ Based on issues #95 and #96, future work should add tests for: - Multiple calls to same function 3. **Linter rules**: - - Test each linter rule individually + - Test each linter rule individually (see `test_linter.py` for comprehensive examples) - Test that invalid formulas are caught - Test that valid formulas pass + - Parameter examples rule (`require-parameter-examples`): All parameters must have non-empty `example` fields; test edge cases like zero (`0`), falsy values, quoted strings (`'""'`), and function calls (`BLANK()`) ## Dependencies diff --git a/tests/test_linter.py b/tests/test_linter.py index dfb9102..a305509 100644 --- a/tests/test_linter.py +++ b/tests/test_linter.py @@ -23,6 +23,7 @@ from lint_formulas import ( NoLeadingEqualsRule, NoTopLevelLambdaRule, + RequireParameterExamplesRule, FormulaLinter, ) @@ -217,6 +218,208 @@ def test_self_executing_lambda_with_whitespace_warns(self): assert len(warnings) == 1 +class TestRequireParameterExamplesRule: + """Test the RequireParameterExamplesRule for parameter example validation.""" + + def setup_method(self): + """Initialize rule before each test.""" + self.rule = RequireParameterExamplesRule() + + def test_parameter_with_non_empty_example_passes(self): + """Test that parameter with non-empty example passes.""" + data = { + 'parameters': [ + { + 'name': 'input', + 'description': 'Test parameter', + 'example': 'A1:B10' + } + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + assert len(warnings) == 0 + + def test_parameter_with_empty_example_fails(self): + """Test that parameter with empty example produces error.""" + data = { + 'parameters': [ + { + 'name': 'replacement', + 'description': 'Replacement value', + 'example': '' + } + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 1 + assert 'empty example' in errors[0].lower() + assert 'replacement' in errors[0].lower() + assert len(warnings) == 0 + + def test_parameter_missing_example_fails(self): + """Test that parameter without example field produces error.""" + data = { + 'parameters': [ + { + 'name': 'input', + 'description': 'Missing example' + } + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 1 + assert 'missing' in errors[0].lower() + assert 'example' in errors[0].lower() + assert 'input' in errors[0].lower() + assert len(warnings) == 0 + + def test_multiple_parameters_all_with_examples_passes(self): + """Test that multiple parameters with examples pass.""" + data = { + 'parameters': [ + {'name': 'range', 'description': 'Data range', 'example': 'A1:Z100'}, + {'name': 'mode', 'description': 'Mode', 'example': '"rows-any"'}, + {'name': 'func', 'description': 'Callback', 'example': 'LAMBDA(x, SUM(x))'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + assert len(warnings) == 0 + + def test_multiple_parameters_one_empty_fails(self): + """Test that multiple parameters with one empty example fails.""" + data = { + 'parameters': [ + {'name': 'range', 'description': 'Data range', 'example': 'A1:Z100'}, + {'name': 'value', 'description': 'Value', 'example': ''}, + {'name': 'func', 'description': 'Callback', 'example': 'LAMBDA(x, SUM(x))'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 1 + assert 'value' in errors[0].lower() + assert 'empty example' in errors[0].lower() + + def test_parameter_with_quoted_example_passes(self): + """Test that parameter with quoted example passes.""" + data = { + 'parameters': [ + {'name': 'mode', 'description': 'Mode', 'example': '"rows-any"'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + + def test_parameter_with_blank_function_example_passes(self): + """Test that parameter with BLANK() example passes.""" + data = { + 'parameters': [ + {'name': 'fill', 'description': 'Fill value', 'example': 'BLANK()'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + + def test_parameter_with_numeric_example_passes(self): + """Test that parameter with numeric example passes.""" + data = { + 'parameters': [ + {'name': 'count', 'description': 'Count', 'example': 10} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + + def test_parameter_with_zero_example_passes(self): + """Test that parameter with zero example passes (falsy but valid).""" + data = { + 'parameters': [ + {'name': 'offset', 'description': 'Offset', 'example': 0} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + + def test_missing_parameters_field_passes(self): + """Test that missing parameters field is skipped.""" + data = {'name': 'TEST_FUNC', 'formula': 'SUM(A1:A10)'} + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + assert len(warnings) == 0 + + def test_non_list_parameters_field_passes(self): + """Test that non-list parameters field is skipped.""" + data = {'parameters': 'not a list'} + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + assert len(warnings) == 0 + + def test_parameter_with_quoted_empty_string_example_fails(self): + """Test that parameter with '""' as example fails (literal empty string).""" + data = { + 'parameters': [ + {'name': 'replacement', 'description': 'Value', 'example': ''} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 1 + assert 'empty example' in errors[0].lower() + + def test_parameter_with_double_quoted_example_passes(self): + """Test that parameter with double-quoted string example passes. + + When the YAML value is '"text"', it represents the string with quotes, + which is a non-empty string, so it should pass. + """ + data = { + 'parameters': [ + {'name': 'text', 'description': 'Text', 'example': '""'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + assert len(errors) == 0 + + def test_non_dict_parameter_element_skipped(self): + """Test that non-dict parameter elements are skipped.""" + data = { + 'parameters': [ + {'name': 'param1', 'description': 'Valid', 'example': 'A1'}, + 'invalid string element', + {'name': 'param2', 'description': 'Also valid', 'example': 'B1'} + ] + } + errors, warnings = self.rule.check(Path("test.yaml"), data) + + # Should check the valid parameters but skip the string element + assert len(errors) == 0 + + def test_error_includes_file_path(self): + """Test that error message includes the file path.""" + data = { + 'parameters': [ + {'name': 'test', 'description': 'Test', 'example': ''} + ] + } + errors, warnings = self.rule.check(Path("formulas/test.yaml"), data) + + assert len(errors) == 1 + assert 'formulas/test.yaml' in errors[0] + + class TestFormulaLinter: """Test the main FormulaLinter class.""" @@ -230,8 +433,9 @@ def test_linter_has_expected_rules(self): assert 'no-leading-equals' in rule_names assert 'no-top-level-lambda' in rule_names + assert 'require-parameter-examples' in rule_names assert 'valid-formula-syntax' in rule_names - assert len(self.linter.rules) == 3 + assert len(self.linter.rules) == 4 def test_lint_file_with_valid_yaml(self): """Test linting a valid YAML file with no linter errors."""