5.0.1 Nullability corrections (#78)

* test to cover all mentioned nullability issues

* minor

* minor

* ci correction

* use analyse action to output errors correctly to the CI dashboard actions

* minor corrections

* crashing dynamic test case

* minor

* show warnings from "dart run test"

* Revert "show warnings from "dart run test""

This reverts commit f0d4ae3.

* introspecting the class itself about the relevant constructors parameters

* have ! only when the field of the class is non-nullable

* Roll back feature with nullable field and non-nullable constructor.

* tests + introducing an error when fields nullability not in sync with the constructor

* remove question mark from dynamic input parameters

* version bump

.github/workflows/run_tests.yml

@@ -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
@@ -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

copy_with_extension/CHANGELOG.md

@@ -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)).

copy_with_extension/analysis_options.yaml

@@ -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:

copy_with_extension/pubspec.yaml

@@ -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

copy_with_extension_gen/CHANGELOG.md

@@ -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)).

copy_with_extension_gen/analysis_options.yaml

@@ -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:

copy_with_extension_gen/lib/src/copy_with_generator.dart

@@ -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,
@@ -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,
   ) {
@@ -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;
@@ -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,
@@ -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(),';
@@ -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},''';
       },
     );
 

copy_with_extension_gen/lib/src/field_info.dart

@@ -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,

copy_with_extension_gen/lib/src/helpers.dart

@@ -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,
 ) {
@@ -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,

copy_with_extension_gen/lib/src/settings.dart

@@ -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,

copy_with_extension_gen/pubspec.yaml

@@ -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

copy_with_extension_gen/test/gen_nullability_test.dart

@@ -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);
+  });
+}

copy_with_extension_gen/test/generated_code_test_cases/custom_settings_test_case.dart

@@ -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()

copy_with_extension_gen/test/generated_code_test_cases/test_case_basic_usage.dart

@@ -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()

copy_with_extension_gen/test/generated_code_test_cases/test_cases_exceptions.dart

@@ -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;
+}