Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[swift2objc] Support properties that throw #1939

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[swift2objc] Support properties that throw #1939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions pkgs/swift2objc/lib/src/ast/_core/interfaces/function_declaration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,5 @@ abstract interface class FunctionDeclaration
CanThrow,
CanAsync {
abstract final ReferredType returnType;
abstract final bool isCallingProperty;
}
4 changes: 4 additions & 0 deletions pkgs/swift2objc/lib/src/ast/declarations/compounds/members/method_declaration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class MethodDeclaration extends AstNode
@override
ReferredType returnType;

@override
bool isCallingProperty;

bool isStatic;

String get fullName => [
Expand All @@ -62,6 +65,7 @@ class MethodDeclaration extends AstNode
this.isOverriding = false,
this.throws = false,
this.async = false,
this.isCallingProperty = false,
}) : assert(!isStatic || !isOverriding);

@override
Expand Down
4 changes: 4 additions & 0 deletions pkgs/swift2objc/lib/src/ast/declarations/globals/globals.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class GlobalFunctionDeclaration extends AstNode implements FunctionDeclaration {
@override
List<String> statements;

@override
bool isCallingProperty;

GlobalFunctionDeclaration({
required this.id,
required this.name,
Expand All @@ -55,6 +58,7 @@ class GlobalFunctionDeclaration extends AstNode implements FunctionDeclaration {
this.statements = const [],
this.throws = false,
this.async = false,
this.isCallingProperty = false,
});

@override
Expand Down
5 changes: 2 additions & 3 deletions pkgs/swift2objc/lib/src/parser/parsers/declaration_parsers/parse_variable_declaration.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,10 @@ bool _parseVariableThrows(Json json) {
final throws = json['declarationFragments']
.any((frag) => matchFragment(frag, 'keyword', 'throws'));
if (throws) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can delete this whole if statement and just return throws.

// TODO(https://github.com/dart-lang/native/issues/1765): Support throwing
// getters.
throw Exception("Throwing getters aren't supported yet, at ${json.path}");
return true;
}
return throws;
return false;
}

bool _parseVariableAsync(Json json) {
Expand Down
25 changes: 24 additions & 1 deletion pkgs/swift2objc/lib/src/transformer/transformers/transform_compound.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import '../../ast/_core/shared/parameter.dart';
import '../../ast/declarations/built_in/built_in_declaration.dart';
import '../../ast/declarations/compounds/class_declaration.dart';
import '../../ast/declarations/compounds/members/initializer_declaration.dart';
import '../../ast/declarations/compounds/members/method_declaration.dart';
import '../../ast/declarations/compounds/members/property_declaration.dart';
import '../../parser/_core/utils.dart';
import '../_core/unique_namer.dart';
Expand Down Expand Up @@ -52,6 +53,7 @@ ClassDeclaration transformCompound(
.fillNestingParents(transformedCompound);

transformedCompound.properties = originalCompound.properties
.where((property) => !property.throws)
.map((property) => transformProperty(
property,
wrappedCompoundInstance,
Expand All @@ -72,7 +74,8 @@ ClassDeclaration transformCompound(
.toList()
..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));

transformedCompound.methods = originalCompound.methods
transformedCompound.methods = (originalCompound.methods +
_convertPropertiesToMethods(originalCompound.properties))
.map((method) => transformMethod(
method,
wrappedCompoundInstance,
Expand Down Expand Up @@ -106,3 +109,23 @@ InitializerDeclaration _buildWrapperInitializer(
hasObjCAnnotation: wrappedClassInstance.hasObjCAnnotation,
);
}

List<MethodDeclaration> _convertPropertiesToMethods(
Copy link
Contributor

Choose a reason for hiding this comment

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

So if I understand correctly, you've added a isCallingProperty field to the method and function declarations, which changes the originalCallStatementGenerator param passed to _transformFunction to not pass args. That transformation happens in transformMethod, which is called immediately after you set this flag. In other words, you've added a flag to the AST node, that is only set here, and then is immediately consumed. The flag is only necessary because you're merging these methods with originalCompound.methods before passing them to transformMethod.

It would be cleaner to remove isCallingProperty, and remove _convertPropertiesToMethods, and do all that transformation logic in transformProperty. transformProperty would check the throws flag and decide whether to return a PropertyDeclaration or a MethodDeclaration (the declared return type would be AstNode). Then you would merge any MethodDeclarations it returned with the other methods after they've been transformed:

// transformedProperties is a List<AstNode?>
final transformedProperties = originalCompound.properties
    .map((property) => transformProperty(
          property,
          wrappedCompoundInstance,
          parentNamer,
          transformationMap,
        )).nonNulls.toList();

final transformedMethods = originalCompound.methods
    .map((method) => transformMethod(
          method,
          wrappedCompoundInstance,
          parentNamer,
          transformationMap,
        )).nonNulls.toList();

transformedCompound.properties =
  transformedProperties.whereType<PropertyDeclaration>()
  ..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));

transformedCompound.methods = (transformedMethods +
  transformedProperties.whereType<MethodDeclaration>())
  ..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));

The same goes for GlobalFunctionDeclaration.

List<PropertyDeclaration> properties,
) {
return properties
.where((property) => property.throws)
.map((property) => MethodDeclaration(
id: property.id,
name: property.name,
returnType: property.type,
params: [],
hasObjCAnnotation: true,
statements: property.getter?.statements ?? [],
isStatic: property.isStatic,
throws: property.throws,
async: property.async,
isCallingProperty: true,
))
.toList();
}
7 changes: 4 additions & 3 deletions pkgs/swift2objc/lib/src/transformer/transformers/transform_function.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ MethodDeclaration? transformMethod(
final methodSource = originalMethod.isStatic
? wrappedClassInstance.type.swiftType
: wrappedClassInstance.name;
return '$methodSource.${originalMethod.name}($arguments)';
return '$methodSource.${originalMethod.name}'
'${originalMethod.isCallingProperty ? '' : '($arguments)'}';
},
);
}
Expand All @@ -56,8 +57,8 @@ MethodDeclaration transformGlobalFunction(
wrapperMethodName: globalNamer.makeUnique(
'${globalFunction.name}Wrapper',
),
originalCallStatementGenerator: (arguments) =>
'${globalFunction.name}($arguments)',
originalCallStatementGenerator: (arguments) => '${globalFunction.name}'
'${globalFunction.isCallingProperty ? '' : '($arguments)'}',
);
}

Expand Down
34 changes: 26 additions & 8 deletions pkgs/swift2objc/lib/src/transformer/transformers/transform_globals.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ ClassDeclaration transformGlobals(
);

transformedGlobals.properties = globals.variables
.where((variable) => !variable.throws)
.map((variable) => transformGlobalVariable(
variable,
globalNamer,
Expand All @@ -30,14 +31,31 @@ ClassDeclaration transformGlobals(
.toList()
..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));

transformedGlobals.methods = globals.functions
.map((function) => transformGlobalFunction(
function,
globalNamer,
transformationMap,
))
.toList()
..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));
transformedGlobals.methods =
(globals.functions + _convertVariablesToFunctions(globals.variables))
.map((function) => transformGlobalFunction(
function,
globalNamer,
transformationMap,
))
.toList()
..sort((Declaration a, Declaration b) => a.id.compareTo(b.id));

return transformedGlobals;
}

List<GlobalFunctionDeclaration> _convertVariablesToFunctions(
List<GlobalVariableDeclaration> variables,
) {
return variables
.where((variable) => variable.throws)
.map((variable) => GlobalFunctionDeclaration(
id: variable.id,
name: variable.name,
params: [],
returnType: variable.type,
throws: variable.throws,
isCallingProperty: true,
))
.toList();
}
29 changes: 29 additions & 0 deletions pkgs/swift2objc/test/integration/throwing_getters_input.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import Foundation

public class MyClass {
public init(y: Int) throws {}

public var classGetter: MyClass {
get throws {
try MyClass(y: 3)
}
}
public var otherClassGetter: OtherClass {
get throws {
OtherClass()
}
}
}

public class OtherClass {}

public var globalClassGetter: MyClass {
get throws {
try MyClass(y: 4)
}
}
public var globalOtherClassGetter: OtherClass {
get throws {
OtherClass()
}
}
49 changes: 49 additions & 0 deletions pkgs/swift2objc/test/integration/throwing_getters_output.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Test preamble text

import Foundation

@objc public class GlobalsWrapper: NSObject {
@objc static public func globalClassGetterWrapper() throws -> MyClassWrapper {
let result = try globalClassGetter
return MyClassWrapper(result)
}

@objc static public func globalOtherClassGetterWrapper() throws -> OtherClassWrapper {
let result = try globalOtherClassGetter
return OtherClassWrapper(result)
}

}

@objc public class OtherClassWrapper: NSObject {
var wrappedInstance: OtherClass

init(_ wrappedInstance: OtherClass) {
self.wrappedInstance = wrappedInstance
}

}

@objc public class MyClassWrapper: NSObject {
var wrappedInstance: MyClass

init(_ wrappedInstance: MyClass) {
self.wrappedInstance = wrappedInstance
}

@objc init(y: Int) throws {
wrappedInstance = try MyClass(y: y)
}

@objc public func otherClassGetter() throws -> OtherClassWrapper {
let result = try wrappedInstance.otherClassGetter
return OtherClassWrapper(result)
}

@objc public func classGetter() throws -> MyClassWrapper {
let result = try wrappedInstance.classGetter
return MyClassWrapper(result)
}

}

Loading