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

[CP][cfe][analyzer] Add errors for base transitivity and base/final implements of a core lib declaration through a 2.19 library and a 3.0 library. #52406

Closed
kallentu opened this issue May 15, 2023 · 3 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable

Comments

@kallentu
Copy link
Member

Commit(s) to merge

95565e2, 7536f86, 6de1afd, ea21df9, 9c34ff7

Target

Stable

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/302851

Issue Description

An error is not produced when a post-feature library implements a pre-feature library declaration that has a final/base core library class as a super declaration.

An error is not produced when a post-feature library subtypes (but not implements) a pre-feature library declaration that has a final/base core library class as a super declaration.

Example

An example with a few cases (not exhaustive, but just to show what the issue is).

Assuming there's some base or final declaration in the core libraries:

// dart:core/collection
abstract base mixin class LinkedListEntry<E extends LinkedListEntry<E>> { ... }
final class MapEntry<K, V> { ... }

Then in a 2.19 or pre-feature library, we subtype the core library declarations which are marked base or final. There are no errors here (as intended).

// a.dart
// @dart=2.19
import 'dart:core';
import 'dart:collection';
class A extends MapEntry<int, int> {}
class B extends LinkedListEntry<Never> {}

AND lastly in a 3.0 or post-feature library, we subtype the 2.19 declarations.

// b.dart
import 'a.dart';
class AImplements implements A {}
class AExtends extends A {}
class AWith with A {}

class BImplements implements B {}
class BExtends extends B {}
class BWith extends B {}

AImplements and BImplements do not produce an error, but should produce an error for implementing the base/final declaration from the core libraries.
AExtends, AWith, BExtends, BWith do not produce an error, but should produce an error for not being marked base/final/sealed.

What is the fix

We add additional error checking for this particular case and make sure we check these transitive rules through pre-feature libraries when we weren't before.

There's a fix added to both the CFE and the analyzer.

Why cherry-pick

This change makes sure users are closer to correct code for the future when they migrate to 3.0 and fills in a hole in the spec for how core library declarations are used in legacy libraries and downwards.

The impacted users would be anyone using a library that's not migrated to 3.0 whose declarations subtype the core libraries' declarations. I don't have the best read on how many users this would be, but I don't suspect it would be many at the moment.

Risk

low

Issue link(s)

#52078, #52115, #52315

Extra Info

No response

@kallentu
Copy link
Member Author

cc. @leafpetersen

@kallentu kallentu added the cherry-pick-review Issue that need cherry pick triage to approve label May 15, 2023
@lrhn lrhn added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label May 17, 2023
@itsjustkevin
Copy link
Contributor

@johnniwinther can you also take a look at this cherry-pick?

@kallentu kallentu added the area-front-end Use area-front-end for front end / CFE / kernel format related issues. label May 19, 2023
@johnniwinther
Copy link
Member

LGTM

@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-approved Label for approved cherrypick request labels May 22, 2023
copybara-service bot pushed a commit that referenced this issue May 22, 2023
…f a core lib declaration through a 2.19 library and a 3.0 library.

Cherry pick for the following behaviour from 3 different CLs.

Error produced when a post-feature library implements
a pre-feature library declaration that has a final/base core library class as a super declaration.

Error produced when a post-feature library subtypes (but not implements)
a pre-feature library declaration that has a final/base core library class as a super declaration.

Fixes: #52406

Bug: #52315, #52115, #52078
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/302365
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/301503
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/300861
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/301442
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/300880
Change-Id: I2a192a157d417c1de7656aa624d3b10d9e67626e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302851
Commit-Queue: Kallen Tu <[email protected]>
Reviewed-by: Konstantin Shcheglov <[email protected]>
Reviewed-by: Johnni Winther <[email protected]>
@itsjustkevin itsjustkevin added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. area-front-end Use area-front-end for front end / CFE / kernel format related issues. cherry-pick-approved Label for approved cherrypick request cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable
Projects
None yet
Development

No branches or pull requests

8 participants