Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

5.0.1 Nullability corrections #78

Merged
merged 17 commits into from
Feb 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/run_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ jobs:

steps:
- name: Checkout
uses: actions/checkout@v2
uses: actions/checkout@v3

- name: Install Dart
uses: dart-lang/setup-dart@v1
Expand All @@ -52,7 +52,7 @@ jobs:
run: dart run build_runner build --delete-conflicting-outputs

- name: Analyze
run: dart analyze
uses: zgosalvez/github-actions-analyze-dart@v1

- name: Run tests
run: dart run test
Expand Down
5 changes: 5 additions & 0 deletions copy_with_extension/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 5.0.1
* [Fix](https://github.com/numen31337/copy_with_extension/issues/72) Warnings when using the `?` operator on a dynamic type.
* [Fix](https://github.com/numen31337/copy_with_extension/issues/75) Warnings when using the `!` operator on a non-nullable type.
* [Fix](https://github.com/numen31337/copy_with_extension/issues/74) Crash when object contains a dynamic field that is `null`.

## 5.0.0
* Allow positioned constructor parameters (thanks [@mrgnhnt96](https://github.com/mrgnhnt96)).
* Ability to define library defaults settings globally (thanks [@mrgnhnt96](https://github.com/mrgnhnt96)).
Expand Down
7 changes: 4 additions & 3 deletions copy_with_extension/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
include: package:flutter_lints/flutter.yaml

analyzer:
strong-mode:
implicit-casts: false
implicit-dynamic: false
language:
strict-casts: true
strict-inference: true
strict-raw-types: true

linter:
rules:
Expand Down
2 changes: 1 addition & 1 deletion copy_with_extension/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: copy_with_extension
version: 5.0.0
version: 5.0.1
description: Annotation for generating `copyWith` extensions code using `copy_with_extension_gen`.
homepage: https://github.com/numen31337/copy_with_extension/tree/master/copy_with_extension
repository: https://github.com/numen31337/copy_with_extension
Expand Down
5 changes: 5 additions & 0 deletions copy_with_extension_gen/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 5.0.1
* [Fix](https://github.com/numen31337/copy_with_extension/issues/72) Warnings when using the `?` operator on a dynamic type.
* [Fix](https://github.com/numen31337/copy_with_extension/issues/75) Warnings when using the `!` operator on a non-nullable type.
* [Fix](https://github.com/numen31337/copy_with_extension/issues/74) Crash when object contains a dynamic field that is `null`.

## 5.0.0
* Allow positioned constructor parameters (thanks [@mrgnhnt96](https://github.com/mrgnhnt96)).
* Ability to define library defaults settings globally (thanks [@mrgnhnt96](https://github.com/mrgnhnt96)).
Expand Down
7 changes: 4 additions & 3 deletions copy_with_extension_gen/analysis_options.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
include: package:flutter_lints/flutter.yaml

analyzer:
strong-mode:
implicit-casts: false
implicit-dynamic: false
language:
strict-casts: true
strict-inference: true
strict-raw-types: true

linter:
rules:
Expand Down
23 changes: 18 additions & 5 deletions copy_with_extension_gen/lib/src/copy_with_generator.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {
final typeParametersNames = typeParametersString(classElement, true);
final typeAnnotation = classElement.name + typeParametersNames;

for (final field in sortedFields) {
if (field.classFieldInfo != null &&
field.nullable != field.classFieldInfo?.nullable) {
throw InvalidGenerationSourceError(
'The nullability of the constructor parameter "${field.name}" does not match the nullability of the corresponding field in the object.',
element: element,
);
}
}

return '''
${_copyWithProxyPart(
classAnnotation.constructor,
Expand All @@ -59,7 +69,7 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {
/// Generates the complete `copyWithNull` function.
String _copyWithNullPart(
String typeAnnotation,
List<FieldInfo> sortedFields,
List<ConstructorParameterInfo> sortedFields,
String? constructor,
bool skipFields,
) {
Expand Down Expand Up @@ -111,7 +121,7 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {
String type,
String typeParameters,
String typeParameterNames,
List<FieldInfo> sortedFields,
List<ConstructorParameterInfo> sortedFields,
bool skipFields,
) {
final typeAnnotation = type + typeParameterNames;
Expand Down Expand Up @@ -151,7 +161,7 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {
/// Generates the callable class function for copyWith(...).
String _copyWithValuesPart(
String typeAnnotation,
List<FieldInfo> sortedFields,
List<ConstructorParameterInfo> sortedFields,
String? constructor,
bool skipFields,
bool isAbstract,
Expand All @@ -162,7 +172,8 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {
if (v.fieldAnnotation.immutable) return r; // Skip the field

if (isAbstract) {
final type = v.type.endsWith('?') ? v.type : '${v.type}?';
final type =
v.type.endsWith('?') || v.isDynamic ? v.type : '${v.type}?';
return '$r $type ${v.name},';
} else {
return '$r Object? ${v.name} = const \$CopyWithPlaceholder(),';
Expand All @@ -182,7 +193,9 @@ class CopyWithGenerator extends GeneratorForAnnotation<CopyWith> {

return '''$r ${v.isPositioned ? "" : '${v.name}:'}
${v.name} == const \$CopyWithPlaceholder() $nullCheckForNonNullable
? _value.${v.name} : ${v.name} as ${v.type},''';
? _value.${v.name}
// ignore: cast_nullable_to_non_nullable
: ${v.name} as ${v.type},''';
},
);

Expand Down
60 changes: 49 additions & 11 deletions copy_with_extension_gen/lib/src/field_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,31 +6,69 @@ import 'package:copy_with_extension/copy_with_extension.dart';
import 'package:copy_with_extension_gen/src/copy_with_field_annotation.dart';
import 'package:source_gen/source_gen.dart' show ConstantReader, TypeChecker;

/// Represents a single class field with the additional metadata needed for code generation.
/// Class field info relevant for code generation.
class FieldInfo {
FieldInfo(
ParameterElement element,
ClassElement classElement, {
required this.isPositioned,
}) : name = element.name,
type = element.type.getDisplayString(withNullability: true),
fieldAnnotation = _readFieldAnnotation(element, classElement),
nullable = element.type.nullabilitySuffix != NullabilitySuffix.none;
FieldInfo({required this.name, required this.nullable, required this.type});

final CopyWithFieldAnnotation fieldAnnotation;
/// Parameter / field type.
final String name;

/// If the type is nullable. `dynamic` is considered non-nullable as it doesn't have nullability flag.
final bool nullable;

/// Type name with nullability flag.
final String type;

/// if the field is positioned in the constructor
/// True if the type is `dynamic`.
bool get isDynamic => type == "dynamic";
}

/// Represents a single class field with the additional metadata needed for code generation.
class ConstructorParameterInfo extends FieldInfo {
ConstructorParameterInfo(
ParameterElement element,
ClassElement classElement, {
required this.isPositioned,
}) : fieldAnnotation = _readFieldAnnotation(element, classElement),
classFieldInfo = _classFieldInfo(element.name, classElement),
super(
name: element.name,
nullable: element.type.nullabilitySuffix != NullabilitySuffix.none,
type: element.type.getDisplayString(withNullability: true),
);

/// Annotation provided by the user with `CopyWithField`.
final CopyWithFieldAnnotation fieldAnnotation;

/// True if the field is positioned in the constructor
final bool isPositioned;

/// Info relevant to the given field taken from the class itself, as contrary to the constructor parameter.
/// If `null`, the field with the given name wasn't found on the class.
final FieldInfo? classFieldInfo;

@override
String toString() {
return 'type:$type name:$name fieldAnnotation:$fieldAnnotation nullable:$nullable';
}

/// Returns the field info for the constructor parameter in the relevant class.
static FieldInfo? _classFieldInfo(
String fieldName,
ClassElement classElement,
) {
final field = classElement.fields
.where((e) => e.name == fieldName)
.fold(null, (previousValue, element) => element);
if (field == null) return null;

return FieldInfo(
name: field.name,
nullable: field.type.nullabilitySuffix != NullabilitySuffix.none,
type: field.type.getDisplayString(withNullability: true),
);
}

/// Restores the `CopyWithField` annotation provided by the user.
static CopyWithFieldAnnotation _readFieldAnnotation(
ParameterElement element,
Expand Down
6 changes: 3 additions & 3 deletions copy_with_extension_gen/lib/src/helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import 'package:source_gen/source_gen.dart'

/// Generates a list of `FieldInfo` for each class field that will be a part of the code generation process.
/// The resulting array is sorted by the field name. `Throws` on error.
List<FieldInfo> sortedConstructorFields(
List<ConstructorParameterInfo> sortedConstructorFields(
ClassElement element,
String? constructor,
) {
Expand Down Expand Up @@ -38,10 +38,10 @@ List<FieldInfo> sortedConstructorFields(
);
}

final fields = <FieldInfo>[];
final fields = <ConstructorParameterInfo>[];

for (final parameter in parameters) {
final field = FieldInfo(
final field = ConstructorParameterInfo(
parameter,
element,
isPositioned: parameter.isPositional,
Expand Down
2 changes: 1 addition & 1 deletion copy_with_extension_gen/lib/src/settings.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class Settings {
required this.skipFields,
});

factory Settings.fromConfig(Map json) {
factory Settings.fromConfig(Map<String, dynamic> json) {
return Settings(
copyWithNull: json['copy_with_null'] as bool? ?? false,
skipFields: json['skip_fields'] as bool? ?? false,
Expand Down
2 changes: 1 addition & 1 deletion copy_with_extension_gen/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: copy_with_extension_gen
version: 5.0.0+6
version: 5.0.1
description: Automatically generating `copyWith` extensions code for classes with `@CopyWith()` annotation.
repository: https://github.com/numen31337/copy_with_extension
homepage: https://github.com/numen31337/copy_with_extension/tree/master/copy_with_extension_gen
Expand Down
34 changes: 34 additions & 0 deletions copy_with_extension_gen/test/gen_nullability_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import 'package:copy_with_extension/copy_with_extension.dart';
import 'package:test/test.dart' show test, expect;

part 'gen_nullability_test.g.dart';

@CopyWith()
class TestNullability {
TestNullability(
this.dynamicField,
this.integers,
);

/// https://github.com/numen31337/copy_with_extension/issues/74
/// Test for crash on `instance.dynamicField!`.
final dynamic dynamicField;

/// https://github.com/numen31337/copy_with_extension/issues/75
/// Warnings during compilation when using `!` on non-nullable value.
/// Use `dart run copy_with_extension_gen/test/gen_nullability_test.dart` to reproduce the warning.
final List<int> integers;
}

void main() {
test('TestNullability', () {
// Test for crash in both flows for `dynamicField`, when `dynamicField` is affected and not affected.
expect(TestNullability(1, [1]).copyWith.integers([2]).dynamicField, 1);
expect(TestNullability(1, [1]).copyWith.dynamicField(2).dynamicField, 2);
expect(TestNullability(1, [1]).copyWith(dynamicField: 2).dynamicField, 2);
expect(TestNullability(1, [1]).copyWith(integers: [1]).dynamicField, 1);
expect(TestNullability(null, [1]).copyWith.dynamicField(1).dynamicField, 1);
expect(
TestNullability(null, [1]).copyWith.integers([2]).dynamicField, null);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ class _$BasicClassCWProxyImpl<T extends Iterable<int>>
}) {
return BasicClass<T>(
id: id == const $CopyWithPlaceholder() || id == null
// ignore: unnecessary_non_null_assertion
? _value.id!
? _value.id
// ignore: cast_nullable_to_non_nullable
: id as String,
optional: optional == const $CopyWithPlaceholder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,7 @@ class _$BasicClassCWProxyImpl<T extends Iterable<int>>
}) {
return BasicClass<T>(
id: id == const $CopyWithPlaceholder() || id == null
// ignore: unnecessary_non_null_assertion
? _value.id!
? _value.id
// ignore: cast_nullable_to_non_nullable
: id as String,
optional: optional == const $CopyWithPlaceholder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,17 @@ class WrongConstructor {}
class NoDefaultConstructor {
NoDefaultConstructor.nonDefault();
}

@ShouldThrow(
'The nullability of the constructor parameter "nullableWithNonNullableConstructor" does not match the nullability of the corresponding field in the object.',
)
@CopyWith()
class TestNullability {
TestNullability(
int this.nullableWithNonNullableConstructor,
);

// Some info on this: https://github.com/numen31337/copy_with_extension/pull/69
// If a field is nullable, you can change the type of the constructor parameter to be non-nullable. However, if you do this, an exception may be thrown because the constructor only accepts non-nullable parameters. The issue is that we cannot guarantee that the parameter will be non-null when calling the constructor in the `copyWith` function. Therefore, even if the constructor constructs an object with a nullable field, it is currently impossible to achieve this in the implementation.
final int? nullableWithNonNullableConstructor;
}