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

Jump to bottom

Bugfix/nullable transaction return #559

Merged
merged 5 commits into from
May 2, 2021
Merged

Bugfix/nullable transaction return #559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
322ed22
Fix nullable transaction parameters and returns
mqus May 1, 2021
8d49080
Fix nullable casts in queryable constructor
mqus May 1, 2021
6770c46
Add one more test for nullable transaction returns
mqus May 2, 2021
b1002de
Add more tests for nullable transaction returns and parameters
mqus May 2, 2021
edda3a8
Merge branch 'develop' into bugfix/nullable-transaction-return
vitusortner May 2, 2021
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
25 changes: 9 additions & 16 deletions floor_generator/lib/processor/queryable_processor.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:collection/collection.dart';
import 'package:floor_annotation/floor_annotation.dart' as annotations;
import 'package:floor_generator/misc/extension/dart_type_extension.dart';
import 'package:floor_generator/misc/extension/set_extension.dart';
import 'package:floor_generator/misc/extension/string_extension.dart';
import 'package:floor_generator/misc/extension/type_converter_element_extension.dart';
Expand Down Expand Up @@ -79,7 +80,6 @@ abstract class QueryableProcessor<T extends Queryable> extends Processor<T> {
if (parameterElement.type.isDefaultSqlType) {
parameterValue = databaseValue.cast(
parameterElement.type,
field.isNullable,
parameterElement,
);
} else {
Expand All @@ -88,7 +88,6 @@ abstract class QueryableProcessor<T extends Queryable> extends Processor<T> {
.getClosest(parameterElement.type);
final castedDatabaseValue = databaseValue.cast(
typeConverter.databaseType,
field.isNullable,
parameterElement,
);

Expand All @@ -107,27 +106,21 @@ abstract class QueryableProcessor<T extends Queryable> extends Processor<T> {
}

extension on String {
String cast(
DartType dartType,
bool isNullable,
ParameterElement parameterElement,
) {
String cast(DartType dartType, ParameterElement parameterElement) {
if (dartType.isDartCoreBool) {
if (isNullable) {
if (dartType.isNullable) {
// if the value is null, return null
// if the value is not null, interpret 1 as true and 0 as false
return '$this == null ? null : ($this as int) != 0';
} else {
return '($this as int) != 0';
}
} else if (dartType.isDartCoreString) {
return '$this as String${isNullable ? '?' : ''}';
} else if (dartType.isDartCoreInt) {
return '$this as int${isNullable ? '?' : ''}';
} else if (dartType.isUint8List) {
return '$this as Uint8List${isNullable ? '?' : ''}';
} else if (dartType.isDartCoreDouble) {
return '$this as double${isNullable ? '?' : ''}';
} else if (dartType.isDartCoreString ||
dartType.isDartCoreInt ||
dartType.isUint8List ||
dartType.isDartCoreDouble) {
final typeString = dartType.getDisplayString(withNullability: true);
return '$this as $typeString';
} else {
throw InvalidGenerationSourceError(
'Trying to convert unsupported type $dartType.',
Expand Down
4 changes: 2 additions & 2 deletions floor_generator/lib/writer/transaction_method_writer.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class TransactionMethodWriter implements Writer {
return Method((builder) => builder
..annotations.add(overrideAnnotationExpression)
..returns = refer(method.returnType.getDisplayString(
withNullability: false,
withNullability: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ Do we also get support for the case described in #557 where we don't return a nullable Future but a Future wrapping a nullable type (e.g Future<Task?>)? If so, could you add another test to verify the behavior?

Copy link
Collaborator Author

@mqus mqus May 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think we had that support already, but I'll add a test.

))
..name = method.name
..requiredParameters.addAll(_generateParameters())
Expand Down Expand Up @@ -47,7 +47,7 @@ class TransactionMethodWriter implements Writer {
return Parameter((builder) => builder
..name = parameter.name
..type = refer(parameter.type.getDisplayString(
withNullability: false,
withNullability: true,
)));
}).toList();
}
Expand Down
82 changes: 82 additions & 0 deletions floor_generator/test/writer/transaction_method_writer_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,88 @@ void main() {
}
'''));
});

test('Generate transaction method with nullable future return', () async {
final transactionMethod = await _createTransactionMethod('''
@transaction
Future<Person>? replacePersons(List<Person> persons) async {
return null;
}
''');

final actual = TransactionMethodWriter(transactionMethod).write();

expect(actual, equalsDart(r'''
@override
Future<Person>? replacePersons(List<Person> persons) async {
if (database is sqflite.Transaction) {
return super.replacePersons(persons);
} else {
return (database as sqflite.Database)
.transaction<Person>((transaction) async {
final transactionDatabase = _$TestDatabase(changeListener)
..database = transaction;
return transactionDatabase.personDao.replacePersons(persons);
});
}
}
'''));
});

test('Generate transaction method with nullable return type parameter',
() async {
final transactionMethod = await _createTransactionMethod('''
@transaction
Future<Person?> replacePersons(List<Person> persons) async {
return null;
}
''');

final actual = TransactionMethodWriter(transactionMethod).write();

expect(actual, equalsDart(r'''
@override
Future<Person?> replacePersons(List<Person> persons) async {
if (database is sqflite.Transaction) {
return super.replacePersons(persons);
} else {
return (database as sqflite.Database)
.transaction<Person>((transaction) async {
final transactionDatabase = _$TestDatabase(changeListener)
..database = transaction;
return transactionDatabase.personDao.replacePersons(persons);
});
}
}
'''));
});

test('Generate transaction method with nullable parameter', () async {
final transactionMethod = await _createTransactionMethod('''
@transaction
Future<Person>? replacePersons(Person? person) async {
return null;
}
''');

final actual = TransactionMethodWriter(transactionMethod).write();

expect(actual, equalsDart(r'''
@override
Future<Person>? replacePersons(Person? person) async {
if (database is sqflite.Transaction) {
return super.replacePersons(person);
} else {
return (database as sqflite.Database)
.transaction<Person>((transaction) async {
final transactionDatabase = _$TestDatabase(changeListener)
..database = transaction;
return transactionDatabase.personDao.replacePersons(person);
});
}
}
'''));
});
}

Future<TransactionMethod> _createTransactionMethod(
Expand Down