From eb3f6c936ac339025e358eb9004851db88f45bdd Mon Sep 17 00:00:00 2001 From: Klemen Tusar Date: Thu, 16 Nov 2023 08:57:44 +0000 Subject: [PATCH] :lipstick: cosmetic refactor (#534) --- chopper/example/definition.chopper.dart | 3 +- chopper/test/test_service.chopper.dart | 3 +- .../test/test_service_base_url.chopper.dart | 3 +- .../test/test_service_variable.chopper.dart | 3 +- chopper_generator/lib/src/generator.dart | 170 ++++++++++-------- .../test/test_service.chopper.dart | 3 +- .../test/test_service_variable.chopper.dart | 3 +- 7 files changed, 106 insertions(+), 82 deletions(-) diff --git a/chopper/example/definition.chopper.dart b/chopper/example/definition.chopper.dart index 577547fe..4ee6e510 100644 --- a/chopper/example/definition.chopper.dart +++ b/chopper/example/definition.chopper.dart @@ -6,6 +6,7 @@ part of 'definition.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$MyService extends MyService { _$MyService([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$MyService extends MyService { } @override - final definitionType = MyService; + final Type definitionType = MyService; @override Future> getResource(String id) { diff --git a/chopper/test/test_service.chopper.dart b/chopper/test/test_service.chopper.dart index 028a7310..491d718c 100644 --- a/chopper/test/test_service.chopper.dart +++ b/chopper/test/test_service.chopper.dart @@ -6,6 +6,7 @@ part of 'test_service.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$HttpTestService extends HttpTestService { _$HttpTestService([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$HttpTestService extends HttpTestService { } @override - final definitionType = HttpTestService; + final Type definitionType = HttpTestService; @override Future> getTest( diff --git a/chopper/test/test_service_base_url.chopper.dart b/chopper/test/test_service_base_url.chopper.dart index 1b77e188..6b859b03 100644 --- a/chopper/test/test_service_base_url.chopper.dart +++ b/chopper/test/test_service_base_url.chopper.dart @@ -6,6 +6,7 @@ part of 'test_service_base_url.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$HttpTestServiceBaseUrl extends HttpTestServiceBaseUrl { _$HttpTestServiceBaseUrl([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$HttpTestServiceBaseUrl extends HttpTestServiceBaseUrl { } @override - final definitionType = HttpTestServiceBaseUrl; + final Type definitionType = HttpTestServiceBaseUrl; @override Future> getAll() { diff --git a/chopper/test/test_service_variable.chopper.dart b/chopper/test/test_service_variable.chopper.dart index 1f709494..e3611022 100644 --- a/chopper/test/test_service_variable.chopper.dart +++ b/chopper/test/test_service_variable.chopper.dart @@ -6,6 +6,7 @@ part of 'test_service_variable.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$HttpTestServiceVariable extends HttpTestServiceVariable { _$HttpTestServiceVariable([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$HttpTestServiceVariable extends HttpTestServiceVariable { } @override - final definitionType = HttpTestServiceVariable; + final Type definitionType = HttpTestServiceVariable; @override Future> getTest( diff --git a/chopper_generator/lib/src/generator.dart b/chopper_generator/lib/src/generator.dart index 007649bf..b8707caa 100644 --- a/chopper_generator/lib/src/generator.dart +++ b/chopper_generator/lib/src/generator.dart @@ -44,6 +44,7 @@ final class ChopperGenerator static Field _buildDefinitionTypeMethod(String superType) => Field( (method) => method ..annotations.add(refer('override')) + ..type = refer('Type') ..name = 'definitionType' ..modifier = FieldModifier.final$ ..assignment = Code(superType), @@ -69,12 +70,13 @@ final class ChopperGenerator TopLevelVariableElement? baseUrlVariableElement; - final VariableElement? posibleBaseUrl = baseUrlReader?.objectValue.variable; + final VariableElement? possibleBaseUrl = + baseUrlReader?.objectValue.variable; - if (posibleBaseUrl is TopLevelVariableElement && - posibleBaseUrl.type.isDartCoreString && - posibleBaseUrl.isConst) { - baseUrlVariableElement = posibleBaseUrl; + if (possibleBaseUrl is TopLevelVariableElement && + possibleBaseUrl.type.isDartCoreString && + possibleBaseUrl.isConst) { + baseUrlVariableElement = possibleBaseUrl; } final String baseUrl = baseUrlReader?.stringValue ?? ''; @@ -93,26 +95,32 @@ final class ChopperGenerator )); }); - const String ignore = '// ignore_for_file: type=lint'; - final DartEmitter emitter = DartEmitter(); + const String ignore = '// coverage:ignore-file\n' + '// ignore_for_file: type=lint'; + final DartEmitter emitter = DartEmitter(useNullSafetySyntax: true); return DartFormatter().format('$ignore\n${classBuilder.accept(emitter)}'); } static Constructor _generateConstructor() => Constructor( - (ConstructorBuilder constructorBuilder) { - constructorBuilder.optionalParameters.add( - Parameter((paramBuilder) { - paramBuilder.name = Vars.client.toString(); - paramBuilder.type = refer('${chopper.ChopperClient}?'); - }), - ); - - constructorBuilder.body = Code( - 'if (${Vars.client} == null) return;\n' - 'this.${Vars.client} = ${Vars.client};', - ); - }, + (ConstructorBuilder constructorBuilder) => constructorBuilder + ..optionalParameters.add( + Parameter( + (paramBuilder) => paramBuilder + ..name = Vars.client.toString() + ..type = TypeReference( + (typeBuilder) => typeBuilder + ..symbol = '${chopper.ChopperClient}' + ..isNullable = true, + ), + ), + ) + ..body = Code( + [ + 'if (${Vars.client} == null) return;', + 'this.${Vars.client} = ${Vars.client};' + ].join('\n'), + ), ); static Iterable _parseMethods( @@ -175,37 +183,33 @@ final class ChopperGenerator final DartType? responseInnerType = _getResponseInnerType(m.returnType) ?? responseType; - return Method((MethodBuilder b) { - b.annotations.add(refer('override')); - b.name = m.displayName; - - /// We don't support returning null Type - b.returns = Reference( - m.returnType.getDisplayString(withNullability: false), - ); - - /// And null Typed parameters - b.types.addAll( - m.typeParameters.map( - (t) => Reference(t.getDisplayString(withNullability: false)), - ), - ); - - b.requiredParameters.addAll( - m.parameters - .where((p) => p.isRequiredPositional) - .map(Utils.buildRequiredPositionalParam), - ); - - b.optionalParameters.addAll( - m.parameters - .where((p) => p.isOptionalPositional) - .map(Utils.buildOptionalPositionalParam), - ); - - b.optionalParameters.addAll( - m.parameters.where((p) => p.isNamed).map(Utils.buildNamedParam), - ); + return Method((MethodBuilder methodBuilder) { + methodBuilder + ..annotations.add(refer('override')) + ..name = m.displayName + // We don't support returning null Type + ..returns = refer( + m.returnType.getDisplayString(withNullability: false), + ) + // And null Typed parameters + ..types.addAll( + m.typeParameters.map( + (t) => refer(t.getDisplayString(withNullability: false)), + ), + ) + ..requiredParameters.addAll( + m.parameters + .where((p) => p.isRequiredPositional) + .map(Utils.buildRequiredPositionalParam), + ) + ..optionalParameters.addAll( + m.parameters + .where((p) => p.isOptionalPositional) + .map(Utils.buildOptionalPositionalParam), + ) + ..optionalParameters.addAll( + m.parameters.where((p) => p.isNamed).map(Utils.buildNamedParam), + ); final List blocks = [ declareFinal(Vars.url.toString(), type: refer('Uri')) @@ -231,14 +235,17 @@ final class ChopperGenerator final bool hasQueryMap = queryMap.isNotEmpty; if (hasQueryMap) { if (queries.isNotEmpty) { - blocks.add(refer('${Vars.parameters}.addAll').call( - [ - /// Check if the parameter is nullable - optionalNullableParameters.contains(queryMap.keys.first) - ? refer(queryMap.keys.first).ifNullThen(refer('const {}')) - : refer(queryMap.keys.first), - ], - ).statement); + blocks.add( + refer(Vars.parameters.toString()).property('addAll').call( + [ + /// Check if the parameter is nullable + if (optionalNullableParameters.contains(queryMap.keys.first)) + refer(queryMap.keys.first).ifNullThen(literalConstMap({})) + else + refer(queryMap.keys.first), + ], + ).statement, + ); } else { blocks.add( declareFinal( @@ -248,7 +255,8 @@ final class ChopperGenerator .assign( /// Check if the parameter is nullable optionalNullableParameters.contains(queryMap.keys.first) - ? refer(queryMap.keys.first).ifNullThen(refer('const {}')) + ? refer(queryMap.keys.first) + .ifNullThen(literalConstMap({})) : refer(queryMap.keys.first), ) .statement, @@ -285,9 +293,11 @@ final class ChopperGenerator final bool hasFieldMap = fieldMap.isNotEmpty; if (hasFieldMap) { if (hasBody) { - blocks.add(refer('${Vars.body}.addAll').call( - [refer(fieldMap.keys.first)], - ).statement); + blocks.add( + refer(Vars.body.toString()).property('addAll').call( + [refer(fieldMap.keys.first)], + ).statement, + ); } else { blocks.add( declareFinal(Vars.body.toString()) @@ -312,7 +322,7 @@ final class ChopperGenerator if (hasPartMap) { if (hasParts) { blocks.add( - refer('${Vars.parts}.addAll').call( + refer(Vars.parts.toString()).property('addAll').call( [refer(partMap.keys.first)], ).statement, ); @@ -329,7 +339,7 @@ final class ChopperGenerator if (hasFileFilesMap) { if (hasParts || hasPartMap) { blocks.add( - refer('${Vars.parts}.addAll').call( + refer(Vars.parts.toString()).property('addAll').call( [refer(fileFieldMap.keys.first)], ).statement, ); @@ -394,19 +404,25 @@ final class ChopperGenerator final List typeArguments = []; if (responseType != null) { - typeArguments - .add(refer(responseType.getDisplayString(withNullability: false))); - typeArguments.add( + typeArguments.addAll([ + refer(responseType.getDisplayString(withNullability: false)), refer(responseInnerType!.getDisplayString(withNullability: false)), - ); + ]); } - blocks.add(refer('${Vars.client}.send') - .call([refer(Vars.request.toString())], namedArguments, typeArguments) - .returned - .statement); + blocks.add( + refer(Vars.client.toString()) + .property('send') + .call( + [refer(Vars.request.toString())], + namedArguments, + typeArguments, + ) + .returned + .statement, + ); - b.body = Block.of(blocks); + methodBuilder.body = Block.of(blocks); }); } @@ -414,7 +430,9 @@ final class ChopperGenerator // ignore: deprecated_member_use function.enclosingElement is ClassElement // ignore: deprecated_member_use - ? '${function.enclosingElement!.name}.${function.name}' + ? refer(function.enclosingElement!.name!) + .property(function.name!) + .toString() : function.name!; static Map _getAnnotation( @@ -584,7 +602,7 @@ final class ChopperGenerator [ literal(Utils.getMethodName(method)), refer(Vars.url.toString()), - refer('${Vars.client}.${Vars.baseUrl}'), + refer(Vars.client.toString()).property(Vars.baseUrl.toString()), ], { if (hasBody) 'body': refer(Vars.body.toString()), diff --git a/chopper_generator/test/test_service.chopper.dart b/chopper_generator/test/test_service.chopper.dart index f070c8a8..2474ce6f 100644 --- a/chopper_generator/test/test_service.chopper.dart +++ b/chopper_generator/test/test_service.chopper.dart @@ -6,6 +6,7 @@ part of 'test_service.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$HttpTestService extends HttpTestService { _$HttpTestService([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$HttpTestService extends HttpTestService { } @override - final definitionType = HttpTestService; + final Type definitionType = HttpTestService; @override Future> getTest( diff --git a/chopper_generator/test/test_service_variable.chopper.dart b/chopper_generator/test/test_service_variable.chopper.dart index 1f709494..e3611022 100644 --- a/chopper_generator/test/test_service_variable.chopper.dart +++ b/chopper_generator/test/test_service_variable.chopper.dart @@ -6,6 +6,7 @@ part of 'test_service_variable.dart'; // ChopperGenerator // ************************************************************************** +// coverage:ignore-file // ignore_for_file: type=lint final class _$HttpTestServiceVariable extends HttpTestServiceVariable { _$HttpTestServiceVariable([ChopperClient? client]) { @@ -14,7 +15,7 @@ final class _$HttpTestServiceVariable extends HttpTestServiceVariable { } @override - final definitionType = HttpTestServiceVariable; + final Type definitionType = HttpTestServiceVariable; @override Future> getTest(