Skip to content

Commit

Permalink
Don't use invalid nullable converter fields
Browse files Browse the repository at this point in the history
  • Loading branch information
simolus3 committed Oct 25, 2023
1 parent 222dc30 commit 0973aa9
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 46 deletions.
19 changes: 0 additions & 19 deletions drift/test/test_utils/test_utils.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: false,
returnValueForMissingStub: false,
) as bool);

@override
set isInTransaction(bool? _isInTransaction) => super.noSuchMethod(
Invocation.setter(
Expand All @@ -73,7 +72,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
),
returnValueForMissingStub: null,
);

@override
_i2.DbVersionDelegate get versionDelegate => (super.noSuchMethod(
Invocation.getter(#versionDelegate),
Expand All @@ -86,7 +84,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
Invocation.getter(#versionDelegate),
),
) as _i2.DbVersionDelegate);

@override
_i2.TransactionDelegate get transactionDelegate => (super.noSuchMethod(
Invocation.getter(#transactionDelegate),
Expand All @@ -99,14 +96,12 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
Invocation.getter(#transactionDelegate),
),
) as _i2.TransactionDelegate);

@override
_i4.FutureOr<bool> get isOpen => (super.noSuchMethod(
Invocation.getter(#isOpen),
returnValue: _i4.Future<bool>.value(false),
returnValueForMissingStub: _i4.Future<bool>.value(false),
) as _i4.FutureOr<bool>);

@override
_i4.Future<void> open(_i5.QueryExecutorUser? db) => (super.noSuchMethod(
Invocation.method(
Expand All @@ -116,7 +111,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);

@override
_i4.Future<void> close() => (super.noSuchMethod(
Invocation.method(
Expand All @@ -126,7 +120,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);

@override
void notifyDatabaseOpened(_i5.OpeningDetails? details) => super.noSuchMethod(
Invocation.method(
Expand All @@ -135,7 +128,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
),
returnValueForMissingStub: null,
);

@override
_i4.Future<_i3.QueryResult> runSelect(
String? statement,
Expand Down Expand Up @@ -171,7 +163,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
),
)),
) as _i4.Future<_i3.QueryResult>);

@override
_i4.Future<int> runUpdate(
String? statement,
Expand All @@ -188,7 +179,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: _i4.Future<int>.value(0),
returnValueForMissingStub: _i4.Future<int>.value(0),
) as _i4.Future<int>);

@override
_i4.Future<int> runInsert(
String? statement,
Expand All @@ -205,7 +195,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: _i4.Future<int>.value(0),
returnValueForMissingStub: _i4.Future<int>.value(0),
) as _i4.Future<int>);

@override
_i4.Future<void> runCustom(
String? statement,
Expand All @@ -222,7 +211,6 @@ class MockDatabaseDelegate extends _i1.Mock implements _i2.DatabaseDelegate {
returnValue: _i4.Future<void>.value(),
returnValueForMissingStub: _i4.Future<void>.value(),
) as _i4.Future<void>);

@override
_i4.Future<void> runBatched(_i5.BatchedStatements? statements) =>
(super.noSuchMethod(
Expand All @@ -246,7 +234,6 @@ class MockDynamicVersionDelegate extends _i1.Mock
returnValue: _i4.Future<int>.value(0),
returnValueForMissingStub: _i4.Future<int>.value(0),
) as _i4.Future<int>);

@override
_i4.Future<void> setSchemaVersion(int? version) => (super.noSuchMethod(
Invocation.method(
Expand All @@ -269,7 +256,6 @@ class MockSupportedTransactionDelegate extends _i1.Mock
returnValue: false,
returnValueForMissingStub: false,
) as bool);

@override
_i4.FutureOr<void> startTransaction(
_i4.Future<dynamic> Function(_i2.QueryDelegate)? run) =>
Expand Down Expand Up @@ -298,7 +284,6 @@ class MockStreamQueries extends _i1.Mock implements _i6.StreamQueryStore {
returnValueForMissingStub:
_i4.Stream<List<Map<String, Object?>>>.empty(),
) as _i4.Stream<List<Map<String, Object?>>>);

@override
_i4.Stream<Set<_i5.TableUpdate>> updatesForSync(
_i5.TableUpdateQuery? query) =>
Expand All @@ -310,7 +295,6 @@ class MockStreamQueries extends _i1.Mock implements _i6.StreamQueryStore {
returnValue: _i4.Stream<Set<_i5.TableUpdate>>.empty(),
returnValueForMissingStub: _i4.Stream<Set<_i5.TableUpdate>>.empty(),
) as _i4.Stream<Set<_i5.TableUpdate>>);

@override
void handleTableUpdates(Set<_i5.TableUpdate>? updates) => super.noSuchMethod(
Invocation.method(
Expand All @@ -319,7 +303,6 @@ class MockStreamQueries extends _i1.Mock implements _i6.StreamQueryStore {
),
returnValueForMissingStub: null,
);

@override
void markAsClosed(
_i6.QueryStream? stream,
Expand All @@ -335,7 +318,6 @@ class MockStreamQueries extends _i1.Mock implements _i6.StreamQueryStore {
),
returnValueForMissingStub: null,
);

@override
void markAsOpened(_i6.QueryStream? stream) => super.noSuchMethod(
Invocation.method(
Expand All @@ -344,7 +326,6 @@ class MockStreamQueries extends _i1.Mock implements _i6.StreamQueryStore {
),
returnValueForMissingStub: null,
);

@override
_i4.Future<void> close() => (super.noSuchMethod(
Invocation.method(
Expand Down
2 changes: 2 additions & 0 deletions drift_dev/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
## 2.13.1-dev

- Add `has_separate_analyzer` option to optimize builds using the `not_shared` builder.
- Avoid illegal references to implicitly nullable variant of type converter when no such
field exists.

## 2.13.0

Expand Down
6 changes: 5 additions & 1 deletion drift_dev/lib/src/analysis/results/column.dart
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,11 @@ class AppliedTypeConverter {
/// column, drift generates a new wrapped type converter which will deal with
/// `null` values.
/// That converter is stored in this field.
String get nullableFieldName => '${fieldName}n';
String get nullableFieldName {
assert(canBeSkippedForNulls && owningColumn?.nullable == true);

return '${fieldName}n';
}

AppliedTypeConverter({
required this.expression,
Expand Down
2 changes: 1 addition & 1 deletion drift_dev/lib/src/writer/tables/table_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ abstract class TableOrViewWriter {
buffer.write('static $typeName ${converter.fieldName} = $code;');

// Generate wrappers for non-nullable type converters that are applied to
// nullable converters.
// nullable columns.
final column = converter.owningColumn!;
if (converter.canBeSkippedForNulls && column.nullable) {
final nullableTypeName = emitter.dartCode(
Expand Down
55 changes: 30 additions & 25 deletions drift_dev/lib/src/writer/writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -99,34 +99,39 @@ abstract class _NodeOrWriter {
/// Returns a Dart expression evaluating to the [converter].
AnnotatedDartCode readConverter(AppliedTypeConverter converter,
{bool forNullable = false}) {
if (converter.owningColumn == null) {
// Type converters applied to individual columns in the result set of a
// `SELECT` query don't have an owning table. We instead write the
// expression here.
return AnnotatedDartCode.build((b) {
final implicitlyNullable =
converter.canBeSkippedForNulls && forNullable;

if (implicitlyNullable) {
b.addSymbol('NullAwareTypeConverter.wrap(', AnnotatedDartCode.drift);
return AnnotatedDartCode.build((b) {
final owningColumn = converter.owningColumn;
final needsImplicitNullableVersion =
forNullable && converter.canBeSkippedForNulls;
final hasNullableVariantInField = owningColumn != null &&
converter.canBeSkippedForNulls &&
owningColumn.nullable;

void addRegularConverter() {
if (owningColumn != null) {
b
..addCode(entityInfoType(owningColumn.owner))
..addText('.${converter.fieldName}');
} else {
// There's no field storing this converter, so evaluate it every time
// it is used.
b.addCode(converter.expression);
}
}

b.addCode(converter.expression);

if (implicitlyNullable) {
switch ((needsImplicitNullableVersion, hasNullableVariantInField)) {
case (false, _):
addRegularConverter();
case (true, false):
b.addSymbol('NullAwareTypeConverter.wrap(', AnnotatedDartCode.drift);
b.addCode(converter.expression);
b.addText(')');
}
});
} else {
final fieldName = forNullable && converter.canBeSkippedForNulls
? converter.nullableFieldName
: converter.fieldName;

return AnnotatedDartCode([
...entityInfoType(converter.owningColumn!.owner).elements,
'.$fieldName',
]);
}
case (true, true):
b
..addCode(entityInfoType(converter.owningColumn!.owner))
..addText('.${converter.nullableFieldName}');
}
});
}

/// A suitable typename to store an instance of the type converter used here.
Expand Down
32 changes: 32 additions & 0 deletions drift_dev/test/writer/writer_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,36 @@ class Database {}
))
}, result.dartOutputs, result.writer);
}, skip: requireDart('3.0.0-dev'));

test(
'references nullable variant of converter on non-nullable column',
() async {
final result = await emulateDriftBuild(
inputs: {
'a|lib/converter.dart': '''
import 'package:drift/drift.dart';
TypeConverter<int, String> get testConverter => throw '';
''',
'a|lib/a.drift': '''
import 'converter.dart';
CREATE TABLE foo (
bar TEXT MAPPED BY `testConverter` NOT NULL
);
CREATE VIEW a AS SELECT nullif(bar, '') FROM foo;
''',
},
modularBuild: true,
logger: loggerThat(neverEmits(anything)),
);

checkOutputs({
'a|lib/a.drift.dart': decodedMatches(
allOf(isNot(contains('converterbarn'))),
),
}, result.dartOutputs, result.writer);
},
);
}

0 comments on commit 0973aa9

Please sign in to comment.