Consider inheritance when checking for getters

This commit is contained in:
Simon Binder 2021-12-19 11:48:18 +01:00
parent 102d0d9246
commit 36b23942f4
No known key found for this signature in database
GPG Key ID: 7891917E4147B8C0
2 changed files with 83 additions and 17 deletions

View File

@ -49,6 +49,10 @@ ExistingRowClass? validateExistingClass(
return null; return null;
} }
// Note: It's ok if not all columns are present in the custom row class, we
// just won't load them in that case.
// However, when we're supposed to generate an insertable, all columns must
// appear as getters in the target class.
final unmatchedColumnsByName = { final unmatchedColumnsByName = {
for (final column in columns) column.dartGetterName: column for (final column in columns) column.dartGetterName: column
}; };
@ -58,22 +62,8 @@ ExistingRowClass? validateExistingClass(
for (final parameter in ctor.parameters) { for (final parameter in ctor.parameters) {
final column = unmatchedColumnsByName.remove(parameter.name); final column = unmatchedColumnsByName.remove(parameter.name);
if (column != null) { if (column != null) {
final matchField = !generateInsertable || columnsToParameter[column] = parameter;
dartClass.classElement.fields _checkType(parameter, column, step);
.any((field) => field.name == parameter.name);
if (matchField) {
columnsToParameter[column] = parameter;
_checkType(parameter, column, step);
} else {
final error = ErrorInDartCode(
affectedElement: parameter,
severity: Severity.criticalError,
message: 'Constructor parameter ${parameter.name} has no matching '
'field. When "generateInsertable" enabled, all constructor '
'parameter must have a matching field. Alternatively, you can '
'declare a getter field.');
throw Exception(error);
}
} else if (!parameter.isOptional) { } else if (!parameter.isOptional) {
errors.report(ErrorInDartCode( errors.report(ErrorInDartCode(
affectedElement: parameter, affectedElement: parameter,
@ -83,6 +73,31 @@ ExistingRowClass? validateExistingClass(
} }
} }
if (generateInsertable) {
// Go through all columns, make sure that the class has getters for them.
final missingGetters = <String>[];
for (final column in columns) {
final matchingField = dartClass.classElement
.lookUpGetter(column.dartGetterName, dartClass.classElement.library);
if (matchingField == null) {
missingGetters.add(column.dartGetterName);
}
}
if (missingGetters.isNotEmpty) {
errors.report(ErrorInDartCode(
affectedElement: dartClass.classElement,
severity: Severity.criticalError,
message:
'This class used as a custom row class for which an insertable '
'is generated. This means that it must define getters for all '
'columns, but some are missing: ${missingGetters.join(', ')}',
));
}
}
return ExistingRowClass( return ExistingRowClass(
desiredClass, ctor, columnsToParameter, generateInsertable, desiredClass, ctor, columnsToParameter, generateInsertable,
typeInstantiation: dartClass.instantiation ?? const []); typeInstantiation: dartClass.instantiation ?? const []);

View File

@ -11,7 +11,7 @@ void main() {
late TestState state; late TestState state;
setUpAll(() { setUpAll(() {
state = TestState.withContent({ state = TestState.withContent(const {
'a|lib/invalid_no_unnamed_constructor.dart': ''' 'a|lib/invalid_no_unnamed_constructor.dart': '''
import 'package:drift/drift.dart'; import 'package:drift/drift.dart';
@ -113,6 +113,42 @@ class Cls {
Cls(Uint8List foo, List<int> bar, Bytes baz) {} Cls(Uint8List foo, List<int> bar, Bytes baz) {}
} }
''', ''',
'a|lib/insertable_missing.dart': '''
import 'package:drift/drift.dart';
@UseRowClass(Cls, generateInsertable: true)
class Tbl extends Table {
TextColumn get foo => text()();
IntColumn get bar => integer()();
}
class Cls {
final String foo;
Cls(this.foo, int bar);
}
''',
'a|lib/insertable_valid.dart': '''
import 'package:drift/drift.dart';
@UseRowClass(Cls, generateInsertable: true)
class Tbl extends Table {
TextColumn get foo => text()();
IntColumn get bar => integer()();
}
class HasBar {
final int bar;
HasBar(this.bar);
}
class Cls extends HasBar {
final String foo;
Cls(this.foo, int bar): super(bar);
}
''',
}); });
}); });
@ -168,6 +204,16 @@ class Cls {
.having((e) => e.message, 'message', 'Parameter must accept int')), .having((e) => e.message, 'message', 'Parameter must accept int')),
); );
}); });
test('when a getter is missing with generateInsertable: true', () async {
final file = await state.analyze('package:a/insertable_missing.dart');
expect(
file.errors.errors,
contains(isA<ErrorInDartCode>().having((e) => e.message, 'message',
contains('but some are missing: bar'))),
);
});
}); });
test('supports generic row classes', () async { test('supports generic row classes', () async {
@ -217,4 +263,9 @@ class Cls {
final file = await state.analyze('package:a/blob.dart'); final file = await state.analyze('package:a/blob.dart');
expect(file.errors.errors, isEmpty); expect(file.errors.errors, isEmpty);
}); });
test('considers inheritance when checking expected getters', () async {
final file = await state.analyze('package:a/insertable_valid.dart');
expect(file.errors.errors, isEmpty);
});
} }