Skip to content

Commit

Permalink
[ffigen] Allow static and instance methods/properties with the same n…
Browse files Browse the repository at this point in the history
…ame (#1706)
  • Loading branch information
liamappelbe authored Nov 10, 2024
1 parent f0ba80c commit 91edc07
Show file tree
Hide file tree
Showing 8 changed files with 1,708 additions and 1,637 deletions.
2 changes: 2 additions & 0 deletions pkgs/ffigen/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
- https://github.com/dart-lang/native/issues/1582
- https://github.com/dart-lang/native/issues/1594
- https://github.com/dart-lang/native/issues/1595
- Allow static and instance methods to have the same name:
https://github.com/dart-lang/native/issues/1136
- __Breaking change__: Change the way ObjC categories are generated. Instead of
inserting their methods into the interface, generate Dart extension methods.
For instance methods this makes no difference to user code (as long as the
Expand Down
27 changes: 17 additions & 10 deletions pkgs/ffigen/lib/src/code_generator/objc_methods.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ mixin ObjCMethods {
List<String> _order = <String>[];

Iterable<ObjCMethod> get methods =>
_order.map((name) => _methods[name]).nonNulls;
ObjCMethod? getMethod(String name) => _methods[name];
_order.map((key) => _methods[key]).nonNulls;
ObjCMethod? getSimilarMethod(ObjCMethod method) => _methods[method.key];

String get originalName;
String get name;
Expand All @@ -29,12 +29,12 @@ mixin ObjCMethods {
void addMethod(ObjCMethod? method) {
if (method == null) return;
if (_shouldIncludeMethod(method)) {
final oldMethod = getMethod(method.originalName);
final oldMethod = getSimilarMethod(method);
if (oldMethod != null) {
_methods[method.originalName] = _maybeReplaceMethod(oldMethod, method);
_methods[method.key] = _maybeReplaceMethod(oldMethod, method);
} else {
_methods[method.originalName] = method;
_order.add(method.originalName);
_methods[method.key] = method;
_order.add(method.key);
}
}
}
Expand Down Expand Up @@ -102,11 +102,11 @@ mixin ObjCMethods {
void filterMethods(bool Function(ObjCMethod method) predicate) {
final newOrder = <String>[];
final newMethods = <String, ObjCMethod>{};
for (final name in _order) {
final method = _methods[name];
for (final key in _order) {
final method = _methods[key];
if (method != null && predicate(method)) {
newMethods[name] = method;
newOrder.add(name);
newMethods[key] = method;
newOrder.add(key);
}
}
_order = newOrder;
Expand Down Expand Up @@ -295,6 +295,13 @@ class ObjCMethod extends AstNode {
}
}

// Key used to dedupe methods in [ObjCMethods]. ObjC is similar to Dart in
// that it doesn't have method overloading, so the [originalName] is mostly
// sufficient as the key. But unlike Dart, ObjC can have static methods and
// instance methods with the same name, so we have to include staticness in
// the key.
String get key => '${isClassMethod ? '+' : '-'}$originalName';

@override
String toString() => '${isOptional ? '@optional ' : ''}$returnType '
'$originalName(${params.join(', ')})';
Expand Down
8 changes: 4 additions & 4 deletions pkgs/ffigen/lib/src/visitor/fix_overridden_methods.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class FixOverriddenMethodsVisitation extends Visitation {
// nullable, to avoid Dart compile errors.
for (var t = node.superType; t != null; t = t.superType) {
for (final method in node.methods) {
final superMethod = t.getMethod(method.originalName);
final superMethod = t.getSimilarMethod(method);
if (superMethod != null &&
!superMethod.isClassMethod &&
!method.isClassMethod) {
Expand Down Expand Up @@ -52,7 +52,7 @@ class FixOverriddenMethodsVisitation extends Visitation {
// isn't allowed in Dart, so we change all such conflicts to properties.
// This change could cause more conflicts in the heirarchy, so first we walk
// up to find the root of the subtree that has this method, then we walk
// down the subtreee to change all conflicting methods to properties.
// down the subtree to change all conflicting methods to properties.
for (final method in node.methods) {
final (root, rootMethod) = _findRootWithMethod(node, method);
if (method.isProperty == rootMethod.isProperty) continue;
Expand All @@ -65,7 +65,7 @@ class FixOverriddenMethodsVisitation extends Visitation {
var root = node;
var rootMethod = method;
for (ObjCInterface? t = node; t != null; t = t.superType) {
final tMethod = t.getMethod(method.originalName);
final tMethod = t.getSimilarMethod(method);
if (tMethod != null) {
root = t;
rootMethod = tMethod;
Expand All @@ -76,7 +76,7 @@ class FixOverriddenMethodsVisitation extends Visitation {

void _convertAllSubtreeMethodsToProperties(
ObjCInterface node, ObjCMethod rootMethod) {
final method = node.getMethod(rootMethod.originalName);
final method = node.getSimilarMethod(rootMethod);
if (method != null && method.kind == ObjCMethodKind.method) {
method.kind = ObjCMethodKind.propertyGetter;
}
Expand Down
6 changes: 6 additions & 0 deletions pkgs/ffigen/test/native_objc_test/method_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,11 @@ void main() {
expect(result.w, 4);
});
});

test('Instance and static methods with same name', () {
// Test for https://github.com/dart-lang/native/issues/1136
expect(testInstance.instStaticSameName(), 123);
expect(MethodInterface.instStaticSameName1(), 456);
});
});
}
13 changes: 13 additions & 0 deletions pkgs/ffigen/test/native_objc_test/method_test.m
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ -(float)addFloats:(float)x Y:(float) y;
-(double)addDoubles:(double)x Y:(double) y;
-(Vec4)Vec4;

// An instance method and a static method with the same name.
// https://github.com/dart-lang/native/issues/1136
- (int32_t)instStaticSameName;
+ (int32_t)instStaticSameName;

@end

@implementation MethodInterface
Expand Down Expand Up @@ -88,4 +93,12 @@ -(Vec4)Vec4 {
return u;
}

- (int32_t)instStaticSameName {
return 123;
}

+ (int32_t)instStaticSameName {
return 456;
}

@end
6 changes: 6 additions & 0 deletions pkgs/ffigen/test/native_objc_test/property_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -79,5 +79,11 @@ void main() {
expect(testInstance.doubleProperty, 1.23);
});
});

test('Instance and static properties with same name', () {
// Test for https://github.com/dart-lang/native/issues/1136
expect(testInstance.instStaticSameName, 123);
expect(PropertyInterface.getInstStaticSameName1(), 456);
});
});
}
13 changes: 13 additions & 0 deletions pkgs/ffigen/test/native_objc_test/property_test.m
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ @interface PropertyInterface : NSObject {
@property double doubleProperty;
@property Vec4 structProperty;

// An instance property and a static property with the same name.
// https://github.com/dart-lang/native/issues/1136
@property(readonly) int32_t instStaticSameName;
@property(class, readonly) int32_t instStaticSameName;

@end

@implementation PropertyInterface
Expand All @@ -43,4 +48,12 @@ + (void)setClassReadWriteProperty:(int32_t)x {
_classReadWriteProperty = x;
}

- (int32_t)instStaticSameName {
return 123;
}

+ (int32_t)instStaticSameName {
return 456;
}

@end
Loading

0 comments on commit 91edc07

Please sign in to comment.