From 417d1f59d97bb8d21ca2989fc4c28522b181a3c8 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Sun, 10 Dec 2023 14:23:19 +0100 Subject: [PATCH] Fix generating variables with custom types --- drift/test/generated/custom_tables.g.dart | 31 ++++++-------- drift/test/generated/todos.g.dart | 34 +++++++-------- drift_dev/CHANGELOG.md | 5 +++ drift_dev/lib/src/analysis/results/dart.dart | 2 + .../lib/src/writer/queries/query_writer.dart | 39 ++++++------------ .../src/writer/tables/data_class_writer.dart | 27 ++---------- .../tables/update_companion_writer.dart | 29 ++----------- drift_dev/lib/src/writer/writer.dart | 41 +++++++++++++++++-- drift_dev/pubspec.yaml | 2 +- extras/drift_postgres/example/main.dart | 5 +++ extras/drift_postgres/example/main.g.dart | 4 +- 11 files changed, 101 insertions(+), 118 deletions(-) diff --git a/drift/test/generated/custom_tables.g.dart b/drift/test/generated/custom_tables.g.dart index 281de5295..ef1c2d0ab 100644 --- a/drift/test/generated/custom_tables.g.dart +++ b/drift/test/generated/custom_tables.g.dart @@ -167,7 +167,7 @@ class WithDefault extends DataClass implements Insertable { Map toColumns(bool nullToAbsent) { final map = {}; if (!nullToAbsent || a != null) { - map['a'] = Variable(a); + map['a'] = Variable(a, const CustomTextType()); } if (!nullToAbsent || b != null) { map['b'] = Variable(b); @@ -645,13 +645,12 @@ class Config extends DataClass implements Insertable { map['config_value'] = Variable(configValue); } if (!nullToAbsent || syncState != null) { - final converter = ConfigTable.$convertersyncStaten; - map['sync_state'] = Variable(converter.toSql(syncState)); + map['sync_state'] = + Variable(ConfigTable.$convertersyncStaten.toSql(syncState)); } if (!nullToAbsent || syncStateImplicit != null) { - final converter = ConfigTable.$convertersyncStateImplicitn; - map['sync_state_implicit'] = - Variable(converter.toSql(syncStateImplicit)); + map['sync_state_implicit'] = Variable( + ConfigTable.$convertersyncStateImplicitn.toSql(syncStateImplicit)); } return map; } @@ -796,15 +795,13 @@ class ConfigCompanion extends UpdateCompanion { map['config_value'] = Variable(configValue.value); } if (syncState.present) { - final converter = ConfigTable.$convertersyncStaten; - - map['sync_state'] = Variable(converter.toSql(syncState.value)); + map['sync_state'] = Variable( + ConfigTable.$convertersyncStaten.toSql(syncState.value)); } if (syncStateImplicit.present) { - final converter = ConfigTable.$convertersyncStateImplicitn; - - map['sync_state_implicit'] = - Variable(converter.toSql(syncStateImplicit.value)); + map['sync_state_implicit'] = Variable(ConfigTable + .$convertersyncStateImplicitn + .toSql(syncStateImplicit.value)); } if (rowid.present) { map['rowid'] = Variable(rowid.value); @@ -1747,12 +1744,10 @@ abstract class _$CustomTablesDb extends GeneratedDatabase { return customSelect( 'SELECT config_key FROM config WHERE ${generatedpred.sql} AND(sync_state = ?1 OR sync_state_implicit IN ($expandedvar2))', variables: [ - Variable(NullAwareTypeConverter.wrapToSql( - ConfigTable.$convertersyncState, var1)), + Variable(ConfigTable.$convertersyncStaten.toSql(var1)), ...generatedpred.introducedVariables, for (var $ in var2) - Variable(NullAwareTypeConverter.wrapToSql( - ConfigTable.$convertersyncStateImplicit, $)) + Variable(ConfigTable.$convertersyncStateImplicitn.toSql($)) ], readsFrom: { config, @@ -1893,7 +1888,7 @@ abstract class _$CustomTablesDb extends GeneratedDatabase { return customSelect( 'SELECT"defaults"."a" AS "nested_0.a", "defaults"."b" AS "nested_0.b", defaults.b AS "\$n_0" FROM with_defaults AS defaults WHERE a = ?1', variables: [ - Variable(var1) + Variable(var1, const CustomTextType()) ], readsFrom: { withConstraints, diff --git a/drift/test/generated/todos.g.dart b/drift/test/generated/todos.g.dart index 509338616..703f57c8a 100644 --- a/drift/test/generated/todos.g.dart +++ b/drift/test/generated/todos.g.dart @@ -119,8 +119,8 @@ class Category extends DataClass implements Insertable { map['id'] = Variable(id); map['desc'] = Variable(description); { - final converter = $CategoriesTable.$converterpriority; - map['priority'] = Variable(converter.toSql(priority)); + map['priority'] = + Variable($CategoriesTable.$converterpriority.toSql(priority)); } return map; } @@ -246,9 +246,8 @@ class CategoriesCompanion extends UpdateCompanion { map['desc'] = Variable(description.value); } if (priority.present) { - final converter = $CategoriesTable.$converterpriority; - - map['priority'] = Variable(converter.toSql(priority.value)); + map['priority'] = Variable( + $CategoriesTable.$converterpriority.toSql(priority.value)); } return map; } @@ -423,8 +422,8 @@ class TodoEntry extends DataClass implements Insertable { map['category'] = Variable(category); } if (!nullToAbsent || status != null) { - final converter = $TodosTableTable.$converterstatusn; - map['status'] = Variable(converter.toSql(status)); + map['status'] = + Variable($TodosTableTable.$converterstatusn.toSql(status)); } return map; } @@ -598,9 +597,8 @@ class TodosTableCompanion extends UpdateCompanion { map['category'] = Variable(category.value); } if (status.present) { - final converter = $TodosTableTable.$converterstatusn; - - map['status'] = Variable(converter.toSql(status.value)); + map['status'] = Variable( + $TodosTableTable.$converterstatusn.toSql(status.value)); } return map; } @@ -1270,9 +1268,8 @@ class TableWithoutPKCompanion extends UpdateCompanion { map['web_safe_int'] = Variable(webSafeInt.value); } if (custom.present) { - final converter = $TableWithoutPKTable.$convertercustom; - - map['custom'] = Variable(converter.toSql(custom.value)); + map['custom'] = Variable( + $TableWithoutPKTable.$convertercustom.toSql(custom.value)); } if (rowid.present) { map['rowid'] = Variable(rowid.value); @@ -1371,8 +1368,8 @@ class PureDefault extends DataClass implements Insertable { Map toColumns(bool nullToAbsent) { final map = {}; if (!nullToAbsent || txt != null) { - final converter = $PureDefaultsTable.$convertertxtn; - map['insert'] = Variable(converter.toSql(txt)); + map['insert'] = + Variable($PureDefaultsTable.$convertertxtn.toSql(txt)); } return map; } @@ -1457,9 +1454,8 @@ class PureDefaultsCompanion extends UpdateCompanion { Map toColumns(bool nullToAbsent) { final map = {}; if (txt.present) { - final converter = $PureDefaultsTable.$convertertxtn; - - map['insert'] = Variable(converter.toSql(txt.value)); + map['insert'] = + Variable($PureDefaultsTable.$convertertxtn.toSql(txt.value)); } if (rowid.present) { map['rowid'] = Variable(rowid.value); @@ -1532,7 +1528,7 @@ class WithCustomTypeData extends DataClass @override Map toColumns(bool nullToAbsent) { final map = {}; - map['id'] = Variable(id); + map['id'] = Variable(id, const UuidType()); return map; } diff --git a/drift_dev/CHANGELOG.md b/drift_dev/CHANGELOG.md index 0fd39f266..e9efbbbaa 100644 --- a/drift_dev/CHANGELOG.md +++ b/drift_dev/CHANGELOG.md @@ -1,3 +1,8 @@ +## 2.14.1 + +- Fix inconsistencies when generating `Variable` instances for columns with + custom types. + ## 2.14.0 - __Breaking change__: The name of the generated row class derived from the name diff --git a/drift_dev/lib/src/analysis/results/dart.dart b/drift_dev/lib/src/analysis/results/dart.dart index 561189c6a..23e8d9e37 100644 --- a/drift_dev/lib/src/analysis/results/dart.dart +++ b/drift_dev/lib/src/analysis/results/dart.dart @@ -25,6 +25,8 @@ class AnnotatedDartCode { AnnotatedDartCode(this.elements) : assert(elements.every((e) => e is String || e is DartTopLevelSymbol)); + AnnotatedDartCode.text(String e) : elements = [e]; + factory AnnotatedDartCode.ast(AstNode node) { return AnnotatedDartCode.build(((builder) => builder.addAstNode(node))); } diff --git a/drift_dev/lib/src/writer/queries/query_writer.dart b/drift_dev/lib/src/writer/queries/query_writer.dart index e370c41d4..fb5be6a33 100644 --- a/drift_dev/lib/src/writer/queries/query_writer.dart +++ b/drift_dev/lib/src/writer/queries/query_writer.dart @@ -843,37 +843,22 @@ class _ExpandedVariableWriter { // Variables without type converters are written as: // `Variable(x)`. When there's a type converter, we instead use // `Variable(typeConverter.toSql(x))`. - // Finally, if we're dealing with a list, we use a collection for to - // write all the variables sequentially. + // Finally, if we're dealing with a list, we use a collection for to write + // all the variables sequentially. String constructVar(String dartExpr) { - // No longer an array here, we apply a for loop if necessary - final type = _emitter - .dartCode(_emitter.innerColumnType(element.sqlType, nullable: false)); - - final varType = _emitter.drift('Variable'); - final buffer = StringBuffer('$varType<$type>('); final capture = element.forCaptured; - - final converter = element.typeConverter; - if (converter != null) { - // Apply the converter. - if (element.nullable && converter.canBeSkippedForNulls) { - final nullAware = _emitter.drift('NullAwareTypeConverter'); - - buffer.write('$nullAware.wrapToSql(' - '${readConverter(_emitter, element.typeConverter!)}, $dartExpr)'); - } else { - buffer.write( - '${readConverter(_emitter, element.typeConverter!)}.toSql($dartExpr)'); - } - } else if (capture != null) { - buffer.write('row.read(${asDartLiteral(capture.helperColumn)})'); - } else { - buffer.write(dartExpr); + if (capture != null) { + dartExpr = ('row.read(${asDartLiteral(capture.helperColumn)})'); } - buffer.write(')'); - return buffer.toString(); + final code = _emitter.wrapInVariable( + element, + AnnotatedDartCode.text(dartExpr), + // No longer an array here, we apply a for loop below and run this on + // individual values only. + ignoreArray: true, + ); + return _emitter.dartCode(code); } final name = element.dartParameterName; diff --git a/drift_dev/lib/src/writer/tables/data_class_writer.dart b/drift_dev/lib/src/writer/tables/data_class_writer.dart index 5a6903352..289a8f064 100644 --- a/drift_dev/lib/src/writer/tables/data_class_writer.dart +++ b/drift_dev/lib/src/writer/tables/data_class_writer.dart @@ -387,7 +387,6 @@ class RowMappingWriter { extension WriteToColumns on TextEmitter { void writeToColumnsOverride(Iterable columns) { final expression = drift('Expression'); - final variable = drift('Variable'); this ..write('@override\nMap toColumns' @@ -409,28 +408,10 @@ extension WriteToColumns on TextEmitter { } if (needsScope) write('{'); - final typeName = dartCode(variableTypeCode(column, nullable: false)); - final mapSetter = 'map[${asDartLiteral(column.nameInSql)}] = ' - '$variable<$typeName>'; - - if (column.typeConverter != null) { - // apply type converter before writing the variable - final converter = column.typeConverter!; - - this - ..write('final converter = ') - ..writeDart(readConverter(converter, forNullable: column.nullable)) - ..writeln(';') - ..write(mapSetter) - ..write('(converter.toSql(${column.nameInDart}));'); - } else { - // no type converter. Write variable directly - this - ..write(mapSetter) - ..write('(') - ..write(column.nameInDart) - ..write(');'); - } + write('map[${asDartLiteral(column.nameInSql)}] = '); + writeDart( + wrapInVariable(column, AnnotatedDartCode.text(column.nameInDart))); + writeln(';'); // This one closes the optional if from before. if (needsScope) write('}'); diff --git a/drift_dev/lib/src/writer/tables/update_companion_writer.dart b/drift_dev/lib/src/writer/tables/update_companion_writer.dart index d5abc2627..0721bc5eb 100644 --- a/drift_dev/lib/src/writer/tables/update_companion_writer.dart +++ b/drift_dev/lib/src/writer/tables/update_companion_writer.dart @@ -192,34 +192,13 @@ class UpdateCompanionWriter { final getterName = thisIfNeeded(column.nameInDart, locals); _buffer.writeln('if ($getterName.present) {'); - final typeName = - _emitter.dartCode(_emitter.variableTypeCode(column, nullable: false)); - final mapSetter = 'map[${asDartLiteral(column.nameInSql)}] = ' - '${_emitter.drift('Variable')}<$typeName>'; + _buffer.write('map[${asDartLiteral(column.nameInSql)}] = '); var value = '$getterName.value'; - final converter = column.typeConverter; - if (converter != null) { - // apply type converter before writing the variable - final fieldName = _emitter.dartCode( - _emitter.readConverter(converter, forNullable: column.nullable)); - _buffer.writeln('final converter = $fieldName;\n'); - value = 'converter.toSql($value)'; - } - - _buffer - ..write(mapSetter) - ..write('($value'); - - if (column.sqlType.isCustom) { - // Also specify the custom type since it can't be inferred from the - // value passed to the variable. - _buffer - ..write(', ') - ..write(_emitter.dartCode(column.sqlType.custom!.expression)); - } + _emitter.writeDart( + _emitter.wrapInVariable(column, AnnotatedDartCode.text(value))); - _buffer.writeln(');}'); + _buffer.writeln(';}'); } _buffer.write('return map; \n}\n'); diff --git a/drift_dev/lib/src/writer/writer.dart b/drift_dev/lib/src/writer/writer.dart index eb9252ff6..a6b36dfca 100644 --- a/drift_dev/lib/src/writer/writer.dart +++ b/drift_dev/lib/src/writer/writer.dart @@ -181,9 +181,10 @@ abstract class _NodeOrWriter { /// The Dart type that matches the type of this column, ignoring type /// converters. /// - /// This is the same as [dartType] but without custom types. - AnnotatedDartCode variableTypeCode(HasType type, {bool? nullable}) { - if (type.isArray) { + /// This is the same as [dartType] but without type converters. + AnnotatedDartCode variableTypeCode(HasType type, + {bool? nullable, bool ignoreArray = false}) { + if (type.isArray && !ignoreArray) { final inner = innerColumnType(type.sqlType, nullable: nullable ?? type.nullable); return AnnotatedDartCode([ @@ -217,6 +218,40 @@ abstract class _NodeOrWriter { }); } + AnnotatedDartCode wrapInVariable(HasType column, AnnotatedDartCode expression, + {bool ignoreArray = false}) { + return AnnotatedDartCode.build((b) { + b + ..addTopLevel(DartTopLevelSymbol.drift('Variable')) + ..addText('<') + ..addCode( + variableTypeCode(column, nullable: false, ignoreArray: ignoreArray)) + ..addText('>('); + + final converter = column.typeConverter; + if (converter != null) { + // apply type converter before writing the variable + b + ..addCode(readConverter(converter, forNullable: column.nullable)) + ..addText('.toSql(') + ..addCode(expression) + ..addText(')'); + } else { + b.addCode(expression); + } + + if (column.sqlType.isCustom) { + // Also specify the custom type since it can't be inferred from the + // value passed to the variable. + b + ..addText(', ') + ..addCode(column.sqlType.custom!.expression); + } + + b.addText(')'); + }); + } + String refUri(Uri definition, String element) { final prefix = writer.generationOptions.imports.prefixFor(definition, element); diff --git a/drift_dev/pubspec.yaml b/drift_dev/pubspec.yaml index bf706b9b8..4a79cf3b5 100644 --- a/drift_dev/pubspec.yaml +++ b/drift_dev/pubspec.yaml @@ -1,6 +1,6 @@ name: drift_dev description: Dev-dependency for users of drift. Contains the generator and development tools. -version: 2.14.0 +version: 2.14.1 repository: https://github.com/simolus3/drift homepage: https://drift.simonbinder.eu/ issue_tracker: https://github.com/simolus3/drift/issues diff --git a/extras/drift_postgres/example/main.dart b/extras/drift_postgres/example/main.dart index 4927c4d58..ad85ad511 100644 --- a/extras/drift_postgres/example/main.dart +++ b/extras/drift_postgres/example/main.dart @@ -8,6 +8,9 @@ part 'main.g.dart'; class Users extends Table { UuidColumn get id => customType(PgTypes.uuid).withDefault(genRandomUuid())(); TextColumn get name => text()(); + + @override + Set>? get primaryKey => {id}; } @DriftDatabase(tables: [Users]) @@ -38,5 +41,7 @@ void main() async { UsersCompanion.insert(name: 'Simon', id: Value(Uuid().v4obj()))); print(user); + await database.users.deleteOne(user); + await database.close(); } diff --git a/extras/drift_postgres/example/main.g.dart b/extras/drift_postgres/example/main.g.dart index 3b79ed016..6b4e754ad 100644 --- a/extras/drift_postgres/example/main.g.dart +++ b/extras/drift_postgres/example/main.g.dart @@ -45,7 +45,7 @@ class $UsersTable extends Users with TableInfo<$UsersTable, User> { } @override - Set get $primaryKey => const {}; + Set get $primaryKey => {id}; @override User map(Map data, {String? tablePrefix}) { final effectivePrefix = tablePrefix != null ? '$tablePrefix.' : ''; @@ -70,7 +70,7 @@ class User extends DataClass implements Insertable { @override Map toColumns(bool nullToAbsent) { final map = {}; - map['id'] = Variable(id); + map['id'] = Variable(id, PgTypes.uuid); map['name'] = Variable(name); return map; }