Skip to content

Commit

Permalink
Merge pull request #1367 from nextcloud/refactor/dynamite/output-high…
Browse files Browse the repository at this point in the history
…er-quality-code

refactor(dynamite): only use raw strings where necessary
  • Loading branch information
Leptopoda authored Dec 28, 2023
2 parents 003b151 + d4cfeda commit b946014
Show file tree
Hide file tree
Showing 16 changed files with 74 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import 'package:dynamite/src/helpers/dart_helpers.dart';
import 'package:dynamite/src/helpers/dynamite.dart';
import 'package:dynamite/src/models/openapi.dart' as openapi;
import 'package:dynamite/src/models/type_result.dart';
import 'package:source_helper/source_helper.dart';

Spec buildBuiltClassSerializer(
final State state,
Expand Down Expand Up @@ -38,7 +39,7 @@ Spec buildBuiltClassSerializer(
..lambda = true
..returns = refer('String')
..annotations.add(refer('override'))
..body = Code("r'$identifier'"),
..body = Code(escapeDartString(identifier)),
),
Method((final b) {
b
Expand Down
12 changes: 8 additions & 4 deletions packages/dynamite/dynamite/lib/src/builder/client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import 'package:dynamite/src/helpers/pattern_check.dart';
import 'package:dynamite/src/models/openapi.dart' as openapi;
import 'package:dynamite/src/models/type_result.dart';
import 'package:intersperse/intersperse.dart';
import 'package:source_helper/source_helper.dart';
import 'package:uri/uri.dart';

Iterable<Class> generateClients(
Expand Down Expand Up @@ -472,15 +473,18 @@ String buildParameterSerialization(
final $default = parameter.schema?.$default;
var defaultValueCode = $default?.value;
if ($default != null && $default.isString) {
defaultValueCode = "'${$default.asString}'";
defaultValueCode = escapeDartString($default.asString);
}
final dartName = toDartName(parameter.name);
final serializedName = '\$$dartName';

final buffer = StringBuffer()..write('var $serializedName = ${result.serialize(dartName)};');
final buffer = StringBuffer();

if ($default != null) {
buffer.writeln('$serializedName ??= $defaultValueCode;');
buffer
..write('var $serializedName = ${result.serialize(dartName)};')
..writeln('$serializedName ??= $defaultValueCode;');
} else {
buffer.write('final $serializedName = ${result.serialize(dartName)};');
}

if (parameter.schema != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ Iterable<Spec> generateSomeOf(
..type = MethodType.getter
..name = 'wireName'
..lambda = true
..body = Code("r'$identifier'"),
..body = Code(escapeDartString(identifier)),
),
Method((final b) {
b
Expand Down
7 changes: 4 additions & 3 deletions packages/dynamite/dynamite/lib/src/builder/resolve_enum.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:dynamite/src/helpers/built_value.dart';
import 'package:dynamite/src/helpers/dart_helpers.dart';
import 'package:dynamite/src/models/openapi.dart' as openapi;
import 'package:dynamite/src/models/type_result.dart';
import 'package:source_helper/source_helper.dart';

TypeResult resolveEnum(
final openapi.OpenAPI spec,
Expand All @@ -22,7 +23,7 @@ TypeResult resolveEnum(
final dartName = toDartName(name);
var value = jsonEncode(enumValue.value);
if (enumValue.isString) {
value = 'r$value';
value = escapeDartString(enumValue.asString);
}

values.add((dartName: dartName, value: value, name: name));
Expand Down Expand Up @@ -62,7 +63,7 @@ TypeResult resolveEnum(
if (enumValue.name != enumValue.dartName) {
b.annotations.add(
refer('BuiltValueEnumConst').call([], {
'wireName': refer("r'${enumValue.name}'"),
'wireName': refer(escapeDartString(enumValue.name)),
}),
);
}
Expand Down Expand Up @@ -169,7 +170,7 @@ TypeResult resolveEnum(
..type = MethodType.getter
..returns = refer('String')
..annotations.add(refer('override'))
..body = Code("r'$identifier'"),
..body = Code(escapeDartString(identifier)),
),
Method((final b) {
b
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Iterable<String> resolveMimeTypeEncode(
if (dartParameterNullable) {
yield 'if ($parameterName != null) {';
}
yield '_body = utf8.encode(${result.encode(parameterName, mimeType: mimeType)}) as Uint8List;';
yield '_body = utf8.encode(${result.encode(parameterName, mimeType: mimeType)});';
if (dartParameterNullable) {
yield '}';
}
Expand Down
14 changes: 9 additions & 5 deletions packages/dynamite/dynamite/lib/src/builder/serializer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,16 @@ List<Spec> buildSerializer(final State state) => [
final serializers =
state.resolvedTypes.map((final type) => type.serializers).expand((final element) => element).toSet();

final bodyBuilder = StringBuffer()
..writeln('(Serializers().toBuilder()')
..writeAll(serializers, '\n')
..writeln(').build()');
if (serializers.isEmpty) {
b.assignment = const Code('Serializers()');
} else {
final bodyBuilder = StringBuffer()
..writeln('(Serializers().toBuilder()')
..writeAll(serializers, '\n')
..writeln(').build()');

b.assignment = Code(bodyBuilder.toString());
b.assignment = Code(bodyBuilder.toString());
}
}),
Field((final b) {
b
Expand Down
6 changes: 5 additions & 1 deletion packages/dynamite/dynamite/lib/src/helpers/built_value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -100,14 +100,18 @@ Method get toJsonMethod => Method(
..body = const Code('jsonSerializers.serializeWith(serializer, this)! as Map<String, dynamic>'),
);

/// Builds the serializer getter for a built class.
///
/// If the serializer [isCustom] the getter is annotated to signal this to the built_value_generator.
/// It is assumed that custom serializers have a const constructor.
Method buildSerializer(final String className, {final bool isCustom = false}) => Method((final b) {
b
..name = 'serializer'
..returns = refer('Serializer<$className>')
..lambda = true
..static = true
..body = Code(
isCustom ? '_\$${className}Serializer()' : '_\$${toCamelCase(className)}Serializer',
isCustom ? 'const _\$${className}Serializer()' : '_\$${toCamelCase(className)}Serializer',
)
..type = MethodType.getter;
if (isCustom) {
Expand Down
3 changes: 2 additions & 1 deletion packages/dynamite/dynamite/lib/src/helpers/type_result.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import 'package:dynamite/src/helpers/dart_helpers.dart';
import 'package:dynamite/src/models/type_result.dart';
import 'package:source_helper/source_helper.dart';

/// Escapes a [value] using the type specific syntax from [result].
///
/// Use [forceString] to ensure the returned value is a `String`.
String valueToEscapedValue(final TypeResult result, final String value, {final bool forceString = false}) {
if ((result is TypeResultBase && result.name == 'String') || forceString) {
return "'$value'";
return escapeDartString(value);
}
if (result is TypeResultList) {
return 'const <${result.subType.name}>$value';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ class TypeResultBase extends TypeResult {
case 'Uint8List':
return object;
case 'String':
return '(utf8.encode($object) as Uint8List)';
return 'utf8.encode($object)';
default:
throw Exception('"$mimeType" can only be Uint8List or String');
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class TypeResultAnyOf extends TypeResultSomeOf {

@override
String deserialize(final String object, [final String? serializerName]) =>
'(${super.deserialize(object, serializerName)}..validateAnyOf())';
'(${super.deserialize(object, serializerName)})..validateAnyOf()';

@override
bool operator ==(final Object other) =>
Expand All @@ -143,7 +143,7 @@ class TypeResultOneOf extends TypeResultSomeOf {

@override
String deserialize(final String object, [final String? serializerName]) =>
'(${super.deserialize(object, serializerName)}..validateOneOf())';
'(${super.deserialize(object, serializerName)})..validateOneOf()';

@override
bool operator ==(final Object other) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,12 @@ sealed class TypeResult {
}

@nonVirtual
String get fullType {
String get fullType => 'const $_fullType';

String get _fullType {
if (generics.isNotEmpty) {
final buffer = StringBuffer('FullType($className, [')
..writeAll(generics.map((final c) => c.fullType), ', ')
..writeAll(generics.map((final c) => c._fullType), ', ')
..write('])');

return buffer.toString();
Expand Down Expand Up @@ -96,7 +98,7 @@ sealed class TypeResult {
/// The serialized result is an [Object]?
@mustCallSuper
String serialize(final String object, [final String? serializerName]) =>
'${serializerName ?? 'jsonSerializers'}.serialize($object, specifiedType: const $fullType)';
'${serializerName ?? 'jsonSerializers'}.serialize($object, specifiedType: $fullType)';

/// Deserializes the variable named [object].
///
Expand All @@ -107,13 +109,13 @@ sealed class TypeResult {
..write(serializerName ?? 'jsonSerializers')
..write('.deserialize(')
..write(object)
..write(', specifiedType: const $fullType)');
..write(', specifiedType: $fullType)');

if (!nullable) {
buffer.write('!');
}

return '($buffer as $name)';
return '$buffer as $name';
}

/// Decodes the variable named [object].
Expand Down
44 changes: 22 additions & 22 deletions packages/dynamite/dynamite/test/type_result_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@ void main() {
final type = TypeResultList('BuiltList', subType);

expect(type.name, 'BuiltList<String>');
expect(type.fullType, 'FullType(BuiltList, [FullType(String)])');
expect(type.fullType, 'const FullType(BuiltList, [FullType(String)])');
expect(
type.serializers.toList(),
const ['..addBuilderFactory(FullType(BuiltList, [FullType(String)]), ListBuilder<String>.new)'],
const ['..addBuilderFactory(const FullType(BuiltList, [FullType(String)]), ListBuilder<String>.new)'],
);
expect(
type.serialize('value'),
'jsonSerializers.serialize(value, specifiedType: const FullType(BuiltList, [FullType(String)]))',
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltList, [FullType(String)]))! as BuiltList<String>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltList, [FullType(String)]))! as BuiltList<String>',
);
});

Expand All @@ -30,12 +30,12 @@ void main() {
type = TypeResultList('BuiltList', type);

expect(type.name, 'BuiltList<BuiltList<String>>');
expect(type.fullType, 'FullType(BuiltList, [FullType(BuiltList, [FullType(String)])])');
expect(type.fullType, 'const FullType(BuiltList, [FullType(BuiltList, [FullType(String)])])');
expect(
type.serializers.toList(),
const [
'..addBuilderFactory(FullType(BuiltList, [FullType(String)]), ListBuilder<String>.new)',
'..addBuilderFactory(FullType(BuiltList, [FullType(BuiltList, [FullType(String)])]), ListBuilder<BuiltList<String>>.new)',
'..addBuilderFactory(const FullType(BuiltList, [FullType(String)]), ListBuilder<String>.new)',
'..addBuilderFactory(const FullType(BuiltList, [FullType(BuiltList, [FullType(String)])]), ListBuilder<BuiltList<String>>.new)',
],
);
expect(
Expand All @@ -44,7 +44,7 @@ void main() {
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltList, [FullType(BuiltList, [FullType(String)])]))! as BuiltList<BuiltList<String>>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltList, [FullType(BuiltList, [FullType(String)])]))! as BuiltList<BuiltList<String>>',
);
});

Expand All @@ -66,11 +66,11 @@ void main() {
final type = TypeResultMap('BuiltMap', subType);

expect(type.name, 'BuiltMap<String, int>');
expect(type.fullType, 'FullType(BuiltMap, [FullType(String), FullType(int)])');
expect(type.fullType, 'const FullType(BuiltMap, [FullType(String), FullType(int)])');
expect(
type.serializers.toList(),
const [
'..addBuilderFactory(FullType(BuiltMap, [FullType(String), FullType(int)]), MapBuilder<String, int>.new)',
'..addBuilderFactory(const FullType(BuiltMap, [FullType(String), FullType(int)]), MapBuilder<String, int>.new)',
],
);
expect(
Expand All @@ -79,7 +79,7 @@ void main() {
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltMap, [FullType(String), FullType(int)]))! as BuiltMap<String, int>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltMap, [FullType(String), FullType(int)]))! as BuiltMap<String, int>',
);
});

Expand All @@ -91,13 +91,13 @@ void main() {
expect(type.name, 'BuiltMap<String, BuiltMap<String, int>>');
expect(
type.fullType,
'FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])])',
'const FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])])',
);
expect(
type.serializers.toList(),
const [
'..addBuilderFactory(FullType(BuiltMap, [FullType(String), FullType(int)]), MapBuilder<String, int>.new)',
'..addBuilderFactory(FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])]), MapBuilder<String, BuiltMap<String, int>>.new)',
'..addBuilderFactory(const FullType(BuiltMap, [FullType(String), FullType(int)]), MapBuilder<String, int>.new)',
'..addBuilderFactory(const FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])]), MapBuilder<String, BuiltMap<String, int>>.new)',
],
);
expect(
Expand All @@ -106,7 +106,7 @@ void main() {
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])]))! as BuiltMap<String, BuiltMap<String, int>>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(BuiltMap, [FullType(String), FullType(BuiltMap, [FullType(String), FullType(int)])]))! as BuiltMap<String, BuiltMap<String, int>>',
);
});

Expand All @@ -128,11 +128,11 @@ void main() {
final type = TypeResultObject('CustomType', generics: BuiltList([subType]));

expect(type.name, 'CustomType<String>');
expect(type.fullType, 'FullType(CustomType, [FullType(String)])');
expect(type.fullType, 'const FullType(CustomType, [FullType(String)])');
expect(
type.serializers.toList(),
const [
'..addBuilderFactory(FullType(CustomType, [FullType(String)]), CustomTypeBuilder<String>.new)',
'..addBuilderFactory(const FullType(CustomType, [FullType(String)]), CustomTypeBuilder<String>.new)',
'..add(CustomType.serializer)',
],
);
Expand All @@ -142,7 +142,7 @@ void main() {
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(CustomType, [FullType(String)]))! as CustomType<String>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(CustomType, [FullType(String)]))! as CustomType<String>',
);
});

Expand All @@ -151,9 +151,9 @@ void main() {
final type = TypeResultObject('ContentString', generics: BuiltList([subType]));

expect(type.name, 'ContentString<int>');
expect(type.fullType, 'FullType(ContentString, [FullType(int)])');
expect(type.fullType, 'const FullType(ContentString, [FullType(int)])');
expect(type.serializers.toList(), const [
'..addBuilderFactory(FullType(ContentString, [FullType(int)]), ContentStringBuilder<int>.new)',
'..addBuilderFactory(const FullType(ContentString, [FullType(int)]), ContentStringBuilder<int>.new)',
'..add(ContentString.serializer)',
]);
expect(
Expand All @@ -162,7 +162,7 @@ void main() {
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(ContentString, [FullType(int)]))! as ContentString<int>)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(ContentString, [FullType(int)]))! as ContentString<int>',
);
});

Expand All @@ -183,15 +183,15 @@ void main() {
final type = TypeResultBase('String');

expect(type.name, 'String');
expect(type.fullType, 'FullType(String)');
expect(type.fullType, 'const FullType(String)');
expect(type.serializers.toList(), const <String>[]);
expect(
type.serialize('value'),
'jsonSerializers.serialize(value, specifiedType: const FullType(String))',
);
expect(
type.deserialize('value'),
'(jsonSerializers.deserialize(value, specifiedType: const FullType(String))! as String)',
'jsonSerializers.deserialize(value, specifiedType: const FullType(String))! as String',
);
});

Expand Down
Loading

0 comments on commit b946014

Please sign in to comment.