fix(cli): return non-zero exit codes on errors

[ajitpratap0]

Fixes #289

Problem

The format and lint CLI commands were returning exit code 0 even when errors occurred (e.g., file not found, format failures). This broke CI/CD integration.

Fix

• format: Return error when FailedFiles > 0 after formatting attempt
• lint: Return error when files have read errors (previously only checked violation counts)
• Updated test to expect error on non-existent file

Verification

gosqlx format "INVALID"    → exit 1 ✓
gosqlx lint "INVALID"      → exit 1 ✓
gosqlx parse "INVALID"     → exit 1 ✓ (already worked)
gosqlx validate "INVALID"  → exit 1 ✓ (already worked)
echo "SELECT 1" | gosqlx format → exit 0 ✓
echo "SELECT 1" | gosqlx lint   → exit 0 ✓

[claude]

Claude finished @ajitpratap0's task —— View job

---

Code Review Complete ✅

Reviewing PR #302: fix(cli): return non-zero exit codes on errors

Analysis Summary

This PR correctly addresses issue #289 by fixing CLI commands to return proper exit codes on errors. The changes are well-implemented and targeted.

Code Quality Assessment

✅ Excellent

• Clear Intent: Changes directly address the problem described in issue CLI returns exit code 0 on parse/validate/format errors #289
• Minimal Impact: Only adds the necessary logic without unnecessary modifications
• Consistent Patterns: Follows existing error handling patterns in the codebase
• Proper Testing: Test expectations were correctly updated to reflect the new behavior

Detailed Review

cmd/gosqlx/cmd/format.go +5/-0

• ✅ Good: Added check for result.FailedFiles > 0 at format.go:106-108
• ✅ Correct: Returns error with descriptive message instead of silent success
• ✅ Placement: Logically placed after the existing check mode validation

cmd/gosqlx/cmd/lint.go +9/-2

• ✅ Excellent: Added fileErrorCount tracking at lint.go:173
• ✅ Robust: Checks fileResult.Error != nil for each file at lint.go:175-177
• ✅ Priority: File errors take precedence over violation-based exits at lint.go:189-191
• ✅ Descriptive: Clear error message format matches format.go pattern

cmd/gosqlx/cmd/lint_test.go +2/-2

• ✅ Correct: Updated test expectation from "should not return error" to "should return error" at lint_test.go:155-156
• ✅ Logical: Test now properly validates the intended error behavior

Potential Issues & Edge Cases

⚠️ Minor Considerations:

1. Error vs os.Exit Consistency: The format command uses return fmt.Errorf() for failed files, while both commands use os.Exit(1) for other conditions. This is actually correct since file errors should bubble up as returnable errors, while validation failures warrant direct exit.

2. Error Message Consistency: Both commands now use consistent error message patterns ("%d file(s) failed/had errors").

Performance Considerations

✅ No Impact: The added logic is O(n) where n is the number of files, which is already being iterated. No performance degradation.

Security Assessment

✅ No Concerns: Changes only affect exit code behavior, no security implications.

Test Coverage

✅ Adequate:

• The updated test in TestLintCmd_NonExistentFile correctly validates the new error behavior
• Existing integration verification shows the fix works as expected
• Manual testing examples in PR description demonstrate comprehensive validation

Recommendations

✅ Ready to Merge - This is a clean, focused fix that:

• Directly solves the reported problem
• Maintains backward compatibility for success cases
• Follows existing code patterns and conventions
• Includes appropriate test updates
• Has been manually verified by the author

The implementation correctly distinguishes between different types of errors and handles them appropriately for CI/CD integration.

---

[ajitpratap0]

Consolidated into batch PR #304