feat: handle nested quotes properly in documentation examples #1219
Conversation
WalkthroughReplaced match expressions with switch-based logic in multiple SDK language classes (src/SDK/Language/Web.php, Deno.php, Node.php, ReactNative.php) to provide explicit handling for TYPE_ARRAY, TYPE_OBJECT, TYPE_INTEGER/NUMBER, TYPE_FILE, TYPE_BOOLEAN, and TYPE_STRING (including JSON decoding/pretty-printing and permission-token normalization). Adjusted spacing in several arrow functions (getGenerics, getReturn, getSubSchema). Updated permission template outputs (templates/*/permission.ts.twig) to use single quotes around role values. No public API signature changes. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
templates/react-native/src/permission.ts.twig (1)
4-4: Switch to single quotes fixes nested-quote breakage — LGTM.Prevents invalid examples like ["read("any")"]. One caveat: if role can contain a single quote, escape it or ensure upstream never includes it.
Option (optional) to harden:
- return `read('${role}')` + return `read('${role.replace(/'/g, "\\'")}')`Also applies to: 8-8, 12-12, 16-16, 20-20
templates/deno/src/permission.ts.twig (1)
4-4: Consistent single-quote usage across permissions — looks good.Same note as RN: escape single quotes in role if possible to avoid malformed permission strings.
Optional hardening:
- return `read('${role}')` + return `read('${role.replace(/'/g, "\\'")}')`Also applies to: 8-8, 12-12, 16-16, 20-20
templates/web/src/permission.ts.twig (1)
12-12: Good fix; avoids nested double-quotes in docs.Keeps examples valid in JSON/JS. Consider escaping single quotes in role to cover rare edge cases.
Optional:
- return `read('${role}')`; + return `read('${role.replace(/'/g, "\\'")}')`;Also applies to: 25-25, 35-35, 45-45, 55-55
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/SDK/Language/Web.php(4 hunks)templates/deno/src/permission.ts.twig(1 hunks)templates/react-native/src/permission.ts.twig(1 hunks)templates/web/src/permission.ts.twig(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/SDK/Language/Web.php (1)
src/SDK/Language.php (1)
toPascalCase(96-99)
🔇 Additional comments (1)
src/SDK/Language/Web.php (1)
309-309: Minor style consistency — good.Arrow function spacing normalized (fn($x) …). No functional changes.
Also applies to: 342-342, 362-362
| switch ($type) { | ||
| case self::TYPE_ARRAY: | ||
| // Try to decode JSON array and re-encode to ensure valid JS array literal | ||
| if (is_array($example)) { | ||
| return json_encode($example, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | ||
| } | ||
|
|
||
| $decoded = json_decode($example, true); | ||
| if (null !== $decoded && is_array($decoded)) { | ||
| return json_encode($decoded, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | ||
| } | ||
|
|
||
| // Fallback: handle permission-like strings that contain unescaped inner quotes, | ||
| // e.g. ["read("any")"] -> ["read('any')"] by converting inner double-quotes to single-quotes | ||
| $fixed = preg_replace_callback('/([a-zA-Z_]+)\(\"([^\"]+)\"\)/', function ($m) { | ||
| return $m[1] . "('" . $m[2] . "')"; | ||
| }, $example); | ||
|
|
||
| return $fixed ?? $example; | ||
|
|
||
| case self::TYPE_INTEGER: | ||
| case self::TYPE_NUMBER: | ||
| return $example; | ||
|
|
||
| case self::TYPE_FILE: | ||
| return 'document.getElementById(\'uploader\').files[0]'; | ||
|
|
||
| case self::TYPE_BOOLEAN: | ||
| return ($example) ? 'true' : 'false'; | ||
|
|
||
| case self::TYPE_OBJECT: | ||
| if ($example === '{}') { | ||
| return '{}'; | ||
| } | ||
| $decodedObj = json_decode($example, true); | ||
| if (null !== $decodedObj && is_array($decodedObj)) { | ||
| $formatted = json_encode($decodedObj, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | ||
| return preg_replace('/\n/', "\n ", $formatted); | ||
| } | ||
| return $example; | ||
|
|
||
| case self::TYPE_STRING: | ||
| default: | ||
| // Use json_encode to produce properly quoted and escaped JS string literal | ||
| return json_encode((string) $example, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | ||
| } |
There was a problem hiding this comment.
❓ Verification inconclusive
Robustness gap: TYPE_OBJECT may throw TypeError when example is array/object.
json_decode expects string; passing array/object can error in PHP 8+. Handle non-string examples directly and tighten decoding.
Proposed patch:
switch ($type) {
case self::TYPE_ARRAY:
// Try to decode JSON array and re-encode to ensure valid JS array literal
if (is_array($example)) {
return json_encode($example, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
}
@@
return ($example) ? 'true' : 'false';
case self::TYPE_OBJECT:
- if ($example === '{}') {
- return '{}';
- }
- $decodedObj = json_decode($example, true);
- if (null !== $decodedObj && is_array($decodedObj)) {
- $formatted = json_encode($decodedObj, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
- return preg_replace('/\n/', "\n ", $formatted);
- }
- return $example;
+ // Accept pre-parsed examples
+ if (is_array($example) || is_object($example)) {
+ $formatted = json_encode($example, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+ return preg_replace('/\n/', "\n ", $formatted);
+ }
+ if ($example === '{}' || \trim((string) $example) === '{}') {
+ return '{}';
+ }
+ $decodedObj = json_decode((string) $example, true);
+ if (json_last_error() === JSON_ERROR_NONE && is_array($decodedObj)) {
+ $formatted = json_encode($decodedObj, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
+ return preg_replace('/\n/', "\n ", $formatted);
+ }
+ return (string) $example;
case self::TYPE_STRING:
default:
// Use json_encode to produce properly quoted and escaped JS string literal
return json_encode((string) $example, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE);
}Also, optional: after fixing inner quotes in TYPE_ARRAY, consider trimming and returning the normalized string (current approach is fine for JS literals).
Handle non-string $example in TYPE_OBJECT to prevent TypeError: In the TYPE_OBJECT case of src/SDK/Language/Web.php, first check is_array($example) || is_object($example) and format with json_encode, then cast $example to string for json_decode and verify json_last_error().
| public function getFiles(): array | ||
| { | ||
| return [ | ||
| [ |
There was a problem hiding this comment.
any reason for removing these files? pretty sure they are needed
There was a problem hiding this comment.
this was accidentally pushed I'm reverting it. Please review changes for web, deno & react-native.
src/SDK/SDK.php
Outdated
| } | ||
| } | ||
|
|
||
| // No post-processing here: per-language getParamExample implementations should |
There was a problem hiding this comment.
Pushed accidentally, I'm reverting it.
| */ | ||
| static read = (role: string): string => { | ||
| return `read("${role}")`; | ||
| return `read('${role}')`; |
There was a problem hiding this comment.
these shoudnt matter no? not related to docs at all
|
|
||
| static read = (role: string): string => { | ||
| return `read("${role}")` | ||
| return `read('${role}')` |
|
|
||
| static read = (role: string): string => { | ||
| return `read("${role}")` | ||
| return `read('${role}')` |
| : (($formatted = json_encode(json_decode($example, true), JSON_PRETTY_PRINT)) | ||
| ? preg_replace('/\n/', "\n ", $formatted) | ||
| : $example), | ||
| self::TYPE_STRING => "'{$example}'", |
There was a problem hiding this comment.
we only needed change in type string, any reasons for changing all the others?
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/SDK/Language/ReactNative.php (2)
231-233: Robust boolean parsing.String "false" evaluates truthy; current code would output true.
Apply:
- case self::TYPE_BOOLEAN: - return ($example) ? 'true' : 'false'; + case self::TYPE_BOOLEAN: + $bool = is_bool($example) ? $example : filter_var($example, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + return ($bool === true) ? 'true' : 'false';
206-248: De-duplicate normalization logic across languages.The ARRAY/OBJECT formatting and permission-token fix are duplicated in Node/Deno. Extract a protected helper (e.g., normalizePermissionTokens(string): string) in a common base to prevent drift.
src/SDK/Language/Deno.php (2)
203-205: Robust boolean parsing.Avoid treating "false" as truthy.
Apply:
- case self::TYPE_BOOLEAN: - return ($example) ? 'true' : 'false'; + case self::TYPE_BOOLEAN: + $bool = is_bool($example) ? $example : filter_var($example, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + return ($bool === true) ? 'true' : 'false';
178-220: Centralize shared logic.Extract the permission-token normalization and JSON pretty-printing into a shared helper to keep Deno/ReactNative/Web/Node aligned.
src/SDK/Language/Node.php (2)
167-169: Robust boolean parsing.Avoid truthiness pitfalls with "false"/"0".
Apply:
- case self::TYPE_BOOLEAN: - return ($example) ? 'true' : 'false'; + case self::TYPE_BOOLEAN: + $bool = is_bool($example) ? $example : filter_var($example, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + return ($bool === true) ? 'true' : 'false';
142-184: Reduce duplication across languages.The ARRAY/OBJECT handling and permission normalization are shared with Web/Deno/ReactNative; factor into a base helper to keep behavior consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/SDK/Language/Deno.php(1 hunks)src/SDK/Language/Node.php(3 hunks)src/SDK/Language/ReactNative.php(2 hunks)src/SDK/SDK.php(1 hunks)templates/deno/src/permission.ts.twig(1 hunks)templates/react-native/src/permission.ts.twig(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/SDK/SDK.php
🚧 Files skipped from review as they are similar to previous changes (2)
- templates/deno/src/permission.ts.twig
- templates/react-native/src/permission.ts.twig
🧰 Additional context used
🧬 Code graph analysis (2)
src/SDK/Language/ReactNative.php (1)
src/SDK/Language.php (1)
toPascalCase(96-99)
src/SDK/Language/Node.php (3)
src/SDK/Language.php (2)
toPascalCase(96-99)getParamExample(56-56)src/SDK/Language/Deno.php (1)
getParamExample(161-221)src/SDK/Language/Web.php (1)
getParamExample(130-193)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: build (8.3, WebNode)
- GitHub Check: build (8.3, Python312)
- GitHub Check: build (8.3, Python313)
- GitHub Check: build (8.3, Node18)
- GitHub Check: build (8.3, Ruby27)
- GitHub Check: kotlin (server)
- GitHub Check: build (8.3, PHP83)
- GitHub Check: build (8.3, KotlinJava11)
- GitHub Check: build (8.3, KotlinJava17)
- GitHub Check: build (8.3, Android14Java17)
- GitHub Check: swift (server)
- GitHub Check: build (8.3, DartBeta)
- GitHub Check: build (8.3, DotNet60)
- GitHub Check: build (8.3, Android5Java17)
- GitHub Check: build (8.3, DartStable)
- GitHub Check: build (8.3, Go112)
- GitHub Check: flutter (client)
- GitHub Check: apple (client)
- GitHub Check: android (client)
🔇 Additional comments (1)
src/SDK/Language/Node.php (1)
109-109: Formatting-only change.No behavioral impact.
| // Fallback: normalize permission-like tokens inside array elements | ||
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | ||
| return $m[1] . "('" . $m[2] . "')"; | ||
| }, $example); | ||
|
|
||
| return $fixed ?? $example; | ||
|
|
There was a problem hiding this comment.
Escape single quotes in permission-token fallback.
Ensure read("O'Connor") becomes read('O'Connor'), not invalid JS.
Apply:
- $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
- return $m[1] . "('" . $m[2] . "')";
- }, $example);
+ $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
+ $inner = str_replace("'", "\\'", $m[2]);
+ return $m[1] . "('{$inner}')";
+ }, (string) $example);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/SDK/Language/Deno.php around lines 189 to 195, the preg_replace_callback
builds a single-quoted token string but doesn’t escape inner single quotes, so
input like read("O'Connor") becomes invalid JS; update the callback to escape
single quotes in $m[2] before embedding (e.g. replace ' with \' via str_replace
or similar) so the returned string is $m[1] . "('" . <escaped $m[2]> . "')";
keep the rest of the logic unchanged.
src/SDK/Language/Node.php
Outdated
| // Fallback for permission-like tokens inside array elements: read("role") -> read('role') | ||
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | ||
| return $m[1] . "('" . $m[2] . "')"; | ||
| }, $example); | ||
|
|
||
| return $fixed ?? $example; | ||
|
|
There was a problem hiding this comment.
Escape single quotes in permission-token fallback.
Prevent invalid JS when value contains apostrophes.
Apply:
- $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
- return $m[1] . "('" . $m[2] . "')";
- }, $example);
+ $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
+ $inner = str_replace("'", "\\'", $m[2]);
+ return $m[1] . "('{$inner}')";
+ }, (string) $example);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fallback for permission-like tokens inside array elements: read("role") -> read('role') | |
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | |
| return $m[1] . "('" . $m[2] . "')"; | |
| }, $example); | |
| return $fixed ?? $example; | |
| // Fallback for permission-like tokens inside array elements: read("role") -> read('role') | |
| $fixed = preg_replace_callback( | |
| '/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', | |
| function ($m) { | |
| $inner = str_replace("'", "\\'", $m[2]); | |
| return $m[1] . "('{$inner}')"; | |
| }, | |
| (string) $example | |
| ); | |
| return $fixed ?? $example; |
🤖 Prompt for AI Agents
In src/SDK/Language/Node.php around lines 153 to 159, the fallback that converts
double-quoted permission tokens to single-quoted form doesn't escape single
quotes (or backslashes) in the token value, which can produce invalid JS when
the value contains an apostrophe; change the callback to escape backslashes and
single quotes in $m[2] (e.g. replace "\" with "\\\\" and "'" with "\\'") before
embedding it in the returned string so the produced single-quoted JS literal is
safe.
src/SDK/Language/Node.php
Outdated
| case self::TYPE_FILE: | ||
| return 'document.getElementById(\'uploader\').files[0]'; | ||
|
|
There was a problem hiding this comment.
Wrong FILE example for NodeJS runtime.
Node examples shouldn’t reference document; use InputFile.fromPath for Node.
Apply:
- case self::TYPE_FILE:
- return 'document.getElementById(\'uploader\').files[0]';
+ case self::TYPE_FILE:
+ return 'InputFile.fromPath(\'/path/to/file\', \'filename\')';🤖 Prompt for AI Agents
In src/SDK/Language/Node.php around lines 164 to 166, the NodeJS example returns
a browser DOM file reference (document.getElementById(...).files[0]) which is
invalid for Node; replace that return value with a Node-friendly InputFile
example such as using InputFile.fromPath('path/to/file') (update any surrounding
example text to reflect a path parameter and mention using InputFile.fromPath
for file uploads in Node).
| // Fallback: normalize permission-like tokens inside array elements | ||
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | ||
| return $m[1] . "('" . $m[2] . "')"; | ||
| }, $example); | ||
|
|
||
| return $fixed ?? $example; | ||
|
|
There was a problem hiding this comment.
Escape single quotes in permission-token fallback.
Values like read("O'Connor") will render invalid JS: read('O'Connor'). Escape inner single quotes and cast $example to string.
Apply:
- $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
- return $m[1] . "('" . $m[2] . "')";
- }, $example);
+ $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) {
+ $inner = str_replace("'", "\\'", $m[2]);
+ return $m[1] . "('{$inner}')";
+ }, (string) $example);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Fallback: normalize permission-like tokens inside array elements | |
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | |
| return $m[1] . "('" . $m[2] . "')"; | |
| }, $example); | |
| return $fixed ?? $example; | |
| // Fallback: normalize permission-like tokens inside array elements | |
| $fixed = preg_replace_callback('/([a-zA-Z_][a-zA-Z0-9_]*)\(\s*"([^"\)]*)"\s*\)/', function ($m) { | |
| $inner = str_replace("'", "\\'", $m[2]); | |
| return $m[1] . "('{$inner}')"; | |
| }, (string) $example); | |
| return $fixed ?? $example; |
🤖 Prompt for AI Agents
In src/SDK/Language/ReactNative.php around lines 217-223, the
preg_replace_callback fallback doesn't escape single quotes and doesn't ensure
$example is a string; update the callback to escape inner single quotes in the
permission token (e.g. replace ' with \') when building the replacement, cast
$example to string when passing it to preg_replace_callback (or at least in the
return), and return the escaped result or (string)$example if no replacement
occurred.
2 participants
What does this PR do?
This patch fixes invalid nested double-quote permission tokens that were appearing in generated examples (e.g. permissions: ["read("any")"]), and hardens Web/Deno parameter example rendering so string/array/object examples produce safe JS literals.
Web.phpImprove getParamExample() so string/array/object examples are JSON encoded / pretty-printed as valid JS literals.permission.ts.twigRender permission tokens with single quotes (e.g. read('${role}')) to avoid nested double-quotes in generated JS/TS examples.permission.ts.twigSame change as above for Deno templates.Testing screenshot for reference
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)
Summary by CodeRabbit
Bug Fixes
Style