Skip to content

Enhance identifier and string literal sanitization in BindingGenerator#1345

Merged
Ryang-21 merged 4 commits intomasterfrom
sanitize-binding-gen-names
Mar 4, 2026
Merged

Enhance identifier and string literal sanitization in BindingGenerator#1345
Ryang-21 merged 4 commits intomasterfrom
sanitize-binding-gen-names

Conversation

@Ryang-21
Copy link
Copy Markdown
Contributor

@Ryang-21 Ryang-21 commented Mar 4, 2026

  • Updated sanitizeIdentifier to strip non-identifier characters and handle edge cases.
  • Implemented escapeStringLiteral for safe interpolation in string literals.
  • Added tests to verify sanitization of struct field names, enum case names, and union case tags.

Copilot AI review requested due to automatic review settings March 4, 2026 19:11
@github-project-automation github-project-automation Bot moved this to Backlog (Not Ready) in DevX Mar 4, 2026
@Ryang-21 Ryang-21 requested a review from quietbits March 4, 2026 19:13
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the safety of generated TypeScript bindings by sanitizing spec-provided identifiers and escaping spec-provided strings when emitting double-quoted string literals, with added integration coverage to prevent regressions.

Changes:

  • Expanded sanitizeIdentifier to replace non-identifier characters and add a _unnamed fallback.
  • Added escapeStringLiteral and applied it when emitting union tags and error-enum message strings.
  • Added integration tests covering quotes in string literals and sanitization of struct/enum identifiers.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

File Description
src/bindings/utils.ts Updates identifier sanitization behavior and introduces string-literal escaping helper used by generators.
src/bindings/types.ts Applies identifier sanitization to struct fields / enum case names and escapes spec-provided strings in emitted string literals.
src/bindings/client.ts Sanitizes function input names (and fromJSON method keys) to avoid invalid TS identifiers in generated client surface.
test/integration/bindings.test.ts Adds integration tests for escaping quotes and sanitizing identifiers across generated types.
Comments suppressed due to low confidence (2)

src/bindings/types.ts:146

  • Sanitizing struct field names can collapse distinct spec field names into the same identifier (e.g. differing punctuation/whitespace), which can lead to duplicate property declarations in the generated interface and makes it impossible to represent both fields accurately. Consider detecting collisions after sanitizeIdentifier and disambiguating (or using quoted property names for the original field name and mapping to a safe alias if needed).
    const fields = struct
      .fields()
      .map((field) => {
        const fieldName = sanitizeIdentifier(field.name().toString());
        const fieldType = parseTypeFromTypeDef(field.type());
        const fieldDoc = formatJSDocComment(field.doc().toString(), 2);

        return `${fieldDoc}  ${fieldName}: ${fieldType};`;
      })

src/bindings/types.ts:198

  • Sanitizing enum case names can introduce duplicate members when multiple spec case names normalize to the same identifier, which will cause a TypeScript compile error for the generated enum. Consider checking for duplicates after sanitization and disambiguating (or using string-literal enum member names where appropriate).
    const members = enumEntry
      .cases()
      .map((enumCase) => {
        const caseName = sanitizeIdentifier(enumCase.name().toString());
        const caseValue = enumCase.value();
        const caseDoc = enumCase.doc().toString() || `Enum Case: ${caseName}`;

        return `${formatJSDocComment(caseDoc, 2)}  ${caseName} = ${caseValue}`;
      })

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/bindings/utils.ts Outdated
if (isNameReserved(identifier)) {
// Append underscore to reserved
return identifier + "_";
// Strip any characters that are not valid in JS/TS identifiers
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says this strips characters that are not valid JS/TS identifier characters, but the implementation only allows ASCII [a-zA-Z0-9_$] and replaces everything else (including many valid Unicode identifier characters) with _. Either adjust the comment to reflect the ASCII-only behavior, or consider a Unicode-aware identifier check if preserving valid Unicode identifiers is desired.

Suggested change
// Strip any characters that are not valid in JS/TS identifiers
// Strip any characters that are not allowed by this ASCII-based identifier pattern

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment seems vaid.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Soroban rust sdk only supports ascii identifiers so I think we should only cater to that encoding

Comment thread src/bindings/utils.ts Outdated
Comment thread src/bindings/client.ts
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 4, 2026

Size Change: +41.3 kB (+0.09%)

Total Size: 45.1 MB

Filename Size Change
dist/stellar-sdk-minimal.js 5.97 MB +6.95 kB (+0.12%)
dist/stellar-sdk-minimal.min.js 5.08 MB +3.37 kB (+0.07%)
dist/stellar-sdk-no-axios.js 5.97 MB +6.95 kB (+0.12%)
dist/stellar-sdk-no-axios.min.js 5.08 MB +3.38 kB (+0.07%)
dist/stellar-sdk-no-eventsource.js 6.22 MB +6.95 kB (+0.11%)
dist/stellar-sdk-no-eventsource.min.js 5.29 MB +3.38 kB (+0.06%)
dist/stellar-sdk.js 6.22 MB +6.95 kB (+0.11%)
dist/stellar-sdk.min.js 5.29 MB +3.38 kB (+0.06%)

compressed-size-action

Comment thread src/bindings/utils.ts Outdated
if (isNameReserved(identifier)) {
// Append underscore to reserved
return identifier + "_";
// Strip any characters that are not valid in JS/TS identifiers
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment seems vaid.

Comment thread src/bindings/utils.ts Outdated
@Ryang-21 Ryang-21 merged commit 2f52d0e into master Mar 4, 2026
10 checks passed
@Ryang-21 Ryang-21 deleted the sanitize-binding-gen-names branch March 4, 2026 23:57
@github-project-automation github-project-automation Bot moved this from Backlog (Not Ready) to Done in DevX Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants