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

Analyzer treats required named parameters in superclasses as non-optional from legacy libraries #43397

Closed
munificent opened this issue Sep 11, 2020 · 5 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. NNBD Issues related to NNBD Release

Comments

@munificent
Copy link
Member

Given this legacy library:

// @dart=2.8
import 'null_safe.dart';

class Legacy extends NullSafe {}

main() {
  Legacy();
}

// And this null-safe one:

class NullSafe {
  NullSafe({required String s});
}

I would expect this code to be OK. The implicit super() call in Legacy to call NullSafe() should be allowed because NullSafe's required named parameter should be treated as optional from the context of a legacy library.

Instead, I get:

Analyzing /Users/rnystrom/Documents/legacy.dart...
  error • The superclass 'NullSafe' doesn't have a zero argument constructor. • /Users/rnystrom/Documents/legacy.dart:4:7 • no_default_super_constructor
1 error found.
@munificent munificent added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. NNBD Issues related to NNBD Release labels Sep 11, 2020
@nshahan
Copy link
Contributor

nshahan commented Sep 11, 2020

And a reference to required parameters being optional in the definition of LEGACY_SUBTYPE(S, T) https://github.com/dart-lang/language/blob/master/accepted/future-releases/nnbd/feature-specification.md#subtyping.

@leafpetersen
Copy link
Member

@munificent we evidently don't have a test for this, any chance you could add one?

@scheglov scheglov self-assigned this Sep 11, 2020
@munificent
Copy link
Member Author

How did I know you were going to say that? :) @scheglov, would you like me to write a test for this or will you in the process of fixing it? I won't get to it until Monday, but I'm happy to then.

@scheglov
Copy link
Contributor

This should fix the issue in the analyzer.

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

dart-bot pushed a commit that referenced this issue Sep 12, 2020
Bug: #43397
Change-Id: I1cd4ae4bea441732d1c6ef68a917fcccb144ec62
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/162584
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
dart-bot pushed a commit that referenced this issue Sep 21, 2020
Change-Id: I756d4f4a768ff08e81fd2b90a6c76d833cc8ba0b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/163740
Auto-Submit: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Bob Nystrom <rnystrom@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
@munificent
Copy link
Member Author

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. NNBD Issues related to NNBD Release
Projects
None yet
Development

No branches or pull requests

4 participants