#1959. Versioning tests added #2029
Conversation
There was a problem hiding this comment.
Some of these tests are obvious. For example, errors expected in LanguageFeatures/Class-modifiers/versioning_A04_t02.dart must be errors, because we can't mix in a non-mixin in new code.
However, several other libraries are quite tricky. For example, LanguageFeatures/Class-modifiers/versioning_A04_t01.dart, where PreFeatureImplementsInterface seems to be a perfectly OK class, in legacy code as well as new code, and hence we shouldn't expect any errors from its subtypes. PreFeatureWithInterface is really tricky, as explained in a comment in this library.
So please take a look, fix the cases that are clear, and complain about the vagueness of the rest as needed. ;-)
LanguageFeatures/Class-modifiers/versioning_pre_feature_lib.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Seems that dart-lang/language#3019 answered all the questions, so I updated the tests accordingly. Please review
LanguageFeatures/Class-modifiers/versioning_pre_feature_lib.dart
Outdated
Show resolved
Hide resolved
LanguageFeatures/Class-modifiers/versioning_pre_feature_lib.dart
Outdated
Show resolved
Hide resolved
| dynamic noSuchMethod(Invocation i) {} | ||
| } | ||
|
|
||
| base mixin BaseMixinOnPreFeatureExtendsFinal on PreFeatureExtendsFinal { |
There was a problem hiding this comment.
These three cases are tricky, too.
We do have a rule that the on type of a mixin cannot be a final class of a different library, dart-lang/language#2933, but I can't find that in the feature spec.
We have a similar rule for a sealed class in a different library (here).
But none of those rules are applicable here, so I'd expect this one plus the next two to be non-errors.
There was a problem hiding this comment.
Moved these cases to a separate test which is not expecting any errors
There was a problem hiding this comment.
As far as I can see we still have MixinOnPreFeatureExtendsFinal, MixinOnPreFeatureImplementsFinal, MixinOnPreFeatureWithFinal in LanguageFeatures/Class-modifiers/versioning_A03_t02.dart. Did you move some other declarations?
There was a problem hiding this comment.
Oh, I'm sorry, it's my mistake!
It is true that PreFeature...Final are all allowed to exist, and the on type of a post-feature mixin is allowed to be any of those PreFeature...Final classes.
But I forgot that the platform class is final and the mixin isn't "base, final, or sealed" (which means base because a mixin can't be the other two). (I also misremembered the situation and thought that there were just 3 declarations, so nothing had been moved. But there were 6 and 3 of them had been moved.)
The transitive check kicks in, and it doesn't care that we're going through a legacy library, we just note that there is a final (indirect) superinterface, and the mixin isn't base. So we should expect the "missing base" error after all.
That doesn't actually fit the @description in LanguageFeatures/Class-modifiers/versioning_A03_t02.dart, but there should be some test about missing base... where they'd fit (or they can just be in a new test if it's hard to find a good one).
Sorry again! 😸
There was a problem hiding this comment.
No problem, I too missed that on relation is like an implements and restrictions for it are transitive. Moved these tests to a separate test with appropriate description.
|
Only |
2023-05-04 [email protected] Fixes dart-lang/co19#2034. Fix roll failures, add issues numbers (dart-lang/co19#2035) 2023-05-03 [email protected] dart-lang/co19#1401. Async for-in tests for patterns (dart-lang/co19#2032) 2023-04-28 [email protected] dart-lang/co19#1959. Grammar tests added (dart-lang/co19#2031) 2023-04-28 [email protected] dart-lang/co19#1959. Versioning tests added (dart-lang/co19#2029) Change-Id: Ic29913ebd7260c4069e6baa27a797219ce12d956 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/301440 Reviewed-by: Alexander Thomas <[email protected]>
Please note that
versioning_A03_t*andversioning_A04_t*are failing on the edge SDK