From c92b1567f8bb838256a6b50a52d9980823880025 Mon Sep 17 00:00:00 2001 From: Markus Richter <8398165+mqus@users.noreply.github.com> Date: Sun, 14 Mar 2021 22:36:49 +0100 Subject: [PATCH] (refactor) Pull out query method return type into own class --- .../lib/processor/dao_processor.dart | 4 +- .../error/query_method_processor_error.dart | 9 ++ .../lib/processor/query_method_processor.dart | 95 +++++++------------ .../lib/value_object/query_method.dart | 49 ++-------- .../query_method_return_type.dart | 63 ++++++++++++ .../lib/writer/query_method_writer.dart | 16 ++-- .../query_method_processor_test.dart | 46 +++++++-- 7 files changed, 160 insertions(+), 122 deletions(-) create mode 100644 floor_generator/lib/value_object/query_method_return_type.dart diff --git a/floor_generator/lib/processor/dao_processor.dart b/floor_generator/lib/processor/dao_processor.dart index 27f62acb..25ca5ff5 100644 --- a/floor_generator/lib/processor/dao_processor.dart +++ b/floor_generator/lib/processor/dao_processor.dart @@ -59,8 +59,8 @@ class DaoProcessor extends Processor { final transactionMethods = _getTransactionMethods(methods); final streamQueryables = queryMethods - .where((method) => method.returnsStream) - .map((method) => method.queryable); + .where((method) => method.returnType.isStream) + .map((method) => method.returnType.queryable); final streamEntities = streamQueryables.whereType().toSet(); final streamViews = streamQueryables.whereType().toSet(); diff --git a/floor_generator/lib/processor/error/query_method_processor_error.dart b/floor_generator/lib/processor/error/query_method_processor_error.dart index 2d587dfa..0705888f 100644 --- a/floor_generator/lib/processor/error/query_method_processor_error.dart +++ b/floor_generator/lib/processor/error/query_method_processor_error.dart @@ -33,6 +33,15 @@ class QueryMethodProcessorError { ); } + InvalidGenerationSourceError get doesNotReturnFutureVoid { + return InvalidGenerationSourceError( + 'A query returning `void` has to return exactly `Future`.', + todo: + 'Define the return type as `Future` or return a non-empty value.', + element: _methodElement, + ); + } + ProcessorError queryMethodParameterIsNullable( final ParameterElement parameterElement, ) { diff --git a/floor_generator/lib/processor/query_method_processor.dart b/floor_generator/lib/processor/query_method_processor.dart index cf0417d3..636bbada 100644 --- a/floor_generator/lib/processor/query_method_processor.dart +++ b/floor_generator/lib/processor/query_method_processor.dart @@ -11,6 +11,7 @@ import 'package:floor_generator/misc/type_utils.dart'; import 'package:floor_generator/processor/error/query_method_processor_error.dart'; import 'package:floor_generator/processor/processor.dart'; import 'package:floor_generator/value_object/query_method.dart'; +import 'package:floor_generator/value_object/query_method_return_type.dart'; import 'package:floor_generator/value_object/queryable.dart'; import 'package:floor_generator/value_object/type_converter.dart'; @@ -34,29 +35,9 @@ class QueryMethodProcessor extends Processor { QueryMethod process() { final name = _methodElement.displayName; final parameters = _methodElement.parameters; - final rawReturnType = _methodElement.returnType; + final returnType = _getAndCheckReturnType(); final query = _getQuery(); - final returnsStream = rawReturnType.isStream; - - _assertReturnsFutureOrStream(rawReturnType, returnsStream); - - final returnsList = _getReturnsList(rawReturnType, returnsStream); - final flattenedReturnType = _getFlattenedReturnType( - rawReturnType, - returnsStream, - returnsList, - ); - - _assertReturnsNullableSingle( - returnsStream, - returnsList, - flattenedReturnType, - ); - - final queryable = _queryables.firstWhereOrNull((queryable) => - queryable.classElement.displayName == - flattenedReturnType.getDisplayString(withNullability: false)); final parameterTypeConverters = parameters .expand((parameter) => @@ -67,20 +48,17 @@ class QueryMethodProcessor extends Processor { _methodElement.getTypeConverters(TypeConverterScope.daoMethod) + parameterTypeConverters; - if (queryable != null) { - final fieldTypeConverters = - queryable.fields.mapNotNull((field) => field.typeConverter); + if (returnType.queryable != null) { + final fieldTypeConverters = returnType.queryable!.fields + .mapNotNull((field) => field.typeConverter); allTypeConverters.addAll(fieldTypeConverters); } return QueryMethod( - _methodElement, name, query, - rawReturnType, - flattenedReturnType, parameters, - queryable, + returnType, allTypeConverters, ); } @@ -115,30 +93,8 @@ class QueryMethodProcessor extends Processor { ); } - DartType _getFlattenedReturnType( - final DartType rawReturnType, - final bool returnsStream, - final bool returnsList, - ) { - final type = returnsStream - ? _methodElement.returnType.flatten() - : _methodElement.library.typeSystem.flatten(rawReturnType); - return returnsList ? type.flatten() : type; - } - - bool _getReturnsList(final DartType returnType, final bool returnsStream) { - final type = returnsStream - ? returnType.flatten() - : _methodElement.library.typeSystem.flatten(returnType); - - return type.isDartCoreList; - } - - void _assertReturnsFutureOrStream( - final DartType rawReturnType, - final bool returnsStream, - ) { - if (!rawReturnType.isDartAsyncFuture && !returnsStream) { + void _assertReturnsFutureOrStream(final DartType rawType) { + if (!rawType.isDartAsyncFuture && !rawType.isStream) { throw _processorError.doesNotReturnFutureNorStream; } } @@ -159,19 +115,36 @@ class QueryMethodProcessor extends Processor { } } - void _assertReturnsNullableSingle( - final bool returnsStream, - final bool returnsList, - final DartType flattenedReturnType, - ) { - if (!returnsList && - !flattenedReturnType.isVoid && - !flattenedReturnType.isNullable) { - if (returnsStream) { + void _assertReturnsNullableSingle(QueryMethodReturnType returnType) { + if (!returnType.isList && + !returnType.isVoid && + !returnType.flat.isNullable) { + if (returnType.isStream) { throw _processorError.doesNotReturnNullableStream; } else { throw _processorError.doesNotReturnNullableFuture; } } } + + void _assertReturnsFutureOnVoid(QueryMethodReturnType returnType) { + if (returnType.isVoid && (returnType.isStream || returnType.isList)) { + throw _processorError.doesNotReturnFutureVoid; + } + } + + QueryMethodReturnType _getAndCheckReturnType() { + _assertReturnsFutureOrStream(_methodElement.returnType); + + final type = QueryMethodReturnType(_methodElement.returnType); + + _assertReturnsNullableSingle(type); + _assertReturnsFutureOnVoid(type); + + type.queryable = _queryables.firstWhereOrNull((queryable) => + queryable.classElement.displayName == + type.flat.getDisplayString(withNullability: false)); + + return type; + } } diff --git a/floor_generator/lib/value_object/query_method.dart b/floor_generator/lib/value_object/query_method.dart index 0a393bd6..d4fc9ec2 100644 --- a/floor_generator/lib/value_object/query_method.dart +++ b/floor_generator/lib/value_object/query_method.dart @@ -1,89 +1,52 @@ import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/type.dart'; import 'package:collection/collection.dart'; import 'package:floor_generator/misc/extension/set_equality_extension.dart'; -import 'package:floor_generator/misc/type_utils.dart'; -import 'package:floor_generator/value_object/queryable.dart'; +import 'package:floor_generator/value_object/query_method_return_type.dart'; import 'package:floor_generator/value_object/type_converter.dart'; /// Wraps a method annotated with Query /// to enable easy access to code generation relevant data. class QueryMethod { - final MethodElement methodElement; - final String name; /// Query where ':' got replaced with '$'. final String query; - final DartType rawReturnType; - - /// Flattened return type. - /// - /// E.g. - /// Future -> T, - /// Future> -> T - /// - /// Stream -> T - /// Stream> -> T - final DartType flattenedReturnType; + final QueryMethodReturnType returnType; final List parameters; - final Queryable? queryable; - final Set typeConverters; QueryMethod( - this.methodElement, this.name, this.query, - this.rawReturnType, - this.flattenedReturnType, this.parameters, - this.queryable, + this.returnType, this.typeConverters, ); - bool get returnsList { - final type = returnsStream - ? rawReturnType.flatten() - : methodElement.library.typeSystem.flatten(rawReturnType); - - return type.isDartCoreList; - } - - bool get returnsStream => rawReturnType.isStream; - - bool get returnsVoid => flattenedReturnType.isVoid; - @override bool operator ==(Object other) => identical(this, other) || other is QueryMethod && runtimeType == other.runtimeType && - methodElement == other.methodElement && name == other.name && query == other.query && - rawReturnType == other.rawReturnType && - flattenedReturnType == other.flattenedReturnType && parameters.equals(other.parameters) && - queryable == other.queryable && + returnType == other.returnType && typeConverters.equals(other.typeConverters); @override int get hashCode => - methodElement.hashCode ^ name.hashCode ^ query.hashCode ^ - rawReturnType.hashCode ^ - flattenedReturnType.hashCode ^ parameters.hashCode ^ - queryable.hashCode ^ + returnType.hashCode ^ typeConverters.hashCode; @override String toString() { - return 'QueryMethod{methodElement: $methodElement, name: $name, query: $query, rawReturnType: $rawReturnType, flattenedReturnType: $flattenedReturnType, parameters: $parameters, queryable: $queryable, typeConverters: $typeConverters}'; + return 'QueryMethod{name: $name, query: $query, parameters: $parameters, returnType: $returnType, typeConverters: $typeConverters}'; } } diff --git a/floor_generator/lib/value_object/query_method_return_type.dart b/floor_generator/lib/value_object/query_method_return_type.dart new file mode 100644 index 00000000..33de303a --- /dev/null +++ b/floor_generator/lib/value_object/query_method_return_type.dart @@ -0,0 +1,63 @@ +import 'package:floor_generator/misc/type_utils.dart'; +import 'package:analyzer/dart/element/type.dart'; +import 'package:floor_generator/value_object/queryable.dart'; + +/// A simple accessor class for providing all properties of +/// the return type of a query method +class QueryMethodReturnType { + final DartType raw; + + late Queryable? queryable; + + // the following attributes are derived on construction and stored. + final bool isStream; + final bool isFuture; + final bool isList; + + /// Flattened return type + /// + /// E.g. + /// Future -> T + /// Stream> -> T + final DartType flat; + + bool get isVoid => flat.isVoid; + bool get isPrimitive => + flat.isVoid || + flat.isDartCoreDouble || + flat.isDartCoreInt || + flat.isDartCoreBool || + flat.isDartCoreString || + flat.isUint8List; + + QueryMethodReturnType(this.raw) + : isStream = raw.isStream, + isFuture = raw.isDartAsyncFuture, + isList = raw.flatten().isDartCoreList, + flat = _flattenWithList(raw); + + @override + bool operator ==(Object other) => + identical(this, other) || + other is QueryMethodReturnType && + runtimeType == other.runtimeType && + raw == other.raw && + queryable == other.queryable; + + @override + int get hashCode => raw.hashCode ^ queryable.hashCode; + + @override + String toString() { + return 'QueryMethodReturnType{raw: $raw, flat: $flat, queryable: $queryable}'; + } + + static DartType _flattenWithList(DartType raw) { + final flattenedOnce = raw.flatten(); + if (flattenedOnce.isDartCoreList) { + return flattenedOnce.flatten(); + } else { + return flattenedOnce; + } + } +} diff --git a/floor_generator/lib/writer/query_method_writer.dart b/floor_generator/lib/writer/query_method_writer.dart index a7e86bee..46d48b46 100644 --- a/floor_generator/lib/writer/query_method_writer.dart +++ b/floor_generator/lib/writer/query_method_writer.dart @@ -20,14 +20,14 @@ class QueryMethodWriter implements Writer { Method write() { final builder = MethodBuilder() ..annotations.add(overrideAnnotationExpression) - ..returns = refer(_queryMethod.rawReturnType.getDisplayString( + ..returns = refer(_queryMethod.returnType.raw.getDisplayString( withNullability: true, )) ..name = _queryMethod.name ..requiredParameters.addAll(_generateMethodParameters()) ..body = Code(_generateMethodBody()); - if (!_queryMethod.returnsStream || _queryMethod.returnsVoid) { + if (!_queryMethod.returnType.isStream || _queryMethod.returnType.isVoid) { builder..modifier = MethodModifier.async; } return builder.build(); @@ -54,9 +54,9 @@ class QueryMethodWriter implements Writer { } final arguments = _generateArguments(); - final queryable = _queryMethod.queryable; + final queryable = _queryMethod.returnType.queryable; // null queryable implies void-returning query method - if (_queryMethod.returnsVoid || queryable == null) { + if (_queryMethod.returnType.isVoid || queryable == null) { _methodBody.write(_generateNoReturnQuery(arguments)); return _methodBody.toString(); } @@ -64,7 +64,7 @@ class QueryMethodWriter implements Writer { final constructor = queryable.constructor; final mapper = '(Map row) => $constructor'; - if (_queryMethod.returnsStream) { + if (_queryMethod.returnType.isStream) { _methodBody.write(_generateStreamQuery(arguments, mapper)); } else { _methodBody.write(_generateQuery(arguments, mapper)); @@ -129,7 +129,7 @@ class QueryMethodWriter implements Writer { if (arguments != null) parameters.write('arguments: $arguments, '); parameters.write('mapper: $mapper'); - if (_queryMethod.returnsList) { + if (_queryMethod.returnType.isList) { return 'return _queryAdapter.queryList($parameters);'; } else { return 'return _queryAdapter.query($parameters);'; @@ -140,7 +140,7 @@ class QueryMethodWriter implements Writer { final String? arguments, final String mapper, ) { - final queryable = _queryMethod.queryable; + final queryable = _queryMethod.returnType.queryable; // can't be null as validated before if (queryable == null) throw ArgumentError.notNull(); @@ -153,7 +153,7 @@ class QueryMethodWriter implements Writer { ..write('isView: $isView, ') ..write('mapper: $mapper'); - if (_queryMethod.returnsList) { + if (_queryMethod.returnType.isList) { return 'return _queryAdapter.queryListStream($parameters);'; } else { return 'return _queryAdapter.queryStream($parameters);'; diff --git a/floor_generator/test/processor/query_method_processor_test.dart b/floor_generator/test/processor/query_method_processor_test.dart index 07103606..e1e93f65 100644 --- a/floor_generator/test/processor/query_method_processor_test.dart +++ b/floor_generator/test/processor/query_method_processor_test.dart @@ -9,6 +9,7 @@ import 'package:floor_generator/processor/query_method_processor.dart'; import 'package:floor_generator/processor/view_processor.dart'; import 'package:floor_generator/value_object/entity.dart'; import 'package:floor_generator/value_object/query_method.dart'; +import 'package:floor_generator/value_object/query_method_return_type.dart'; import 'package:floor_generator/value_object/view.dart'; import 'package:source_gen/source_gen.dart'; import 'package:test/test.dart'; @@ -38,13 +39,12 @@ void main() { actual, equals( QueryMethod( - methodElement, 'findAllPersons', 'SELECT * FROM Person', - await getDartTypeWithPerson('Future>'), - await getDartTypeWithPerson('Person'), [], - entities.first, + QueryMethodReturnType( + await getDartTypeWithPerson('Future>')) + ..queryable = entities.first, {}, ), ), @@ -65,13 +65,11 @@ void main() { actual, equals( QueryMethod( - methodElement, 'findAllNames', 'SELECT * FROM name', - await getDartTypeWithName('Future>'), - await getDartTypeWithName('Name'), [], - views.first, + QueryMethodReturnType(await getDartTypeWithName('Future>')) + ..queryable = views.first, {}, ), ), @@ -247,6 +245,38 @@ void main() { expect(actual, throwsInvalidGenerationSourceError(error)); }); + test('exception when method does not return Future for void', + () async { + final methodElement = await _createQueryMethodElement(''' + @Query('SELECT * FROM Person') + Future> findAllPersons(); + '''); + + final actual = () => + QueryMethodProcessor(methodElement, [...entities, ...views], {}) + .process(); + + final error = + QueryMethodProcessorError(methodElement).doesNotReturnFutureVoid; + expect(actual, throwsInvalidGenerationSourceError(error)); + }); + + test('exception when method does not return Future for void', + () async { + final methodElement = await _createQueryMethodElement(''' + @Query('SELECT * FROM Person') + Stream findAllPersons(); + '''); + + final actual = () => + QueryMethodProcessor(methodElement, [...entities, ...views], {}) + .process(); + + final error = + QueryMethodProcessorError(methodElement).doesNotReturnFutureVoid; + expect(actual, throwsInvalidGenerationSourceError(error)); + }); + test('exception when query is empty string', () async { final methodElement = await _createQueryMethodElement(''' @Query('')