fix: require mutation expression when using --in-place

[nhomble]

Validates that the expression is a mutation (contains =, |=, del, etc.) before allowing --in-place. This prevents silent data loss when users accidentally use query expressions like . + {} instead of . += {}.

Users must use proper jq mutation syntax:

• .field = value
• . += {field: value}
• del(.field)

[claude]

PR Review: Require mutation expression when using --in-place

Summary

This PR adds important validation to prevent data loss when using the --in-place flag with non-mutation expressions. Overall, this is a solid safety improvement that addresses a real user-facing issue.

Code Quality and Best Practices

Positives:

• Clean, focused change addressing a single concern
• Good separation of concerns - validation logic in query.rs, CLI validation in main.rs
• Consistent error handling using the existing patterns (eprintln! + process::exit(1))
• Well-structured test that validates both failure case and error message content

Suggestions:

1. Error message consistency (src/main.rs:31):
   The error message could be more consistent with standard CLI conventions. Consider:

   eprintln!("error: --in-place requires a mutation expression (=, |=, +=, del, etc.)");

2. String allocation (src/query.rs:18):
   Minor optimization opportunity, but current approach is fine for clarity.

Potential Bugs or Issues

Edge Cases in is_mutation function:

1. False positives with comparison operators (src/query.rs:31-34):
   The current code could produce false positives with expressions like:

   • .x != .y (contains != which has =)
   • .x <= .y or .x >= .y (contains =)

   Recommendation: Replace comparison operators too:

   let without_comparisons = expr.replace("==", "").replace("!=", "").replace("<=", "").replace(">=", "");
   if without_comparisons.contains('=') {
       return true;
   }

2. String literals containing mutation operators (src/query.rs:17-44):
   The function doesn't account for operators appearing in string literals:

   • .title | contains("del(x)") - incorrectly identified as mutation

   Consider adding a comment documenting this limitation:

   // Note: This is a heuristic check that may produce false positives
   // if mutation operators appear in string literals
   pub fn is_mutation(expr: &str) -> bool {

Test Coverage

Strengths:

• Integration test covers the main use case
• Tests both failure condition and error message content
• Good cleanup with fs::remove_file(&temp_file).ok()

Suggestions:

1. Add unit tests for edge cases in src/query.rs to cover:

   • Comparison operators (!=, <=, >=)
   • Other mutation operators (-=, *=, /=, etc.)
   • setpath() and delpaths()

2. Test file content preservation: Verify that the file content is NOT modified when the command fails

Performance and Security

• Performance: No concerns, validation happens once at startup
• Security: This PR actually improves security by preventing accidental data loss

Overall Assessment

This is a well-implemented safety feature. Main recommendations:

1. Must fix: Handle comparison operators to avoid false positives
2. Should add: Unit tests for edge cases
3. Nice to have: Document limitations of the heuristic approach

The PR is ready to merge after addressing the comparison operators issue. Great work on improving user safety!