feat(js_interop_gen): support nested typealiases, namespace parent resolutions, and aliased variable exports in compiler core#553
Conversation
kevmoo
commented
May 28, 2026
kevmoo
commented
- Added surgical support in 'type_resolver.dart' for recursive parent mapping of nested namespace typealiases and direct same-file import resolutions.
- Upgraded export specifier and declaration parsers inside 'transformer.dart' using target getAliasedSymbol lookups to cleanly map and rename aliased global constants in-place.
- Added sanitization logic for union/intersection identifiers, and mapped 0-size unions/keyofs safely to primitive 'never' types.
…solutions, and aliased variable exports in compiler core - Added surgical support in 'type_resolver.dart' for recursive parent mapping of nested namespace typealiases and direct same-file import resolutions. - Upgraded export specifier and declaration parsers inside 'transformer.dart' using target getAliasedSymbol lookups to cleanly map and rename aliased global constants in-place. - Added sanitization logic for union/intersection identifiers, and mapped 0-size unions/keyofs safely to primitive 'never' types.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces identifier sanitization for union, intersection, and enum names, resolves aliased symbols and nested type aliases in namespaces, and improves type resolution logic. Feedback highlights three key areas for improvement: optimizing the _sanitizeIdentifier helper by replacing the while loop with a regex, removing redundant manual caching checks in type_resolver.dart, and replacing a potentially unsafe .reduce call with .expand to prevent runtime StateError exceptions when handling empty iterables.
…e potential runtime StateErrors Address critical review feedback on pull request #553: - Optimize double-underscore sanitization in _sanitizeIdentifier inside types.dart using a single-pass RegExp(r'_{2,}') instead of a heap-allocating while loop, improving matching performance to O(N) with a single allocation. - Eliminate redundant external caching checks in type_resolver.dart before calling transformAndReturn, allowing the target method to correctly encapsulate caching and preventing implementation leakage. - Prevent a potential runtime StateError by replacing list-concatenating .reduce on a mapped collection (which is susceptible to empty iterables) with a safe, flattening .expand default-guarded with const <Node>[].
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces identifier sanitization for union, intersection, and enum types to prevent invalid Dart identifiers, and enhances the type transformer and resolver to properly handle aliased symbols, import/export specifiers, and nested type aliases. Feedback on the changes highlights a potential null pointer exception in type_resolver.dart when resolving symbols, as well as a duplicate comment in transformer.dart.
| var symbol = transformer.typeChecker.getSymbolAtLocation(typeName); | ||
| if (symbol != null) { | ||
| final declarations = symbol.getDeclarations()?.toDart ?? []; | ||
| for (final decl in declarations) { | ||
| if (decl.kind == TSSyntaxKind.ImportSpecifier || | ||
| decl.kind == TSSyntaxKind.ExportSpecifier) { | ||
| symbol = transformer.typeChecker.getAliasedSymbol(symbol!); | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The symbol variable can be null if getSymbolAtLocation(typeName) returns null. If symbol is null, the subsequent call to transformer.typeChecker.getTypeOfSymbol(symbol!) on line 669 will throw a runtime Null Pointer Exception because of the null assertion operator (!). To prevent this runtime crash and handle the null case gracefully, we should add an early null check and throw a descriptive exception, which also aligns with our general rule on avoiding the null assertion operator directly on nullable variables.
| var symbol = transformer.typeChecker.getSymbolAtLocation(typeName); | |
| if (symbol != null) { | |
| final declarations = symbol.getDeclarations()?.toDart ?? []; | |
| for (final decl in declarations) { | |
| if (decl.kind == TSSyntaxKind.ImportSpecifier || | |
| decl.kind == TSSyntaxKind.ExportSpecifier) { | |
| symbol = transformer.typeChecker.getAliasedSymbol(symbol!); | |
| break; | |
| } | |
| } | |
| } | |
| var symbol = transformer.typeChecker.getSymbolAtLocation(typeName); | |
| if (symbol == null) { | |
| throw Exception('Could not resolve type: symbol is null'); | |
| } | |
| final declarations = symbol.getDeclarations()?.toDart ?? []; | |
| for (final decl in declarations) { | |
| if (decl.kind == TSSyntaxKind.ImportSpecifier || | |
| decl.kind == TSSyntaxKind.ExportSpecifier) { | |
| symbol = transformer.typeChecker.getAliasedSymbol(symbol); | |
| break; | |
| } | |
| } |
References
- Avoid using the null assertion operator ('!') directly on nullable parameters. Instead, implement explicit null checks or early guard clauses to handle null values gracefully, prevent runtime crashes, and enable static type promotion.
| // get a name | ||
| // get a name |
There was a problem hiding this comment.
There is a duplicate comment // get a name on consecutive lines. We should remove the redundant duplicate comment to keep the codebase clean.
| // get a name | |
| // get a name | |
| // get a name |
…ove redundant comment Address second round of pull request review comments: - Add a robust early null check on symbol inside getTypeFromDeclaration to prevent runtime Null Check Operator errors when type checker symbol lookups return null. - Utilize static variable declaration and type promotion to completely eliminate unsafe force-null assertions (!) on the resolved symbol. - Clean up a duplicate // get a name comment on consecutive lines inside transformer.dart.
1 participant
