chore(scanner): address PR review comments on limit, safe-int, and string escaping

Author: chalmerlowe

scripts/version_scanner/tests/unit/test_version_scanner.py

@@ -26,6 +26,7 @@
     _truncate_context,
     _wrap_sheet_hyperlink,
     _wrap_sheet_string,
+    _safe_int,
     format_for_raw_csv,
     format_for_spreadsheet,
     format_for_console
@@ -475,6 +476,42 @@ def test_main_stdout(capsys):
     assert "test.py:1 [test] 3.7" in captured.out
 
 
+def test_main_without_stdout_limits_output(capsys):
+    """Test that main() without --stdout prints only 10 matches and shows a suffix."""
+    test_args = ['version_scanner.py', '-d', 'python', '-v', '3.7']
+    matches = [{'file_path': f'test_{i}.py', 'line_number': i, 'matched_string': '3.7', 'rule_name': 'test'} for i in range(15)]
+    with mock.patch('sys.argv', test_args):
+        from version_scanner import main
+        with mock.patch('version_scanner.scan_repository', return_value=matches):
+            with pytest.raises(SystemExit):
+                main()
+    
+    captured = capsys.readouterr()
+    # Should only print first 10 matches
+    for i in range(10):
+        assert f"test_{i}.py:{i} [test] 3.7" in captured.out
+    for i in range(10, 15):
+        assert f"test_{i}.py:{i} [test] 3.7" not in captured.out
+    assert "... and 5 more matches." in captured.out
+
+
+def test_main_with_stdout_prints_all(capsys):
+    """Test that main() with --stdout prints all matches without limiting."""
+    test_args = ['version_scanner.py', '-d', 'python', '-v', '3.7', '--stdout']
+    matches = [{'file_path': f'test_{i}.py', 'line_number': i, 'matched_string': '3.7', 'rule_name': 'test'} for i in range(15)]
+    with mock.patch('sys.argv', test_args):
+        from version_scanner import main
+        with mock.patch('version_scanner.scan_repository', return_value=matches):
+            with pytest.raises(SystemExit):
+                main()
+    
+    captured = capsys.readouterr()
+    # Should print all 15 matches
+    for i in range(15):
+        assert f"test_{i}.py:{i} [test] 3.7" in captured.out
+    assert "... and 5 more matches." not in captured.out
+
+
 def test_main_does_not_print_rules(capsys):
     """Test that main() does not print the list of loaded rules to stdout."""
     test_args = ['version_scanner.py', '-d', 'python', '-v', '3.7']
@@ -576,9 +613,29 @@ def test_wrap_sheet_hyperlink():
 
 def test_wrap_sheet_string():
     assert _wrap_sheet_string("3.10") == '="3.10"'
+    assert _wrap_sheet_string('python_requires = ">=3.7"') == '="python_requires = "">=3.7"""'
     assert _wrap_sheet_string("") == ""
     assert _wrap_sheet_string(None) == ""
 
+def test_safe_int():
+    assert _safe_int("123") == 123
+    assert _safe_int("") == 0
+    assert _safe_int(None) == 0
+    assert _safe_int("abc") == 0
+
+def test_format_for_raw_csv_handles_empty_line_number():
+    match = {
+        "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",
+        "repo_path": "packages/pkg_a/setup.py",
+        "package_name": "pkg_a",
+        "rule_name": "python_requires_check",
+        "line_number": "",
+        "matched_string": "3.7",
+        "context_line": "python_requires = '>=3.7'"
+    }
+    formatted = format_for_raw_csv(match)
+    assert formatted["line_number"] == 0
+
 def test_format_for_raw_csv():
     match = {
         "file_path": "google-cloud-python/main/packages/pkg_a/setup.py",

scripts/version_scanner/version_scanner.py

@@ -23,7 +23,7 @@
 import os
 import re
 import sys
-from typing import Dict, List, Tuple
+from typing import Dict, List, Tuple, Any
 import yaml
 
 class ConfigManager:
@@ -208,7 +208,15 @@ def _wrap_sheet_hyperlink(url: str, label: str) -> str:
 def _wrap_sheet_string(value: str) -> str:
     if value is None:
         return ""
-    return f'="{value}"' if value else ""
+    escaped_value = value.replace('"', '""')
+    return f'="{escaped_value}"' if value else ""
+
+
+def _safe_int(value: Any, default: int = 0) -> int:
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return default
 
 
 def format_for_raw_csv(match: Dict[str, str]) -> Dict[str, str]:
@@ -217,7 +225,7 @@ def format_for_raw_csv(match: Dict[str, str]) -> Dict[str, str]:
         "file_path": match.get("file_path", ""),
         "package_name": match.get("package_name", ""),
         "rule_name": match.get("rule_name", ""),
-        "line_number": int(match.get("line_number", 0)) if match.get("line_number") is not None else 0,
+        "line_number": _safe_int(match.get("line_number")),
         "matched_string": match.get("matched_string", ""),
         "context_line": _truncate_context(match.get("context_line", ""), match.get("matched_string", ""))
     }
@@ -661,9 +669,13 @@ def main():
     all_matches = scan_repository(args.path, rules, target_packages, ignore_dirs, version_string=args.version)
 
     print(f"\nFound {len(all_matches)} matches.")
-    for m in all_matches:
+    display_matches = all_matches if args.stdout else all_matches[:10]
+    for m in display_matches:
         print(format_for_console(m))
 
+    if not args.stdout and len(all_matches) > 10:
+        print(f"  ... and {len(all_matches) - 10} more matches.")
+        
     # Get and print summary counts
     rule_counts, package_counts = get_match_counts(all_matches)
     print_summary_table(rule_counts, package_counts)