From 9c76027a14b56c82c321414a37fef5078b0a203c Mon Sep 17 00:00:00 2001 From: Remi Rousselet Date: Tue, 29 Nov 2022 12:04:17 +0100 Subject: [PATCH 1/2] fix: copyWith sometimes not working with null fixes #807 fixes #803 --- packages/freezed/lib/builder.dart | 2 +- .../lib/src/templates/parameter_template.dart | 4 +- .../freezed/lib/src/templates/properties.dart | 4 +- packages/freezed/lib/src/tools/type.dart | 10 +++++ packages/freezed/test/deep_copy_test.dart | 39 +++++++++++++++++++ .../freezed/test/integration/generic.dart | 15 +++++++ .../integration/single_class_constructor.dart | 10 +++++ 7 files changed, 79 insertions(+), 5 deletions(-) diff --git a/packages/freezed/lib/builder.dart b/packages/freezed/lib/builder.dart index 3f5dda5e..3edec600 100644 --- a/packages/freezed/lib/builder.dart +++ b/packages/freezed/lib/builder.dart @@ -13,7 +13,7 @@ Builder freezed(BuilderOptions options) { // coverage:ignore-file // GENERATED CODE - DO NOT MODIFY BY HAND // ignore_for_file: type=lint -// ignore_for_file: unused_element, deprecated_member_use, deprecated_member_use_from_same_package, use_function_type_syntax_for_parameters, unnecessary_const, avoid_init_to_null, invalid_override_different_default_values_named, prefer_expression_function_bodies, annotate_overrides, invalid_annotation_target +// ignore_for_file: unused_element, deprecated_member_use, deprecated_member_use_from_same_package, use_function_type_syntax_for_parameters, unnecessary_const, avoid_init_to_null, invalid_override_different_default_values_named, prefer_expression_function_bodies, annotate_overrides, invalid_annotation_target, unnecessary_question_mark ''', options: options, ); diff --git a/packages/freezed/lib/src/templates/parameter_template.dart b/packages/freezed/lib/src/templates/parameter_template.dart index b70ade6a..9d6a6e00 100644 --- a/packages/freezed/lib/src/templates/parameter_template.dart +++ b/packages/freezed/lib/src/templates/parameter_template.dart @@ -1,9 +1,9 @@ import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:build/build.dart'; import 'package:freezed/src/templates/concrete_template.dart'; import 'package:freezed/src/templates/properties.dart'; import 'package:freezed/src/templates/prototypes.dart'; +import 'package:freezed/src/tools/type.dart'; import 'package:freezed/src/utils.dart'; class GenericsDefinitionTemplate { @@ -70,7 +70,7 @@ class ParametersTemplate { Future asParameter(ParameterElement e) async { final value = Parameter( name: e.name, - isNullable: e.type.nullabilitySuffix == NullabilitySuffix.question, + isNullable: e.type.isNullable, isDartList: e.type.isDartCoreList, isDartMap: e.type.isDartCoreMap, isDartSet: e.type.isDartCoreSet, diff --git a/packages/freezed/lib/src/templates/properties.dart b/packages/freezed/lib/src/templates/properties.dart index e485b087..e6155e60 100644 --- a/packages/freezed/lib/src/templates/properties.dart +++ b/packages/freezed/lib/src/templates/properties.dart @@ -1,7 +1,7 @@ import 'package:analyzer/dart/element/element.dart'; -import 'package:analyzer/dart/element/nullability_suffix.dart'; import 'package:analyzer/dart/element/type.dart'; import 'package:build/build.dart'; +import 'package:freezed/src/tools/type.dart'; import 'package:source_gen/source_gen.dart'; import '../utils.dart'; @@ -49,7 +49,7 @@ class Property { return Property( name: element.name, - isNullable: element.type.nullabilitySuffix == NullabilitySuffix.question, + isNullable: element.type.isNullable, isDartList: element.type.isDartCoreList, isDartMap: element.type.isDartCoreMap, isDartSet: element.type.isDartCoreSet, diff --git a/packages/freezed/lib/src/tools/type.dart b/packages/freezed/lib/src/tools/type.dart index b34f4315..bf9ada42 100644 --- a/packages/freezed/lib/src/tools/type.dart +++ b/packages/freezed/lib/src/tools/type.dart @@ -5,6 +5,16 @@ import 'package:collection/collection.dart'; import 'imports.dart'; +extension DartTypeX on DartType { + bool get isNullable { + final that = this; + if (that is TypeParameterType) { + return that.bound.isNullable; + } + return isDynamic || nullabilitySuffix == NullabilitySuffix.question; + } +} + /// Returns the [Element] for a given [DartType] /// /// this is usually type.element, except if it is a typedef then it is diff --git a/packages/freezed/test/deep_copy_test.dart b/packages/freezed/test/deep_copy_test.dart index a8403951..8596f40b 100644 --- a/packages/freezed/test/deep_copy_test.dart +++ b/packages/freezed/test/deep_copy_test.dart @@ -5,6 +5,9 @@ import 'package:test/test.dart'; import 'common.dart'; import 'integration/deep_copy.dart'; +import 'integration/generic.dart' + show AnyGeneric, NullableGeneric, NonNullableGeneric; +import 'integration/single_class_constructor.dart' show Dynamic; void main() { // TODO: copyWith is identical to itself if don't have descendants @@ -67,6 +70,42 @@ void main() { () => company.copyWith.director!.assistant!(name: 'John'), throwsA(isA()), ); + + expect( + Dynamic(foo: 42, bar: 21).copyWith(foo: null, bar: null), + Dynamic(foo: null, bar: null), + ); + expect( + Dynamic(foo: 42, bar: 21).copyWith(), + Dynamic(foo: 42, bar: 21), + ); + + expect( + AnyGeneric(42).copyWith(value: null), + AnyGeneric(null), + ); + expect( + AnyGeneric(42).copyWith(), + AnyGeneric(42), + ); + + expect( + NullableGeneric(42).copyWith(value: null), + NullableGeneric(null), + ); + expect( + NullableGeneric(42).copyWith(), + NullableGeneric(42), + ); + + expect( + NonNullableGeneric(42).copyWith(value: 21), + NonNullableGeneric(21), + ); + expect( + NonNullableGeneric(42).copyWith(), + NonNullableGeneric(42), + ); }); group('DeepGeneric', () { diff --git a/packages/freezed/test/integration/generic.dart b/packages/freezed/test/integration/generic.dart index d0726848..c323d117 100644 --- a/packages/freezed/test/integration/generic.dart +++ b/packages/freezed/test/integration/generic.dart @@ -7,6 +7,21 @@ class Model { final T value; } +@freezed +class AnyGeneric with _$AnyGeneric { + factory AnyGeneric(T value) = _AnyGeneric; +} + +@freezed +class NullableGeneric with _$NullableGeneric { + factory NullableGeneric(T value) = _NullableGeneric; +} + +@freezed +class NonNullableGeneric with _$NonNullableGeneric { + factory NonNullableGeneric(T value) = _NonNullableGeneric; +} + @freezed class Generic> with _$Generic { factory Generic(T model) = _Generic; diff --git a/packages/freezed/test/integration/single_class_constructor.dart b/packages/freezed/test/integration/single_class_constructor.dart index 0075d103..d1a8b7ac 100644 --- a/packages/freezed/test/integration/single_class_constructor.dart +++ b/packages/freezed/test/integration/single_class_constructor.dart @@ -1,3 +1,5 @@ +// ignore_for_file: unnecessary_question_mark + import 'dart:async'; import 'dart:collection'; @@ -5,6 +7,14 @@ import 'package:freezed_annotation/freezed_annotation.dart'; part 'single_class_constructor.freezed.dart'; +@freezed +class Dynamic with _$Dynamic { + factory Dynamic({ + dynamic foo, + dynamic? bar, + }) = DynamicFirst; +} + class CustomMap extends MapBase { CustomMap(this._source); From f046088494ca79c5dc5643dffc191abc8b800606 Mon Sep 17 00:00:00 2001 From: Remi Rousselet Date: Tue, 29 Nov 2022 12:05:10 +0100 Subject: [PATCH 2/2] Changelog --- packages/freezed/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/freezed/CHANGELOG.md b/packages/freezed/CHANGELOG.md index e07ddcc5..2de3e250 100644 --- a/packages/freezed/CHANGELOG.md +++ b/packages/freezed/CHANGELOG.md @@ -1,3 +1,7 @@ +# [Unreleased fix] + +Fixes copyWith not working with `null` on some scenarios. + # 2.2.1 Upgrade analyzer