-
Notifications
You must be signed in to change notification settings - Fork 189
feat: handle nested quotes properly in documentation examples #1219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -105,7 +105,7 @@ public function getReturn(array $method, array $spec): string | |||||||||||||||||||||||||||||||||||
$this->populateGenerics($method['responseModel'], $spec, $models); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
$models = array_unique($models); | ||||||||||||||||||||||||||||||||||||
$models = array_filter($models, fn ($model) => $model != $this->toPascalCase($method['responseModel'])); | ||||||||||||||||||||||||||||||||||||
$models = array_filter($models, fn($model) => $model != $this->toPascalCase($method['responseModel'])); | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
if (!empty($models)) { | ||||||||||||||||||||||||||||||||||||
$ret .= '<' . implode(', ', $models) . '>'; | ||||||||||||||||||||||||||||||||||||
|
@@ -118,14 +118,14 @@ public function getReturn(array $method, array $spec): string | |||||||||||||||||||||||||||||||||||
return 'Promise<{}>'; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||
/** | ||||||||||||||||||||||||||||||||||||
* @param array $param | ||||||||||||||||||||||||||||||||||||
* @return string | ||||||||||||||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||||||||||||||
public function getParamExample(array $param): string | ||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||
$type = $param['type'] ?? ''; | ||||||||||||||||||||||||||||||||||||
$example = $param['example'] ?? ''; | ||||||||||||||||||||||||||||||||||||
$type = $param['type'] ?? ''; | ||||||||||||||||||||||||||||||||||||
$example = $param['example'] ?? ''; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
$hasExample = !empty($example) || $example === 0 || $example === false; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
@@ -139,16 +139,49 @@ public function getParamExample(array $param): string | |||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return match ($type) { | ||||||||||||||||||||||||||||||||||||
self::TYPE_ARRAY, self::TYPE_FILE, self::TYPE_INTEGER, self::TYPE_NUMBER => $example, | ||||||||||||||||||||||||||||||||||||
self::TYPE_BOOLEAN => ($example) ? 'true' : 'false', | ||||||||||||||||||||||||||||||||||||
self::TYPE_OBJECT => ($example === '{}') | ||||||||||||||||||||||||||||||||||||
? '{}' | ||||||||||||||||||||||||||||||||||||
: (($formatted = json_encode(json_decode($example, true), JSON_PRETTY_PRINT)) | ||||||||||||||||||||||||||||||||||||
? preg_replace('/\n/', "\n ", $formatted) | ||||||||||||||||||||||||||||||||||||
: $example), | ||||||||||||||||||||||||||||||||||||
self::TYPE_STRING => "'{$example}'", | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
switch ($type) { | ||||||||||||||||||||||||||||||||||||
case self::TYPE_ARRAY: | ||||||||||||||||||||||||||||||||||||
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 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) { | |
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.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason for removing these files? pretty sure they are needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was accidentally pushed I'm reverting it. Please review changes for web, deno & react-native.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -203,16 +203,49 @@ public function getParamExample(array $param): string | |||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
return match ($type) { | ||||||||||||||||||||||||||||
self::TYPE_ARRAY, self::TYPE_FILE, self::TYPE_INTEGER, self::TYPE_NUMBER => $example, | ||||||||||||||||||||||||||||
self::TYPE_BOOLEAN => ($example) ? 'true' : 'false', | ||||||||||||||||||||||||||||
self::TYPE_OBJECT => ($example === '{}') | ||||||||||||||||||||||||||||
? '{}' | ||||||||||||||||||||||||||||
: (($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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we only needed change in type string, any reasons for changing all the others? |
||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
switch ($type) { | ||||||||||||||||||||||||||||
case self::TYPE_ARRAY: | ||||||||||||||||||||||||||||
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: 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; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
Comment on lines
+217
to
+223
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||
case self::TYPE_INTEGER: | ||||||||||||||||||||||||||||
case self::TYPE_NUMBER: | ||||||||||||||||||||||||||||
return $example; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
case self::TYPE_FILE: | ||||||||||||||||||||||||||||
return 'InputFile.fromPath(\'/path/to/file\', \'filename\')'; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
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: | ||||||||||||||||||||||||||||
return json_encode((string) $example, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
public function getReturn(array $method, array $spec): string | ||||||||||||||||||||||||||||
|
@@ -241,7 +274,7 @@ public function getReturn(array $method, array $spec): string | |||||||||||||||||||||||||||
$this->populateGenerics($method['responseModel'], $spec, $models); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
$models = array_unique($models); | ||||||||||||||||||||||||||||
$models = array_filter($models, fn ($model) => $model != $this->toPascalCase($method['responseModel'])); | ||||||||||||||||||||||||||||
$models = array_filter($models, fn($model) => $model != $this->toPascalCase($method['responseModel'])); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (!empty($models)) { | ||||||||||||||||||||||||||||
$ret .= '<' . implode(', ', $models) . '>'; | ||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,17 +144,52 @@ public function getParamExample(array $param): string | |
}; | ||
} | ||
|
||
return match ($type) { | ||
self::TYPE_ARRAY, self::TYPE_INTEGER, self::TYPE_NUMBER => $example, | ||
self::TYPE_FILE => 'document.getElementById(\'uploader\').files[0]', | ||
self::TYPE_BOOLEAN => ($example) ? 'true' : 'false', | ||
self::TYPE_OBJECT => ($example === '{}') | ||
? '{}' | ||
: (($formatted = json_encode(json_decode($example, true), JSON_PRETTY_PRINT)) | ||
? preg_replace('/\n/', "\n ", $formatted) | ||
: $example), | ||
self::TYPE_STRING => "'{$example}'", | ||
}; | ||
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); | ||
} | ||
Comment on lines
+147
to
+192
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❓ Verification inconclusiveRobustness 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 |
||
} | ||
|
||
public function getReadOnlyProperties(array $parameter, string $responseModel, array $spec = []): array | ||
|
@@ -270,7 +305,7 @@ public function getGenerics(string $model, array $spec, bool $skipFirst = false) | |
} | ||
|
||
$generics = array_unique($generics); | ||
$generics = array_map(fn ($type) => "{$type} extends Models.{$type} = Models.Default{$type}", $generics); | ||
$generics = array_map(fn($type) => "{$type} extends Models.{$type} = Models.Default{$type}", $generics); | ||
|
||
return '<' . implode(', ', $generics) . '>'; | ||
} | ||
|
@@ -303,7 +338,7 @@ public function getReturn(array $method, array $spec): string | |
$this->populateGenerics($method['responseModel'], $spec, $models); | ||
|
||
$models = array_unique($models); | ||
$models = array_filter($models, fn ($model) => $model != $this->toPascalCase($method['responseModel'])); | ||
$models = array_filter($models, fn($model) => $model != $this->toPascalCase($method['responseModel'])); | ||
|
||
if (!empty($models)) { | ||
$ret .= '<' . implode(', ', $models) . '>'; | ||
|
@@ -323,7 +358,7 @@ public function getSubSchema(array $property, array $spec, string $methodName = | |
$generics = []; | ||
$this->populateGenerics($property['sub_schema'], $spec, $generics); | ||
|
||
$generics = array_filter($generics, fn ($model) => $model != $this->toPascalCase($property['sub_schema'])); | ||
$generics = array_filter($generics, fn($model) => $model != $this->toPascalCase($property['sub_schema'])); | ||
|
||
$ret .= $this->toPascalCase($property['sub_schema']); | ||
if (!empty($generics)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -916,6 +916,9 @@ protected function render(TemplateWrapper $template, string $destination, ?strin | |
throw new Exception('No minifier found for ' . $ext . ' file'); | ||
} | ||
} | ||
|
||
// No post-processing here: per-language getParamExample implementations should | ||
|
||
// produce properly escaped, language-appropriate example literals. | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,22 +1,22 @@ | ||
export class Permission { | ||
|
||
static read = (role: string): string => { | ||
return `read("${role}")` | ||
return `read('${role}')` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here, not related |
||
} | ||
|
||
static write = (role: string): string => { | ||
return `write("${role}")` | ||
return `write('${role}')` | ||
} | ||
|
||
static create = (role: string): string => { | ||
return `create("${role}")` | ||
return `create('${role}')` | ||
} | ||
|
||
static update = (role: string): string => { | ||
return `update("${role}")` | ||
return `update('${role}')` | ||
} | ||
|
||
static delete = (role: string): string => { | ||
return `delete("${role}")` | ||
return `delete('${role}')` | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Escape single quotes in permission-token fallback.
Ensure read("O'Connor") becomes read('O'Connor'), not invalid JS.
Apply:
🤖 Prompt for AI Agents