BridgeJS: normalize keyword-escaped names in generated code - #797
Open
kateinoigakukun wants to merge 1 commit into
Open
BridgeJS: normalize keyword-escaped names in generated code#797kateinoigakukun wants to merge 1 commit into
kateinoigakukun wants to merge 1 commit into
Conversation
`@JS func `default`() {}` produced `bjs_`default`` as the WebAssembly
export symbol — an invalid `@_expose`/`@_cdecl` name — because the
codegenerator read `TokenSyntax.text` (which keeps backticks for
keyword-escaped identifiers) and never normalized it. The backticks
leaked into ABI names, generated Swift identifiers, and JS/d.ts names.
The fix splits name escaping into three context-aware helpers, grounded
in swift-syntax's grammar (`isValidSwiftIdentifier(for:)`):
- `backtickIfNeeded()` — declaration position (`var`/`let`/property/
`let`-binding). Keywords are not valid bare here; escapes all
keywords including `self` (`var self: Int` is invalid).
- `backtickIfNeededForMemberAccess()` — `.name` position. Most keywords
are valid bare (`obj.class`, `.break`); `self` is the exception
because `obj.self` is the identity expression, not a member access,
so a property named `self` must be written as `` obj.`self` ``.
- `backtickIfNeededForLocalReference()` — body references. `self` is
valid bare (refers to the parameter); other keywords still need
escaping. Parameter *declarations* (`_ name: Type`) stay bare for
every keyword except `inout`.
All helpers are idempotent (no double-wrapping) and escape dotted
paths per-component. Names are normalized (backticks stripped) at
extraction in `SwiftToSkeleton` so ABI/JSON/JS names are clean.
Adds `IdentifierEscapingTests` pinning the rules against
`isValidSwiftIdentifier(for:)`, a `KeywordNames` codegen snapshot
covering all positions, and macro tests for keyword names under
`@JSFunction`/`@JSGetter`/`@JSSetter`. 200 tests pass; no existing
snapshots changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@JS funcdefault() {}producedbjs_default`` as the WebAssembly export symbol — an invalid@_expose/`@_cdecl` name — because the BridgeJSTool codegenerator read `TokenSyntax.text` (which keeps backticks for keyword-escaped identifiers) and never normalized them. The backticks leaked into ABI names, generated Swift identifiers, and JS/d.ts names.The macro plugin (
@JSFunction/@JSGetter/@JSSetter) was already correct viastripBackticks; only the codegenerator needed fixing.Root cause
TokenSyntax.textfor`default`returns the string`default`including the backticks. The export-side collector inSwiftToSkeleton.swiftextracted names with raw.textand stored them with backticks intact, so they flowed unchanged into:bjs_default``_bjs_`default``default`Fix
Split name escaping into three context-aware helpers (in
ImportTS.swift), grounded in swift-syntax's grammar (SwiftParser.IsValidIdentifier/isValidSwiftIdentifier(for:)):var/let/property/let-binding)backtickIfNeeded()var self: Intis invalid.name)backtickIfNeededForMemberAccess()self—obj.selfis the identity expr, not a member accessbacktickIfNeededForLocalReference()self+ non-keywords —selfrefers to the parameter_ name: Type)inoutAll helpers are idempotent (strip existing backticks first → no double-wrapping) and escape dotted paths per-component. Names are normalized (backticks stripped) at extraction in
SwiftToSkeletonso ABI/JSON/JS names are clean, then re-escaped with the correct context helper at every Swift emission site.The non-obvious finding:
obj.selfcompiles but returnsobj(identity expression), silently accessing the wrong thing — soselfis the only keyword that must be escaped in member access, whileclass/where/in/case/etc. are valid bare there.Tests
IdentifierEscapingTests.swift(12 tests): pins each context with explicit keyword lists, plus a parametric test asserting the helpers agree withisValidSwiftIdentifier(for:)for 23 names — so the rules track swift-syntax's grammar rather than a hand-maintained keyword list. Covers idempotency, double-wrap avoidance, and dotted-path per-component escaping.KeywordNames.swiftcodegen input + snapshots (.swift/.json/.js/.d.ts) covering all positions with keywords:default,Parser(in/self/class/where),Record(case/continue/as),Token(break/return/switch),Observer(do/for/if/else). The generated Swift snapshot parses cleanly underswiftc -parse.@JSFunction/@JSGetter/@JSSetter.200 tests pass; no existing snapshots changed.