OptionParser than can generate shell completion scripts

[carlhoerberg]

For bash, zsh, and fish.

Fixes #1538

[claude]

Claude finished @carlhoerberg's task —— View job

---

Code Review Complete

Reviewing the shell completion option parser implementation for:

• Read and analyze src/stdlib/option_parser.cr
• Check for bugs and security vulnerabilities
• Look for performance issues and missing error handling
• Identify Crystal anti-patterns
• Provide final review feedback

Issues Found

1. Missing Method Definition - src/stdlib/option_parser.cr:17

The code calls parse_flag_definition(short_flag) and parse_flag_definition(long_flag) but this method is not defined in the class. This appears to be a private method from the parent OptionParser class that may not be accessible, causing potential compilation errors.

2. Code Duplication - Lines 14-35 and 37-58

The two on method overloads are nearly identical (352 lines total, ~44 lines duplicated). This violates the DRY principle and creates maintenance overhead. Both methods have identical logic for parsing flags and determining value types.

3. Potential Security Issue - Shell Injection Risk - src/stdlib/option_parser.cr:89

func_name = "_#{program_name.gsub(/[^a-zA-Z0-9_]/, "_")}"

While the regex sanitizes most characters, the generated bash function name is directly interpolated into shell scripts without additional validation. If program_name contains unusual characters, it could potentially cause issues in shell environments.

4. Unsafe String Access - src/stdlib/option_parser.cr:107,228,291,296

Multiple calls to .not_nil! without prior nil checks:

subcommands << (info.short_flag || info.long_flag).not_nil!

If both short_flag and long_flag are nil, this will raise a runtime exception.

5. Missing Error Handling - Fish Shell String Slicing

io << " -s " << short[1..]  # line 326
io << " -l " << long[2..]   # line 331

No bounds checking before slicing strings. If short or long flags don't have the expected prefix format, this could cause index errors.