Skip to content

Commit

Permalink
Don't generate const constr for non-const parent
Browse files Browse the repository at this point in the history
Closes #2735
  • Loading branch information
simolus3 committed Nov 19, 2023
1 parent 3980fd0 commit 39018d3
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 24 deletions.
3 changes: 3 additions & 0 deletions drift_dev/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## 2.14.0-dev

- Don't generate `const` row classes when they are extending a class which
isn't const.

## 2.13.2

- Fix generated queries relying on custom types.
Expand Down
4 changes: 2 additions & 2 deletions drift_dev/lib/src/analysis/resolver/dart/helper.dart
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ extension TypeUtils on DartType {

class DataClassInformation {
final String enforcedName;
final AnnotatedDartCode? extending;
final CustomParentClass? extending;
final ExistingRowClass? existingClass;

DataClassInformation(
Expand Down Expand Up @@ -234,7 +234,7 @@ class DataClassInformation {
}

String name;
AnnotatedDartCode? customParentClass;
CustomParentClass? customParentClass;
ExistingRowClass? existingClass;

if (dataClassName != null) {
Expand Down
26 changes: 21 additions & 5 deletions drift_dev/lib/src/analysis/resolver/shared/data_class.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import 'package:analyzer/dart/constant/value.dart';
import 'package:analyzer/dart/element/element.dart';
import 'package:analyzer/dart/element/type.dart';
import 'package:drift_dev/src/analysis/results/result_sets.dart';

import '../../driver/error.dart';
import '../../results/dart.dart';
Expand All @@ -24,7 +25,7 @@ String dataClassNameForClassName(String tableName) {
return '${tableName}Data';
}

AnnotatedDartCode? parseCustomParentClass(
CustomParentClass? parseCustomParentClass(
String dartTypeName,
DartObject dataClassName,
ClassElement element,
Expand All @@ -44,7 +45,6 @@ AnnotatedDartCode? parseCustomParentClass(
'DataClass',
),
);
return null;
}

if (extendingType.typeArguments.length > 1) {
Expand All @@ -55,7 +55,22 @@ AnnotatedDartCode? parseCustomParentClass(
'type parameter',
),
);
return null;
}

final defaultConstructor =
extendingType.lookUpConstructor(null, element.library);
var isConst = true;
AnnotatedDartCode code;
if (defaultConstructor == null) {
resolver.reportError(
DriftAnalysisError.forDartElement(
element,
'Parameter `extending` in @DataClassName must have an unnamed '
'constructor.',
),
);
} else {
isConst = defaultConstructor.isConst;
}

// For legacy reasons, if we're extending an existing class with a type
Expand All @@ -64,7 +79,7 @@ AnnotatedDartCode? parseCustomParentClass(
if (extendingType.typeArguments.length == 1) {
final genericType = extendingType.typeArguments[0];
if (genericType.isDartCoreObject || genericType is DynamicType) {
return AnnotatedDartCode([
code = AnnotatedDartCode([
DartTopLevelSymbol.topLevelElement(extendingType.element),
'<',
DartTopLevelSymbol(dartTypeName, null),
Expand All @@ -84,7 +99,8 @@ AnnotatedDartCode? parseCustomParentClass(
}
}

return AnnotatedDartCode.type(extendingType);
code = AnnotatedDartCode.type(extendingType);
return CustomParentClass(parentClass: code, isConst: isConst);
} else {
resolver.reportError(
DriftAnalysisError.forDartElement(
Expand Down
15 changes: 14 additions & 1 deletion drift_dev/lib/src/analysis/results/result_sets.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ import 'element.dart';
import 'column.dart';
import 'dart.dart';

/// Drift allows specifying a base class which the generated row class will then
/// extend. This class describes the specified parent class along with
/// additional information needed for code generation.
class CustomParentClass {
final AnnotatedDartCode parentClass;
final bool isConst;

CustomParentClass({
required this.parentClass,
required this.isConst,
});
}

abstract class DriftElementWithResultSet extends DriftSchemaElement {
/// The columns declared in this table or view.
List<DriftColumn> get columns;
Expand All @@ -18,7 +31,7 @@ abstract class DriftElementWithResultSet extends DriftSchemaElement {
ExistingRowClass? get existingRowClass;

/// Class that added to data class as implementation
AnnotatedDartCode? get customParentClass;
CustomParentClass? get customParentClass;

/// Whether this table has an existing row class, meaning that drift will not
/// generate one on its own.
Expand Down
3 changes: 1 addition & 2 deletions drift_dev/lib/src/analysis/results/table.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import 'package:collection/collection.dart';
import 'package:drift/drift.dart' show DriftSqlType;
import 'package:sqlparser/sqlparser.dart' as sql;

import 'dart.dart';
import 'element.dart';

import 'column.dart';
Expand All @@ -22,7 +21,7 @@ class DriftTable extends DriftElementWithResultSet {
final ExistingRowClass? existingRowClass;

@override
final AnnotatedDartCode? customParentClass;
final CustomParentClass? customParentClass;

/// The fixed [entityInfoName] to use, overriding the default.
final String? fixedEntityInfoName;
Expand Down
2 changes: 1 addition & 1 deletion drift_dev/lib/src/analysis/results/view.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class DriftView extends DriftElementWithResultSet {
final DriftViewSource source;

@override
final AnnotatedDartCode? customParentClass;
final CustomParentClass? customParentClass;

@override
String entityInfoName;
Expand Down
34 changes: 26 additions & 8 deletions drift_dev/lib/src/analysis/serializer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ class ElementSerializer {
for (final constraint in element.tableConstraints)
_serializeTableConstraint(constraint),
],
'custom_parent_class': element.customParentClass?.toJson(),
'custom_parent_class':
_serializeCustomParentClass(element.customParentClass),
'fixed_entity_info_name': element.fixedEntityInfoName,
'base_dart_name': element.baseDartName,
'row_class_name': element.nameOfRowClass,
Expand Down Expand Up @@ -141,7 +142,8 @@ class ElementSerializer {
'existing_data_class': element.existingRowClass != null
? _serializeExistingRowClass(element.existingRowClass!)
: null,
'custom_parent_class': element.customParentClass?.toJson(),
'custom_parent_class':
_serializeCustomParentClass(element.customParentClass),
'name_of_row_class': element.nameOfRowClass,
'source': serializedSource,
};
Expand Down Expand Up @@ -310,6 +312,15 @@ class ElementSerializer {
};
}

Map<String, Object?>? _serializeCustomParentClass(CustomParentClass? pc) {
if (pc == null) return null;

return {
'class': pc.parentClass.toJson(),
'const': pc.isConst,
};
}

String? _serializeReferenceAction(ReferenceAction? action) {
return action?.name;
}
Expand Down Expand Up @@ -519,9 +530,8 @@ class ElementDeserializer {
for (final constraint in json.list('table_constraints'))
await _readTableConstraint(constraint as Map, columnByName),
],
customParentClass: json['custom_parent_class'] != null
? AnnotatedDartCode.fromJson(json['custom_parent_class'] as Map)
: null,
customParentClass:
_readCustomParentClass(json['custom_parent_class'] as Map?),
fixedEntityInfoName: json['fixed_entity_info_name'] as String?,
baseDartName: json['base_dart_name'] as String,
nameOfRowClass: json['row_class_name'] as String,
Expand Down Expand Up @@ -658,9 +668,8 @@ class ElementDeserializer {
references: references,
columns: columns,
entityInfoName: json['entity_info_name'] as String,
customParentClass: json['custom_parent_class'] != null
? AnnotatedDartCode.fromJson(json['custom_parent_class'] as Map)
: null,
customParentClass:
_readCustomParentClass(json['custom_parent_class'] as Map?),
nameOfRowClass: json['name_of_row_class'] as String,
existingRowClass: json['existing_data_class'] != null
? await _readExistingRowClass(
Expand Down Expand Up @@ -805,6 +814,15 @@ class ElementDeserializer {
);
}

CustomParentClass? _readCustomParentClass(Map? json) {
if (json == null) return null;

return CustomParentClass(
parentClass: AnnotatedDartCode.fromJson(json['class'] as Map),
isConst: json['const'] as bool,
);
}

ReferenceAction? _readAction(String? value) {
return value == null ? null : ReferenceAction.values.byName(value);
}
Expand Down
9 changes: 5 additions & 4 deletions drift_dev/lib/src/writer/tables/data_class_writer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ class DataClassWriter {
}

void write() {
final parentClass = table.customParentClass != null
? _emitter.dartCode(table.customParentClass!)
final customParent = table.customParentClass;
final parentClass = customParent != null
? _emitter.dartCode(customParent.parentClass)
: _emitter.drift('DataClass');
_buffer.write('class ${table.nameOfRowClass} extends $parentClass ');

Expand Down Expand Up @@ -76,8 +77,8 @@ class DataClassWriter {
}

// write constructor with named optional fields

if (!scope.options.generateMutableClasses) {
if (!scope.options.generateMutableClasses &&
customParent?.isConst != false) {
_buffer.write('const ');
}
_emitter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class FooData {
expect(file.allErrors, isEmpty);

final table = file.analyzedElements.single as DriftTable;
expect(table.customParentClass?.toString(), 'BaseModel');
expect(table.customParentClass?.parentClass.toString(), 'BaseModel');
});

test('check valid with type argument', () async {
Expand Down Expand Up @@ -473,6 +473,14 @@ class FooData {
'@DataClassName must be subtype of DataClass'))
],
);

final table = file.analyzedElements.single as DriftTable;
expect(
table.customParentClass,
isA<CustomParentClass>()
.having((e) => e.isConst, 'isConst', false)
.having(
(e) => e.parentClass.toString(), 'parentClass', 'BaseModel'));
});

test('wrong type argument in extending', () async {
Expand Down

0 comments on commit 39018d3

Please sign in to comment.