fix: Escape Special Characters in String Literals - BED-7883#69
fix: Escape Special Characters in String Literals - BED-7883#69LawsonWillard wants to merge 1 commit intomainfrom
Conversation
WalkthroughThe changes update the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cypher/models/cypher/format/format_test.go (1)
94-106: Consider adding a combined escape case in the emitter-path test.You already test combined escaping at the unit level; adding the same pattern here would harden end-to-end coverage.
➕ Optional test-case addition
{ name: "single quote in name", propertyKey: "name", value: `O'Brien`, expectedQuery: `match (n {name: 'O\'Brien'}) return n`, }, + { + name: "backslash and single quote together", + propertyKey: "path", + value: `path\to\file's location`, + expectedQuery: `match (n {path: 'path\\to\\file\'s location'}) return n`, + },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cypher/models/cypher/format/format_test.go` around lines 94 - 106, Add a new table-driven test case to the existing test cases in format_test.go (the test that asserts emitted Cypher strings using the table entries with fields propertyKey, value, expectedQuery) that combines both backslash and single-quote characters in the value (e.g., value containing both \ and '), and set expectedQuery to the correctly escaped Cypher literal (double-escape backslashes and escape single quotes inside the single-quoted string) so the emitter-path end-to-end behavior is validated for combined escaping.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cypher/models/cypher/format/format_test.go`:
- Around line 94-106: Add a new table-driven test case to the existing test
cases in format_test.go (the test that asserts emitted Cypher strings using the
table entries with fields propertyKey, value, expectedQuery) that combines both
backslash and single-quote characters in the value (e.g., value containing both
\ and '), and set expectedQuery to the correctly escaped Cypher literal
(double-escape backslashes and escape single quotes inside the single-quoted
string) so the emitter-path end-to-end behavior is validated for combined
escaping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9e015794-3e71-4031-9033-81d5c32537f9
📒 Files selected for processing (2)
cypher/models/cypher/format/format_test.gocypher/models/cypher/model.go
Description
Motivation:
Resolves: BED-7883
Special characters in cypher query string literals were not escaped causing query breakage.
Type of Change
Testing
Added new unit tests to ensure escaping works as expected.
Tested locally with a BHCE instance to ensure it works as expected.
Unit tests added / updated
Integration tests added / updated
Manual integration tests run (
go test -tags manual_integration ./integration/...)Driver Impact
drivers/pg)drivers/neo4j)Checklist
go.mod/go.sumare up to date if dependencies changedSummary by CodeRabbit
Bug Fixes
Tests