[analyzer] No error produced for not-implementing-final-classes through legacy libraries.

[lrhn]

Example:

// old_lib.dart
// @dart=2.19

// MapEntry is `final`, but can be implemented due to SDK exception.
class M implements MapEntry<int, int> {
  int get key => 0;
  int get value => 1;
  String toString() => "Bad";
}

and

// new_lib.dart
// @dart=3.0

import "old_lib.dart";

class N implements M {
  int get key => 0;
  int get value => 1;
  String toString() => "Evil";
}

void main() {
  print(N()); // Evil.
}

This gives no errors, or warnings, in the front end or analyzer.

According to specification, you are not allowed to implement any interface which has any superinterface whose declaration is marked base or final. (Unless you are, yourself, a pre-feature library and all such superinterfaces are in platform libraries.)

Here N is not in a pre-feature library, so it gets no exemptions.
M has a superinterface marked final, so it must not be implemented by N.

(We don't have a rule against N extending M, so its not like we can prevent everything, but we can prevent new implements of a final platform type in 3.0-libraries.)

Filing as area-fe-analyzer-shared. Split this into two issues if it's two different bugs.

[lrhn]

@kallentu

[kallentu]

It's not shared code so I'm gonna split this bug.

And this is in the same area as the base implements bug, will look into it

[leafpetersen]

I think we're probably just going to accept this behavior here. It would be breaking to change this post release, and I don't see it rising to cherry pick level at this point. I'm also not 100% convinced that it's what we want. The problem here is in the legacy library. The Dart 3 code has no ability to control or fix the problem. Eventually, the legacy library will have to do something other than implement MapEntry, and at that point, the Dart 3 code will be correct. Until then, it seems better to just let the code work.

[eernstg]

@leafpetersen, if I'm reading the timezone info correct then your comment here was written before the language team meeting yesterday, and I believe the decision at that meeting was to adjust the tool behavior to fit the current specification (so class N in this comment will be an error).

We do have co19 tests covering this case, but they are only being rolled in right now. For example, https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Class-modifiers/versioning_A03_t02.dart has a few dozen post-feature classes that implement a pre-feature class that implements or extends or mixes in a final platform class.

[kallentu]

@sgrekhov Just to let you know, the analyzer errors in https://github.com/dart-lang/co19/blob/master/LanguageFeatures/Class-modifiers/versioning_A03_t01.dart should now be added.

Analyzer (not CFE yet) work for this bug is complete.

[sgrekhov]

@kallentu thank you! I checked the tests on the just built SDK on Linux x64. We had 3 failing tests for the case when superinterface chain goes through a pre-feature library

versioning_A03_t01.dart - checks that final class cannot be extended
versioning_A03_t02.dart - checks that final class cannot be implemented
versioning_A03_t04.dart - checks that mixin on a final class cannot be declared

Testing shows me that versioning_A03_t02.dart is fixed and passes now. But other two tests are still failing. I think they should be fixed as well

$ python3 tools/test.py -n analyzer-asserts-linux co19/LanguageFeatures/Class-modifiers/versioning_A03*
No build targets found.
Test configuration:
    analyzer-asserts-linux(architecture: x64, compiler: dart2analyzer, mode: release, runtime: none, system: linux, enable-asserts, use-sdk)
Suites tested: co19
[00:04 |  25% | +    1 | -    0]

FAILED: dart2analyzer-none release_x64 co19/LanguageFeatures/Class-modifiers/versioning_A03_t01
Expected: Pass
Actual: MissingCompileTimeError

--- Command "dart2analyzer" (took 04.000019s):
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/dart-sdk/bin/dart out/ReleaseX64/gen/dartanalyzer.dart.snapshot -Dtest_runner.configuration=analyzer-asserts-linux --ignore-unrecognized-flags --packages=/home/sgrekhov/Google/dart-sdk/sdk/.dart_tool/package_config.json --format=json /home/sgrekhov/Google/dart-sdk/sdk/tests/co19/src/LanguageFeatures/Class-modifiers/versioning_A03_t01.dart

static error failures:
- Missing expected unspecified error at line 20, column 7, length 34.

- Missing expected unspecified error at line 25, column 7, length 37.

- Missing expected unspecified error at line 30, column 7, length 31.

- Missing expected unspecified error at line 35, column 17, length 43.

- Missing expected unspecified error at line 40, column 17, length 46.

- Missing expected unspecified error at line 45, column 17, length 40.

- Missing expected unspecified error at line 50, column 16, length 42.

- Missing expected unspecified error at line 55, column 16, length 45.

- Missing expected unspecified error at line 60, column 16, length 39.

- Missing expected unspecified error at line 65, column 26, length 51.

- Missing expected unspecified error at line 70, column 26, length 54.

- Missing expected unspecified error at line 75, column 26, length 48.

--- Re-run this test:
python3 tools/test.py -n analyzer-asserts-linux co19/LanguageFeatures/Class-modifiers/versioning_A03_t01
[00:04 |  75% | +    2 | -    1]

FAILED: dart2analyzer-none release_x64 co19/LanguageFeatures/Class-modifiers/versioning_A03_t04
Expected: Pass
Actual: MissingCompileTimeError

--- Command "dart2analyzer" (took 04.000061s):
DART_CONFIGURATION=ReleaseX64 out/ReleaseX64/dart-sdk/bin/dart out/ReleaseX64/gen/dartanalyzer.dart.snapshot -Dtest_runner.configuration=analyzer-asserts-linux --ignore-unrecognized-flags --packages=/home/sgrekhov/Google/dart-sdk/sdk/.dart_tool/package_config.json --format=json /home/sgrekhov/Google/dart-sdk/sdk/tests/co19/src/LanguageFeatures/Class-modifiers/versioning_A03_t04.dart

static error failures:
- Missing expected unspecified error at line 20, column 7, length 29.

- Missing expected unspecified error at line 27, column 7, length 32.

- Missing expected unspecified error at line 34, column 7, length 26.

--- Re-run this test:
python3 tools/test.py -n analyzer-asserts-linux co19/LanguageFeatures/Class-modifiers/versioning_A03_t04
[00:04 | 100% | +    2 | -    2]

=== 2 tests passed, 2 failed ===

[eernstg]

Sounds like we would need to reopen this issue, but I suspect there may be an issue which is a better fit (or a new one should be created, rather than reopening this one):

This issue is about having an implements superinterface edge in new code which causes a compile-time error (because there is a final class in the superinterface graph, and this is still an error even in the case where all paths in the superinterface graph go through a legacy class before they reach that final class).

The failures in LanguageFeatures/Class-modifiers/versioning_A03_t01.dart and ..._t04.dart are caused by the fact that the CFE and the analyzer do not report a compile-time error based on the missing base/final/sealed modifier. That error occurs for extends and on edges in the superinterface graph as well as implements edges. Presumably the failure only arises because the relevant edges in the superinterface graph go through a legacy library.

Is there an open issue about this situation? Otherwise we should probably handle it in a new issue.

Edit: We now have #52321, covering the cases mentioned here.