Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 19 additions & 30 deletions pkgs/jni/tool/generate_jni_bindings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,36 +21,25 @@ const Map<String, String> _constructorAllowList = {
'Short': 's',
};

class Renamer extends Visitor {
late ClassDecl _class;

@override
void visitClass(ClassDecl c) {
_class = c;
c.name = 'J${c.originalName}';
}

@override
void visitMethod(Method m) {
if (!m.isConstructor) return;
final sig = _constructorAllowList[_class.originalName];
if (sig == null) return;
final lister = ListParams();
m.accept(lister);
m.isExcluded = !(lister.params.length == 1 && lister.params.first == sig);
}
}

class ListParams extends Visitor {
List<String> params = [];

@override
void visitParam(Param p) {
params.add(p.originalName);
}
}

Future<void> main() async {
ClassDecl? currentClass;
final renamerVisitor = Visitor.callback(
visitClass: (c) {
currentClass = c;
c.name = 'J${c.originalName}';
},
visitMethod: (m) {
if (!m.isConstructor) return;
final sig = _constructorAllowList[currentClass?.originalName];
if (sig == null) return;
final params = <String>[];
m.accept(Visitor.callback(
visitParam: (p) => params.add(p.originalName),
));
m.isExcluded = !(params.length == 1 && params.first == sig);
},
);

final classes = [
'java.lang.Boolean',
'java.lang.Byte',
Expand Down Expand Up @@ -95,7 +84,7 @@ Future<void> main() async {
hide: classes,
preamble: preamble,
generateStubs: false,
visitors: [Renamer()],
visitors: [renamerVisitor],
),
);
}
46 changes: 46 additions & 0 deletions pkgs/jnigen/lib/src/elements/j_elements.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ abstract class _Element {
/// Users can extend this class to create custom visitors that modify the AST
/// before code generation.
abstract class Visitor {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

abstract base class

const Visitor();

factory Visitor.callback({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's unfortunate this has to be called callback.

Does it work to just call it Visitor(...)? I guess not because the subclasses written by users need to call the parent constructor and it's weird if that one cannot be called Visitor(...).

How about Visitor.inline() or Visitor.fromCallbacks(...), Visitor.concrete(...), Visitor.implementation(...)

Or we should consider making the default constructor not the default name. E.g. that makes the inline visitors more pretty and the custom subclasses slightly less pretty.

class MyVisitor extends Visitor
  MyVisitor(...):super.base();

Side note. I feel we should make a decision whether we want users to subclass or implement visitor. It should either be a base class (non-implementable) or interface class (non-extendible).

If it's only implementable and not extendible, then we don't need a default constructor for sub classes.

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 downside of making it only implementable is that users have to override every method. That's not a big deal for JNIgen's visitors, but FFIgen will have heaps of visit methods. I've gone with the super.base() option.

void Function(ClassDecl node)? visitClass,
void Function(Method node)? visitMethod,
void Function(Field node)? visitField,
void Function(Param node)? visitParam,
}) = _VisitorImpl;

/// Visits a class declaration.
void visitClass(ClassDecl c) {}

Expand All @@ -28,6 +37,43 @@ abstract class Visitor {
void visitParam(Param parameter) {}
}

class _VisitorImpl extends Visitor {
const _VisitorImpl({
void Function(ClassDecl node)? visitClass,
void Function(Method node)? visitMethod,
void Function(Field node)? visitField,
void Function(Param node)? visitParam,
}) : _visitClass = visitClass,
_visitMethod = visitMethod,
_visitField = visitField,
_visitParam = visitParam;

final void Function(ClassDecl node)? _visitClass;
final void Function(Method node)? _visitMethod;
final void Function(Field node)? _visitField;
final void Function(Param node)? _visitParam;

@override
void visitClass(ClassDecl c) {
_visitClass?.call(c);
}

@override
void visitMethod(Method method) {
_visitMethod?.call(method);
}

@override
void visitField(Field field) {
_visitField?.call(field);
}

@override
void visitParam(Param parameter) {
_visitParam?.call(parameter);
}
}

/// A collection of class declarations.
class Classes implements _Element {
Classes(this._classes);
Expand Down
106 changes: 46 additions & 60 deletions pkgs/jnigen/test/user_visitor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,64 +29,6 @@ extension on Iterable<ast.Field> {
List<String> get finalNames => map((f) => f.finalName).toList();
}

// This is customizable by the user
class UserExcluder extends Visitor {
@override
void visitClass(ClassDecl c) {
if (c.binaryName.contains('y')) {
c.isExcluded = true;
}
}

@override
void visitMethod(Method method) {
if (method.name == 'Bar') {
method.isExcluded = true;
}
}

@override
void visitField(Field field) {
if (field.name == 'Bar') {
field.isExcluded = true;
}
}
}

// This is customizable by the user
class UserRenamer extends Visitor {
@override
void visitClass(ClassDecl c) {
if (c.originalName.contains('Foo')) {
c.name = c.originalName.replaceAll('Foo', 'Bar');
}
}

@override
void visitMethod(Method method) {
if (method.originalName.contains('Foo')) {
method.name = method.originalName.replaceAll('Foo', 'Bar');
}
if (method.isConstructor) {
method.name = 'constructor';
}
}

@override
void visitField(Field field) {
if (field.originalName.contains('Foo')) {
field.name = field.originalName.replaceAll('Foo', 'Bar');
}
}

@override
void visitParam(Param parameter) {
if (parameter.originalName.contains('Foo')) {
parameter.name = parameter.originalName.replaceAll('Foo', 'Bar');
}
}
}

Future<void> rename(ast.Classes classes) async {
final config = Config(
outputConfig: OutputConfig(
Expand Down Expand Up @@ -135,7 +77,25 @@ void main() {
});

final simpleClasses = Classes(classes);
simpleClasses.accept(UserExcluder());
simpleClasses.accept(
Visitor.callback(
visitClass: (c) {
if (c.binaryName.contains('y')) {
c.isExcluded = true;
}
},
visitMethod: (method) {
if (method.name == 'Bar') {
method.isExcluded = true;
}
},
visitField: (field) {
if (field.name == 'Bar') {
field.isExcluded = true;
}
},
),
);

expect(classes.decls['y.Foo']?.isExcluded, true);
expect(classes.decls['Foo']?.isExcluded, false);
Expand Down Expand Up @@ -181,7 +141,33 @@ void main() {
});

final simpleClasses = Classes(classes);
simpleClasses.accept(UserRenamer());
simpleClasses.accept(
Visitor.callback(
visitClass: (c) {
if (c.originalName.contains('Foo')) {
c.name = c.originalName.replaceAll('Foo', 'Bar');
}
},
visitMethod: (method) {
if (method.originalName.contains('Foo')) {
method.name = method.originalName.replaceAll('Foo', 'Bar');
}
if (method.isConstructor) {
method.name = 'constructor';
}
},
visitField: (field) {
if (field.originalName.contains('Foo')) {
field.name = field.originalName.replaceAll('Foo', 'Bar');
}
},
visitParam: (parameter) {
if (parameter.originalName.contains('Foo')) {
parameter.name = parameter.originalName.replaceAll('Foo', 'Bar');
}
},
),
);

await rename(classes);

Expand Down
Loading