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

NNBD Wrong or no error for different type getter/setter with the same basename #40333

Closed
sgrekhov opened this issue Jan 27, 2020 · 2 comments
Closed
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. NNBD Issues related to NNBD Release type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@sgrekhov
Copy link
Contributor

NNBD spec reads

It is an error if a class has a setter and a getter with the same basename where the return type of the getter is not a subtype of the argument type of the setter. Note that this error specifically requires subtyping and not assignability and hence makes no exception for dynamic.

In fact analyzer finds no issues in the code below

class C {
  void set test(int v) {}
  dynamic get test => 3.14;
}

main() {
  C c = new C();
  c.test = 1;
  c.test;
}

In the case when we do have an error, error message is wrong

class C {
  void set test(int v) {}
  num get test => 3.14;
}

main() {
  C c = new C();
  c.test = 1;
  c.test;
}

Here we have error - The return type of getter 'test' is 'num' which isn't assignable to the type 'int' of its setter 'test'.

But NNBD spec clearly says about 'subtyping', not about 'assignability'

Tested on dartanalyzer version 2.8.0-dev.5.0

@lrhn lrhn added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. NNBD Issues related to NNBD Release type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Jan 27, 2020
@scheglov
Copy link
Contributor

Thank you for the report.
Yes, checking for types of getters / setters is not right for NNBD.

https://dart-review.googlesource.com/c/sdk/+/133640 is a first step to fix it, using subtyping and a better error message.

For the first example analyzer was reporting an error in NNBD libraries, with the bleeding edge version. I don't remember fixing it, but maybe this happened accidentally.

@scheglov scheglov self-assigned this Jan 28, 2020
dart-bot pushed a commit that referenced this issue Jan 29, 2020
…TYPE_SETTER_TYPES for NNBD.

Bug: #40333
Change-Id: I265d535956204644859b05d02dd39a0bfdc8c19a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133640
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@scheglov
Copy link
Contributor

dart-bot pushed a commit that referenced this issue Jan 29, 2020
Bug: #40333
Change-Id: I10f366d4f451a94f8afa7b30cd6daa99073d7eda
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/133665
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
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 type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Development

No branches or pull requests

3 participants