[jnigen] Support renaming interface mixins - #3528
Conversation
liamappelbe
left a comment
liamappelbe
left a comment
There was a problem hiding this comment.
You've also got merge conflicts to resolve.
| ..._reservedTopLevelNames, | ||
| }; | ||
| final Map<ClassDecl, Map<String, int>> nameCounts = {}; | ||
| final Map<String, Map<String, int>> fileNameCounts = {}; |
There was a problem hiding this comment.
Ok, so it looks like before this change, we were making the assumption that we didn't need to resolve class name conflicts in singleFile mode. This was valid because there was only one user namable class in each file (since the interface mixin's name was based on the main class's name).
Now that the user can provide an arbitrary name for the interface mixin, there's a potential for a name conflict with the main class. So you added this extra map that is used to resolve those conflicts within files. This works, but is overcomplicated.
A simpler solution would be to delete uniquifyName and topLevelNameCounts, and have all top-level names go through fileNameCounts, regardless of what how many files we're outputting to.
While we're at it, fileNameCounts is a bit of an odd name, so call it topLevelNameCounts, like the map you just deleted.
|
|
||
| final generatedFileNameCounts = uniquifyName | ||
| ? topLevelNameCounts | ||
| : fileNameCounts.putIfAbsent( |
There was a problem hiding this comment.
This putIfAbsent call feels like it should be in a little private util method on this class.
| /// By default, the mixin for an interface named `Foo` is named `$Foo`. | ||
| String get interfaceMixinName => | ||
| _classDecl.userDefinedInterfaceMixinName ?? | ||
| '\$${_classDecl.userDefinedName ?? _classDecl.name}'; |
There was a problem hiding this comment.
I don't like this duplication of logic. It might get out of sync with the renamer. Better to just make this getter nullable.
c505921 to
65d1ad4
Compare
2 participants
Description
Adds support for renaming generated interface mixins through the jnigen config visitor API while preserving the existing
$ClassNamedefault.Custom names use the same identifier preprocessing and conflict resolution as other generated names and are used consistently in generated bindings.
Adds tests for renaming, name conflicts, and generated output.
Related Issues
Fixes #3495
PR Checklist
dart tool/ci.dart --alllocally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevelpubspec.yamlworkspace.CHANGELOG.mdfor the relevant packages. (Not needed for small changes such as doc typos).