-
Notifications
You must be signed in to change notification settings - Fork 29
#3054. Add covariant parameters tests #3059
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
Conversation
eernstg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
I commented on a couple of tests because I think they could be stronger if they would use an actual type argument for X which is different from a top type: This means that we can see a failure if there is a bug where the type of a formal parameter is relaxed to Object?, but it was already Object? because that was the value of X.
On the other hand, this is of course a corner case, too, so maybe it's fine to have a test where X is bound to Object?.
However, it's more important to ensure that the relaxation of the formal parameter type to Object? does take place than it is to check that nothing wrong happens when that relaxation is a no-op (unless this is already covered by some other test).
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t02.dart
Outdated
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t02.dart
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t03.dart
Outdated
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t03.dart
Outdated
Show resolved
Hide resolved
...Extraction/Instance_Method_Closurization/method_closurization_positional_params_A03_t02.dart
Show resolved
Hide resolved
sgrekhov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanx! Updated. PTAL
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t02.dart
Outdated
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t02.dart
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t03.dart
Outdated
Show resolved
Hide resolved
...erty_Extraction/Instance_Method_Closurization/method_closurization_named_params_A03_t03.dart
Outdated
Show resolved
Hide resolved
...Extraction/Instance_Method_Closurization/method_closurization_positional_params_A03_t02.dart
Show resolved
Hide resolved
eernstg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM!
2025-01-29 sgrekhov22@gmail.com dart-lang/co19#3054. Add covariant parameters tests (dart-lang/co19#3059) 2025-01-29 sgrekhov22@gmail.com dart-lang/co19#3054. Existing tests renamed to Instance Method Closurization (dart-lang/co19#3058) 2025-01-27 sgrekhov22@gmail.com dart-lang/co19#2162. Add static member and non-redirecting factory constructor conflict test (dart-lang/co19#3056) 2025-01-27 sgrekhov22@gmail.com dart-lang/co19#2145. Update existing "Evaluation of Implicit Variable Getters" tests (dart-lang/co19#3052) 2025-01-24 sgrekhov22@gmail.com dart-lang/co19#2138. Add more tests for `call` method/getter (dart-lang/co19#3053) 2025-01-24 sgrekhov22@gmail.com dart-lang/co19#2162. Add more constructor vs. static member conflict tests (dart-lang/co19#3055) 2025-01-17 sgrekhov22@gmail.com dart-lang/co19#2976. Add more tests for nullable and FutureOr types (dart-lang/co19#3050) Cq-Include-Trybots: luci.dart.try:analyzer-linux-release-try Change-Id: Ibeafe23be5d8b913db4e83b23f3820cafc3802ac Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/406940 Reviewed-by: Erik Ernst <eernst@google.com> Reviewed-by: Alexander Thomas <athom@google.com>
Add covariant parameters tests
If this PR is Ok I'll add the same tests for mixins, extension types etc in the next one.